Allow default values for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2819

harry75369 · 2021-06-15T02:25:21Z

These two macros use json::at for deserializing from json to obj, but it will throw exceptions and is not good for backward-compatibility if the struct adds new fields but the json is still unchanged.

This simple patch uses the default value from struct type (the type should be default constructible), and use json::value to set default value if some fileds is missing in json value. Hope it helps.

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 file single_include/nlohmann/json.hpp. The whole process is described here.

…DEFINE_TYPE_NON_INTRUSIVE

harry75369 · 2021-06-15T02:48:39Z

It failed unit test "unit-udt_macro.cpp" as line 256 expects an exception is thrown. No idea if this behavior should be changed.

nlohmann · 2021-06-15T06:12:16Z

This would be a breaking change, hence the tests fail. Best would be to introduce a new macro, though I'm not sure what a good name could be.

gregmarr · 2021-06-15T16:08:17Z

This change also requires that the destination type be default constructible, which I believe is explicitly not a requirement currently.

…NE_TYPE_NON_INTRUSIVE; added new macros NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

harry75369 · 2021-06-22T01:04:34Z

Changes to old macros are reverted and new macros are added witih passed unit tests, which are

NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

nlohmann

Can you please also edit

coveralls · 2021-06-22T06:37:44Z

Coverage increased (+0.02%) to 100.0% when pulling 321cc51 on harry75369:develop into 18a5f4c on nlohmann:develop.

…MANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT accordingly

harry75369 · 2021-06-25T11:47:40Z

Can you please also edit

* https://github.com/nlohmann/json/blob/develop/doc/mkdocs/docs/features/macros.md

* https://github.com/nlohmann/json/blob/develop/doc/mkdocs/docs/features/arbitrary_types.md#simplify-your-life-with-macros

Done.

gregmarr · 2021-06-25T15:59:28Z

include/nlohmann/detail/macro_scope.hpp

@@ -291,6 +296,10 @@
    inline void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    inline void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

+#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(Type, ...)  \
+    inline void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
+    inline void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { Type default_obj; NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM_WITH_DEFAULT, __VA_ARGS__)) }


I was thinking that having default_obj be const static would save constructing and destructing this object on every pass through the function, but there could be reasons that users wouldn't want that. I suppose that this is just something that users will need to be aware of.

Making it const static sounds a good idea. Any specific reason for not wanting that?

Nothing in general, just could be specific use cases where having a long-lived one of these would be a problem for people.

Thinking about this, I also think having a long-lived object to be surprising.

nlohmann_json_t is already default constructed. The WITH_DEFAULT macros can just not set it if the key is missing, and then no Type default_obj; is needed.

That's not necessarily true. We ran into situations recently where to_json was called on a json object that already had a value, and the functions didn't clear it properly, resulting in a memory leak.

gregmarr · 2021-06-25T16:02:50Z

doc/mkdocs/docs/features/macros.md

@@ -79,6 +79,10 @@ The first parameter is the name of the class/struct, and all remaining parameter

 See [Simplify your life with macros](arbitrary_types.md#simplify-your-life-with-macros) for an example.

+## `NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(type, member...)`
+
+This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE` but will throw no exceptions. When converting JSON object to type object, if any field is missing in JSON object, it will use default value constructed by the type itself.


I'm thinking this should call out that it requires that the type be default constructible and that it constructs an object inside the from_json() function. I think it may also still throw exceptions if the types don't match.

Perhaps something like this:

This macro is similar to NLOHMANN_DEFINE_TYPE_INTRUSIVE. It will not throw an exception in from_json() due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The from_json() function default constructs an object and uses its values as the defaults when calling the value() function.

Sounds better, thanks. Already updated.

…OHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

nlohmann · 2021-06-26T09:41:34Z

doc/mkdocs/docs/features/arbitrary_types.md


-In both macros, the first parameter is the name of the class/struct, and all remaining parameters name the members.
+In all macros, the first parameter is the name of the class/struct, and all remaining parameters name the members. You can read more docs about them starting from [here](https://github.com/nlohmann/json/blob/develop/doc/mkdocs/docs/features/macros.md#nlohmann_define_type_intrusivetype-member).


Suggested change

In all macros, the first parameter is the name of the class/struct, and all remaining parameters name the members. You can read more docs about them starting from [here](https://github.com/nlohmann/json/blob/develop/doc/mkdocs/docs/features/macros.md#nlohmann_define_type_intrusivetype-member).

In all macros, the first parameter is the name of the class/struct, and all remaining parameters name the members. You can read more docs about them starting from [here](macros.md#nlohmann_define_type_intrusivetype-member).

nlohmann · 2021-06-26T09:42:53Z

doc/mkdocs/docs/features/macros.md

@@ -79,6 +79,10 @@ The first parameter is the name of the class/struct, and all remaining parameter

 See [Simplify your life with macros](arbitrary_types.md#simplify-your-life-with-macros) for an example.

+## `NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT(type, member...)`
+
+This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.


Suggested change

This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.

This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON object, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.

nlohmann · 2021-06-26T09:43:14Z

doc/mkdocs/docs/features/macros.md

@@ -88,6 +92,10 @@ The first parameter is the name of the class/struct, and all remaining parameter

 See [Simplify your life with macros](arbitrary_types.md#simplify-your-life-with-macros) for an example.

+## `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(type, member...)`
+
+This macro is similar to `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.


Suggested change

This macro is similar to `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.

This macro is similar to `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON object, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.

nlohmann · 2021-06-26T09:44:59Z

doc/mkdocs/docs/features/arbitrary_types.md


 - `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(name, member1, member2, ...)` is to be defined inside of the namespace of the class/struct to create code for.
 - `NLOHMANN_DEFINE_TYPE_INTRUSIVE(name, member1, member2, ...)` is to be defined inside of the class/struct to create code for. This macro can also access private members.
+- `NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(name, member1, member2, ...)` is to be defined inside of the namespace of the class/struct to create code for.


It would be good to add a comment about the difference to the previous macros.

Furthermore, the order should be.

NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE

NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

NLOHMANN_DEFINE_TYPE_INTRUSIVE

NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT

nlohmann · 2021-06-26T09:46:16Z

include/nlohmann/detail/macro_scope.hpp

@@ -291,6 +296,10 @@
    inline void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    inline void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

+#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(Type, ...)  \
+    inline void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
+    inline void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { Type default_obj; NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM_WITH_DEFAULT, __VA_ARGS__)) }


It might be safer to add nlohmann_json_ as prefix to default_obj to avoid name clashes.

nlohmann · 2021-06-26T09:47:27Z

test/src/unit-udt_macro.cpp

@@ -59,6 +59,42 @@ class person_with_private_data
    NLOHMANN_DEFINE_TYPE_INTRUSIVE(person_with_private_data, age, name, metadata)
 };

+class person_with_private_data_2


Do you actually test how JSON objects with missing values are serialized?

Test added.

…MANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

harry75369 · 2021-07-09T14:46:20Z

Why is this PR still not merged yet after so many days? All including docs are already editted as sugguested.

nlohmann · 2021-07-09T15:46:32Z

Because this is a side project of mine and i did not find the time to check your last changes.

nlohmann · 2021-07-11T12:16:45Z

I messed up the CI lately, I'm sorry. Can you please update to the latest develop branch?

harry75369 · 2021-07-12T12:07:03Z

I messed up the CI lately, I'm sorry. Can you please update to the latest develop branch?

Do you mean I make an empty commit to trigger the CI?

nlohmann · 2021-07-12T12:44:29Z

Just update your branch and commit the changes.

Martinii89 · 2021-08-06T13:14:01Z

I was surprised to find this just as I made my own macro for pretty much the same thing. I'd love for this to get merged in.

Here was my attempt on it

#define NLOHMANN_JSON_FROM_OPTIONAL(v1) if (const auto it = nlohmann_json_j.find(#v1); it != nlohmann_json_j.end()) {it->get_to(nlohmann_json_t.v1);}

#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_OPTIONAL(Type, ...)  \
    inline void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    inline void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM_OPTIONAL, __VA_ARGS__)) }

* ♻️ split Unicode tests

* 🔥 clean CMake files and Makefile * 💡 update documentation

Fixes nlohmann#2491

nlohmann · 2021-08-08T11:27:08Z

I'm confused - did you update to the latest develop branch?

harry75369 · 2021-08-08T15:13:13Z

I'm confused - did you update to the latest develop branch?

Yes, I pulled latest develop branch, and rebased.

nlohmann · 2021-08-09T06:11:46Z

I'm confused - did you update to the latest develop branch?

Yes, I pulled latest develop branch, and rebased.

I am confused, because the diff now shows me changes in 51 files. 🤔

NoumanNawaz51 · 2021-09-02T15:17:55Z

I still have no idea that if this works or not. But from the README.md that macro does'nt work at all. Or is it in progress ?

nlohmann · 2021-09-02T18:08:26Z

I still have no idea that if this works or not. But from the README.md that macro does'nt work at all. Or is it in progress ?

No, this PR is not yet merged.

nlohmann · 2021-09-02T18:09:18Z

I'm confused - did you update to the latest develop branch?

Yes, I pulled latest develop branch, and rebased.

I am confused, because the diff now shows me changes in 51 files. 🤔

Hey @harry75369, can you please fix the branch by updating it to the latest develop branch?

lljbash · 2021-11-09T06:55:12Z

Any update on this? I really like the macros for backward compatibility. Please let me know if there's anything I can help with to get it merged.

stale · 2022-01-09T02:24:32Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

AndTessitore · 2022-01-25T10:56:23Z

Still not merged? Those macros are really usefull!

nlohmann · 2022-01-25T11:33:15Z

Still not merged? Those macros are really usefull!

The PR needs to be rebased. Can you do this, @harry75369 ?

pketelsen · 2022-01-26T09:35:26Z

Still not merged? Those macros are really usefull!

The PR needs to be rebased. Can you do this, @harry75369 ?

Hi Nils!
I cherry-picked @harry75369 's commits in PR #3143. I don't know the protocol for stalled PRs, I hope that this is ok. I would really like to have this feature as well.

nlohmann · 2022-01-30T21:07:45Z

Can be closed as #3143 is merged. Thanks a lot everybody!

Allow default values for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_…

76f6740

…DEFINE_TYPE_NON_INTRUSIVE

harry75369 requested a review from nlohmann as a code owner June 15, 2021 02:25

Reverted changes for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFI…

7aeda9d

…NE_TYPE_NON_INTRUSIVE; added new macros NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

nlohmann requested changes Jun 22, 2021

View reviewed changes

Updated docs for NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and NLOH…

5a5832e

…MANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT accordingly

gregmarr reviewed Jun 25, 2021

View reviewed changes

Rephrased docs for NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and NL…

5664233

…OHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

nlohmann reviewed Jun 26, 2021

View reviewed changes

nlohmann requested changes Jun 26, 2021

View reviewed changes

harry75369 added 4 commits June 28, 2021 18:02

Updated docs for NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and NLOH…

d8ba9f4

…MANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT

Renamed default_obj in to avoid name clashes

35c37c8

Added test for serialization of default constructed object

f452a92

Add const to getters for macro tests

98ac25b

trigger upstream CI

db0287d

nlohmann added the please rebase Please rebase your branch to origin/develop label Jul 22, 2021

Consistency with using in README.md

a5104d1

nlohmann and others added 12 commits August 8, 2021 08:56

👥 update contributors

9661e66

Refactor Unicode tests (nlohmann#2889)

8da9e25

* ♻️ split Unicode tests

CMake cleanup (nlohmann#2885)

a281280

* 🔥 clean CMake files and Makefile * 💡 update documentation

Properly constrain the basic_json conversion operator

d9562c1

Fixes nlohmann#2491

🚨 fix useless-cast warnings

e9797ac

🚨 fix useless-cast warnings

974e386

🚨 fix useless-cast warnings

b83acfe

📄 add CITATION file

0c4af65

📝 add note for wstring handling

9e7550a

Supress -Wfloat-equal on intended float comparisions

8c6ea74

Add more suppressions on float comparisons

5b6fd51

Remove -Wfloat-equal suppressions on tests

321cc51

nlohmann removed the please rebase Please rebase your branch to origin/develop label Aug 8, 2021

This was referenced Nov 10, 2021

Fixing your nlohmann/json pull request. harry75369/json#1

Closed

Add macros NLOHMANN_DEFINE_TYPE_INTRUSIVE_WITH_DEFAULT and ..._NON_INTRUSIVE_WITH_DEFAULT #3143

Merged

stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jan 9, 2022

stale bot removed the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jan 25, 2022

nlohmann closed this Jan 30, 2022


		In both macros, the first parameter is the name of the class/struct, and all remaining parameters name the members.
		In all macros, the first parameter is the name of the class/struct, and all remaining parameters name the members. You can read more docs about them starting from [here](https://github.com/nlohmann/json/blob/develop/doc/mkdocs/docs/features/macros.md#nlohmann_define_type_intrusivetype-member).

	This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.
	This macro is similar to `NLOHMANN_DEFINE_TYPE_INTRUSIVE`. It will not throw an exception in `from_json()` due to a missing value in the JSON object, but can throw due to a mismatched type. In order to support that it requires that the type be default constructible. The `from_json()` function default constructs an object and uses its values as the defaults when calling the `value()` function.

Allow default values for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2819

Allow default values for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2819

Conversation

harry75369 commented Jun 15, 2021

Pull request checklist

harry75369 commented Jun 15, 2021

nlohmann commented Jun 15, 2021

gregmarr commented Jun 15, 2021

harry75369 commented Jun 22, 2021

nlohmann left a comment

Choose a reason for hiding this comment

coveralls commented Jun 22, 2021 • edited Loading

harry75369 commented Jun 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harry75369 commented Jul 9, 2021

nlohmann commented Jul 9, 2021

nlohmann commented Jul 11, 2021

harry75369 commented Jul 12, 2021

nlohmann commented Jul 12, 2021

Martinii89 commented Aug 6, 2021

nlohmann commented Aug 8, 2021

harry75369 commented Aug 8, 2021

nlohmann commented Aug 9, 2021

NoumanNawaz51 commented Sep 2, 2021

nlohmann commented Sep 2, 2021

nlohmann commented Sep 2, 2021

lljbash commented Nov 9, 2021

stale bot commented Jan 9, 2022

AndTessitore commented Jan 25, 2022

nlohmann commented Jan 25, 2022

pketelsen commented Jan 26, 2022

nlohmann commented Jan 30, 2022

coveralls commented Jun 22, 2021 •

edited

Loading