alternative macro for throwing on enum serialization errors #3996

pabloariasal · 2023-03-25T21:27:49Z

#3992

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.

Please don't

The C++11 support varies between different compilers and versions. Please note the list of supported compilers. Some compilers like GCC 4.7 (and earlier), Clang 3.3 (and earlier), or Microsoft Visual Studio 13.0 and earlier are known not to work due to missing or incomplete C++11 support. Please refrain from proposing changes that work around these compiler's limitations with #ifdefs or other means.
Specifically, I am aware of compilation problems with Microsoft Visual Studio (there even is an issue label for this kind of bug). I understand that even in 2016, complete C++11 support isn't there yet. But please also understand that I do not want to drop features or uglify the code just to make Microsoft's sub-standard compiler happy. The past has shown that there are ways to express the functionality such that the code compiles with the most recent MSVC - unfortunately, this is not the main objective of the project.
Please refrain from proposing changes that would break JSON conformance. If you propose a conformant extension of JSON to be supported by the library, please motivate this extension.
Please do not open pull requests that address multiple issues.

nlohmann · 2023-03-25T21:29:16Z

docs/examples/nlohmann_json_serialize_enum.cpp

@@ -26,7 +26,7 @@ enum class Color
    red, green, blue, unknown
 };

-NLOHMANN_JSON_SERIALIZE_ENUM(Color,


Please add a new example instead of editing this one.

github-actions · 2023-03-25T21:30:20Z

🔴 Amalgamation check failed! 🔴

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

coveralls · 2023-03-25T22:18:32Z

Coverage: 100.0%. Remained the same when pulling 2c9eda7 on pabloariasal:issue3992 into 6af826d on nlohmann:develop.

github-actions · 2023-03-26T10:06:08Z

🔴 Amalgamation check failed! 🔴

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

gregmarr · 2023-03-27T14:19:55Z

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum.md

+??? example "Example 3: Strict error handling"
+
+    The example shows how to define strict error handling for enum serialization.
+    In the example conversion between `Color` and json throw an exception in case of erros (instead of defauting to the first value).


typo: erros instead of errors

gregmarr · 2023-03-27T14:20:28Z

docs/mkdocs/docs/features/enum_conversion.md

@@ -27,6 +27,8 @@ NLOHMANN_JSON_SERIALIZE_ENUM( TaskState, {
 The [`NLOHMANN_JSON_SERIALIZE_ENUM()` macro](../api/macros/nlohmann_json_serialize_enum.md) declares a set of
 `to_json()` / `from_json()` functions for type `TaskState` while avoiding repetition and boilerplate serialization code.

+An alternative macro [`NLOHMANN_JSON_SERIALIZE_ENUM_STRICT()` macro](../api/macros/nlohmann_json_serialize_enum.md) can be used when a more strict error handling is preffered, throwing in case of serialization errors instead of defaulting to the first enum value defined in the macro.


typo: preffered instead of preferred

gregmarr · 2023-03-27T14:23:39Z

include/nlohmann/detail/macro_scope.hpp

@@ -230,6 +230,46 @@
        e = ((it != std::end(m)) ? it : std::begin(m))->first;                                  \
    }

+#if (defined(__cpp_exceptions) || defined(__EXCEPTIONS) || defined(_CPPUNWIND)) && !defined(JSON_NOEXCEPTION)


If this only works when JSON_NOEXCEPTION isn't defined, then that should be noted in the docs. However, I think you can just use JSON_THROW instead of throw and use the normal exception override behavior.

agree, will note it in the docs. I think there are some arguments for not using JSON_NOEXCEPTION:

nested macros don't work (the preprocessor doesn't expand a macro inside another one)

it can lead to silent and hard to debug erros if JSON_NOEXCEPTION is defined and the macro is used -> what should we do in that case?

the macro itself doens't make any sense if JSON_NOEXCEPTION is defined (hence better not define it in the first place.

nested macros don't work (the preprocessor doesn't expand a macro inside another one)

It certainly does. There are many nested macros in the library. https://godbolt.org/z/bbWv8Ye7r

it can lead to silent and hard to debug erros if JSON_NOEXCEPTION is defined and the macro is used -> what should we do in that case?

JSON_THROW calls std::abort when JSON_NOEXCEPTION is defined, unless the user has overridden that by defining JSON_THROW_USER.

the macro itself doens't make any sense if JSON_NOEXCEPTION is defined (hence better not define it in the first place.

The library already has well-defined behavior for what happens to throw calls when JSON_NOEXCEPTION is defined.

ok, very interesting. Thanks for the godbolt link! Now I'm confused, my compiler (gcc 12, fedora 37) doesn't like that:

/home/pablo/workspace/json/tests/src/unit-conversions.cpp:1665:9: required from here /home/pablo/workspace/json/include/nlohmann/detail/macro_scope.hpp:249:23: error: ‘JSON_THROW’ was not declared in this scope; did you mean ‘JSON_TRY’? 249 | JSON_THROW(nlohmann::detail::type_error::create(302, \ | ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 250 | nlohmann::detail::concat("can't serialize enum value ", std::to_string(static_cast<int>(e)), \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 251 | " of enum '", #ENUM_TYPE, "' to json. ", \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 252 | "Did you list the value in NLOHMANN_JSON_SERIALIZE_ENUM_STRICT?"), &j)); \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/pablo/workspace/json/tests/src/unit-conversions.cpp:1618:1: note: in expansion of macro ‘NLOHMANN_JSON_SERIALIZE_ENUM_STRICT’ 1618 | NLOHMANN_JSON_SERIALIZE_ENUM_STRICT(game, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I wonder what I'm doing wrong

My first thought was that JSON_THROW wasn't defined yet, but that's not the case. It appears right before that macro, and it's not possible to get there without it being defined. I'm not sure why gcc would choke on that. The only other thing I can think of is that somehow you have something equivalent to #define JSON_THROW JSON_THROW. Maybe #define JSON_THROW_USER JSON_THROW. Can you reproduce this on compiler explorer?

I'm really clueless, would it be possible for you to try the branch out with the proposed changes locally? Looks like my preprocessor doesn't see JSON_THROW and lets it through to the compiler...

gregmarr · 2023-03-27T14:25:41Z

include/nlohmann/detail/macro_scope.hpp

+        if (it == std::end(m))                                                                                                \
+        {                                                                                                                     \
+            throw nlohmann::detail::type_error::create(302,                                                                   \
+                    nlohmann::detail::concat("can't serialize enum value ", std::to_string(static_cast<int>(e)),              \


Would it make sense to say something about the value not being a enum value listed in NLOHMANN_JSON_SERIALIZE_ENUM_STRICT?

yes, this is a great suggestion.

gregmarr · 2023-03-27T14:26:02Z

include/nlohmann/detail/macro_scope.hpp

+        if (it == std::end(m))                                                                                                \
+        {                                                                                                                     \
+            throw nlohmann::detail::type_error::create(302,                                                                   \
+                    nlohmann::detail::concat("can't deserialize json: '", j, "' to enum '", #ENUM_TYPE "'"), &j);             \


As above, and add the string that couldn't be found.

added same hint as above. The string that could not be found is already being printed, it is j

Ah, right, missed that.

bear24rw · 2023-03-28T18:37:29Z

docs/examples/nlohmann_json_serialize_enum_3.cpp

+{
+    { Color::green, "green" },
+    { Color::blue, "blue" },
+    { Color::red, "rot" } // note that serialization for yellow is missing


Should this say "red" instead of "rot"?

yes, changed

bear24rw · 2023-03-28T18:42:01Z

docs/mkdocs/docs/api/macros/nlohmann_json_serialize_enum.md

 ```

 By default, enum values are serialized to JSON as integers. In some cases this could result in undesired behavior. If an
 enum is modified or re-ordered after data has been serialized to JSON, the later de-serialized JSON data may be
 undefined or a different enum value than was originally intended.

-The `NLOHMANN_JSON_SERIALIZE_ENUM` allows to define a user-defined serialization for every enumerator.
+The `NLOHMANN_JSON_SERIALIZE_ENUM` and `NLOHMANN_JSON_SERIALIZE_ENUM_STRICT` macros allow to define a user-defined serialization for every enumerator.


The wording of this sentence reads really weird to me, a possible alternative:

Suggested change

The `NLOHMANN_JSON_SERIALIZE_ENUM` and `NLOHMANN_JSON_SERIALIZE_ENUM_STRICT` macros allow to define a user-defined serialization for every enumerator.

The `NLOHMANN_JSON_SERIALIZE_ENUM` and `NLOHMANN_JSON_SERIALIZE_ENUM_STRICT` macros allow specifying a user-defined serialization for every enumerator.

This then matches the language in the following sentence: "first value specified in the macro".

much better, thank you

pabloariasal requested a review from nlohmann as a code owner March 25, 2023 21:27

github-actions bot added documentation L tests labels Mar 25, 2023

nlohmann requested changes Mar 25, 2023

View reviewed changes

pabloariasal changed the title ~~alternative macro for strict enum serialization~~ alternative macro for throwing on enum serialization errors Mar 25, 2023

gregmarr reviewed Mar 27, 2023

View reviewed changes

bear24rw reviewed Mar 28, 2023

View reviewed changes

pabloariasal closed this Jul 3, 2023

github-actions bot removed documentation L tests labels Jul 3, 2023

bear24rw mentioned this pull request Aug 7, 2023

NLOHMANN_JSON_SERIALIZE_ENUM defaulting to the first enum value is surprising behavior #3992

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

alternative macro for throwing on enum serialization errors #3996

alternative macro for throwing on enum serialization errors #3996

pabloariasal commented Mar 25, 2023 •

edited

Loading

nlohmann Mar 25, 2023

pabloariasal Mar 26, 2023

github-actions bot commented Mar 25, 2023

coveralls commented Mar 25, 2023 •

edited

Loading

github-actions bot commented Mar 26, 2023

gregmarr Mar 27, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 27, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 27, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 28, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 28, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 27, 2023

pabloariasal Mar 28, 2023

gregmarr Mar 27, 2023 •

edited

Loading

pabloariasal Mar 28, 2023

gregmarr Mar 28, 2023

bear24rw Mar 28, 2023

pabloariasal Apr 23, 2023

bear24rw Mar 28, 2023

pabloariasal Apr 23, 2023

	The `NLOHMANN_JSON_SERIALIZE_ENUM` and `NLOHMANN_JSON_SERIALIZE_ENUM_STRICT` macros allow to define a user-defined serialization for every enumerator.
	The `NLOHMANN_JSON_SERIALIZE_ENUM` and `NLOHMANN_JSON_SERIALIZE_ENUM_STRICT` macros allow specifying a user-defined serialization for every enumerator.

alternative macro for throwing on enum serialization errors #3996

alternative macro for throwing on enum serialization errors #3996

Conversation

pabloariasal commented Mar 25, 2023 • edited Loading

Pull request checklist

Please don't

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2023

🔴 Amalgamation check failed! 🔴

coveralls commented Mar 25, 2023 • edited Loading

github-actions bot commented Mar 26, 2023

🔴 Amalgamation check failed! 🔴

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

gregmarr Mar 27, 2023 • edited Loading

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

pabloariasal commented Mar 25, 2023 •

edited

Loading

coveralls commented Mar 25, 2023 •

edited

Loading

gregmarr Mar 27, 2023 •

edited

Loading