Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NLOHMANN_DEFINE_DERIVED_TYPE_* macros #4033

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

rotolof
Copy link

@rotolof rotolof commented May 18, 2023

I created the new macros:

  • NLOHMANN_DEFINE_DERIVED_TYPE_INTRUSIVE
  • NLOHMANN_DEFINE_DERIVED_TYPE_INTRUSIVE_WITH_DEFAULT
  • NLOHMANN_DEFINE_DERIVED_TYPE_NON_INTRUSIVE
  • NLOHMANN_DEFINE_DERIVED_TYPE_NON_INTRUSIVE_WITH_DEFAULT

to recreate the proposed fix to #2199.
The macros accept as second parameter the base class.

Usage example:

class A {
  double Aa;
  double Ab;
  NLOHMANN_DEFINE_TYPE_INTRUSIVE(A, Aa, Ab)
};

class B : public A {
  int Ba;
  int Bb;
  NLOHMANN_DEFINE_DERIVED_TYPE_INTRUSIVE(B, A, Ba, Bb)
};

What they do is basically calling the to_json/from_json functions of the base class before serialising/deserialising the class members:

void to_json(nlohmann::json& j, const B& b) {
    nlohmann::to_json(j, static_cast<const A&>(b));
    // ...
}

void from_json(const nlohmann::json& j, B& b) {
    nlohmann::from_json(j, static_cast<A&>(b));
    // ...
}

Pull request checklist

Read the Contribution Guidelines for detailed information.

  • Changes are described in the pull request, or an existing issue is referenced.
  • The test suite compiles and runs without error.
  • Code coverage is 100%. Test cases can be added by editing the test suite.
  • The source code is amalgamated; that is, after making changes to the sources in the include/nlohmann directory, run make amalgamate to create the single-header files single_include/nlohmann/json.hpp and single_include/nlohmann/json_fwd.hpp. The whole process is described here.

@coveralls
Copy link

coveralls commented May 18, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling b6e2fd1 on rotolof:derived_macros into edffad0 on nlohmann:develop.

@hacker-cb
Copy link

Any updates about this PR?

Niccolò Iardella added 2 commits September 16, 2023 08:40
# Conflicts:
#	include/nlohmann/detail/macro_scope.hpp
#	single_include/nlohmann/json.hpp
@rotolof
Copy link
Author

rotolof commented Sep 16, 2023

I just solved the conflict that had arisen in the meantime.

@nlohmann
Copy link
Owner

I am currently finishing #2998 to finally have a stable CI. Once this is merged, please update your branch.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nlohmann
Copy link
Owner

(Also, please update from the develop branch and fix the conflicts.)

@nlohmann nlohmann added the please rebase Please rebase your branch to origin/develop label Sep 24, 2023
@eyalk1
Copy link

eyalk1 commented Sep 26, 2023

since this code now actually modifies the json object from base - what do you guys think about using merge_patch or += instead of = for each member?
for example:

#define NLOHMANN_JSON_PAIR(member) {#member, nlohmann_json_t.member},

#define NLOHMANN_INIT_PAIRS(...)                                               \
  NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_PAIR, __VA_ARGS__))

#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_INHERITANCE(derived, base, ...)     \
  void to_json(nlohmann::json &nlohmann_json_j,                                \
               derived const &nlohmann_json_t) {                               \
    to_json(nlohmann_json_j, static_cast<base const &>(nlohmann_json_t));      \
    nlohmann_json_j.merge_patch(nlohmann::json{                                \
        NLOHMANN_JSON_EXPAND(NLOHMANN_INIT_PAIRS(__VA_ARGS__))});              \
  }                                                                            \
  void from_json(nlohmann::json const &nlohmann_json_j,                        \
                 derived &nlohmann_json_t) {                                   \
    from_json(nlohmann_json_j, static_cast<base &>(nlohmann_json_t));          \
    NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) \
  }

Niccolò Iardella added 2 commits October 18, 2023 15:57
# Conflicts:
#	single_include/nlohmann/json_fwd.hpp
@rotolof
Copy link
Author

rotolof commented Oct 18, 2023

I updated to the latest develop and added documentation, let me know it is ok now

@nlohmann nlohmann removed the please rebase Please rebase your branch to origin/develop label Oct 18, 2023
@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @rotolof
Please read and follow the Contribution Guidelines.

1 similar comment
@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @rotolof
Please read and follow the Contribution Guidelines.

@gregmarr
Copy link
Contributor

@rotolof Looks like you might be using a different version of astyle (perhaps even a newer one) which has different behavior than the current version being used. #4180 (comment)

@rotolof
Copy link
Author

rotolof commented Oct 20, 2023

@gregmarr Thanks, I was using macOS brew's astyle that is 3.4.9. I tried with the suggested 3.1 but it does not support squeeze-lines, wtf?

@nlohmann
Copy link
Owner

Comment that line from the config and try again. Sorry for the inconvenience - I did not find the time to fix this yet.

@Aeshal
Copy link

Aeshal commented May 9, 2024

Are there any chances to release that soon? Seems like the most of work has been done and it just hanging almost a year now.

@rotolof
Copy link
Author

rotolof commented May 9, 2024

Are there any chances to release that soon? Seems like the most of work has been done and it just hanging almost a year now.

Sorry, I completely forgot about this until now! Let me check it again.

Niccolò Iardella added 2 commits May 9, 2024 17:11
@rotolof rotolof requested a review from nlohmann May 10, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants