Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2532

YarikTH · 2020-12-14T00:56:28Z

As I described in #2528 (comment)
Ideally, NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE should work with any basic_json overload.
Like here:
https://godbolt.org/z/d16erK

In this pull request, I added my templated macroses as two new macroses with ugly names
NLOHMANN_DEFINE_TYPE_INTRUSIVE_2
NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_2
I'm not sure if their content should just replace NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE or they should be separated, but have a better name (suggestions are welcome!).

Also any way some tests should be written for them, but I don't know where. Maybe in unit-udt_macro.cpp? I have to include ordered_json to it in this case.

TODO:

Add tests for new macroses in unit-udt_macro.cpp
Choise better names for macroses
Constrain JsonType with enable_if? Don't need. Existing NLOHMANN_JSON_SERIALIZE_ENUM don't constrain JsonType. If it need, it should be done for all such cases in separate PR

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.

…_TYPE_NON_INTRUSIVE_2 added to make work with arbitrary basic_json overloads easier.

coveralls · 2020-12-14T01:29:45Z

Coverage remained the same at 100.0% when pulling c2d50d6 on YarikTH:feature/templated_define_type_macroses into a8398a7 on nlohmann:develop.

include/nlohmann/detail/json_ref.hpp

single_include/nlohmann/json.hpp

nlohmann · 2020-12-14T09:43:41Z

Tests should to to test/src/unit-udt_macro.cpp.

…define_type_macroses

nlohmann · 2020-12-14T09:45:16Z

Thanks for the proposal, I like it! In fact, the introduction of ordered_json was quite rough and we forgot about it in many places.

I'm hesitant on replacing the existing macros, because this may break existing code, wouldn't it?

YarikTH · 2020-12-14T09:51:07Z

I'm hesitant on replacing the existing macros, because this may break existing code, wouldn't it?

Theoretically, it shouldn't I just added template parametrisation of json type, but who knows. There are only a few tests that check this macro. But problems may occur in some real-life usage that is hard to predict.

BTW, it's better to constrain template type somehow. enable_if<is_basic_json>?

YarikTH · 2020-12-14T09:55:18Z

I added TODO section in PR description

…_DEFINE_TYPE_NON_INTRUSIVE_2 updated to match NLOHMANN_JSON_SERIALIZE_ENUM

YarikTH · 2020-12-14T10:15:00Z

In fact, the introduction of ordered_json was quite rough and we forgot about it in many places.

Speaking about this. Actually, it's quite easy to find all the problematic places. Just search "nlohmann::json" with full words option.
So what I found:

/*!
@brief macro
@def NLOHMANN_DEFINE_TYPE_INTRUSIVE
@since version 3.9.0
*/
#define NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type, ...)  \
    friend void to_json(nlohmann::json& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    friend void from_json(const nlohmann::json& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

/*!
@brief macro
@def NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE
@since version 3.9.0
*/
#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(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, __VA_ARGS__)) }

/*!
@brief SAX interface

This class describes the SAX interface used by @ref nlohmann::json::sax_parse.
Each function is called in different situations while the input is parsed. The
boolean return value informs the parser whether to continue processing the
input.
*/
template<typename BasicJsonType>
struct json_sax
{

/// hash value for JSON objects
template<>
struct hash<nlohmann::json>
{
    /*!
    @brief return a hash value for a JSON object

    @since version 1.0.0
    */
    std::size_t operator()(const nlohmann::json& j) const
    {
        return nlohmann::detail::hash(j);
    }
};

/*!
@brief exchanges the values of two JSON objects

@since version 1.0.0
*/
template<>
inline void swap<nlohmann::json>(nlohmann::json& j1, nlohmann::json& j2) noexcept(
    is_nothrow_move_constructible<nlohmann::json>::value&&
    is_nothrow_move_assignable<nlohmann::json>::value
                              )
{
    j1.swap(j2);
}

/*!
@brief user-defined string literal for JSON values

This operator implements a user-defined string literal for JSON objects. It
can be used by adding `"_json"` to a string literal and returns a JSON object
if no parse error occurred.

@param[in] s  a string representation of a JSON object
@param[in] n  the length of string @a s
@return a JSON object

@since version 1.0.0
*/
JSON_HEDLEY_NON_NULL(1)
inline nlohmann::json operator "" _json(const char* s, std::size_t n)
{
    return nlohmann::json::parse(s, s + n);
}

/*!
@brief user-defined string literal for JSON pointer

This operator implements a user-defined string literal for JSON Pointers. It
can be used by adding `"_json_pointer"` to a string literal and returns a JSON pointer
object if no parse error occurred.

@param[in] s  a string representation of a JSON Pointer
@param[in] n  the length of string @a s
@return a JSON pointer object

@since version 2.0.0
*/
JSON_HEDLEY_NON_NULL(1)
inline nlohmann::json::json_pointer operator "" _json_pointer(const char* s, std::size_t n)
{
    return nlohmann::json::json_pointer(std::string(s, n));
}

It seems more like too strict glueing to nlohmann::json despite the fact that it's just default specialization, not only one.
But it's subject to another PR anyway. Or it isn't? I can convert PR to more general one, but I can't think of the proper name for it.

YarikTH · 2020-12-14T10:55:30Z

Some possible improvements for NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE. I'm not sure should it be done in this PR or not.

/*!
@brief macro to briefly define intrusive serialization of a given type to/from JSON
@note you can define your own specialized macroses like NLOHMANN_DEFINE_TYPE_INTRUSIVE
@def NLOHMANN_DEFINE_TYPE_INTRUSIVE_IMPL
@since version 3.9.2
*/
#define NLOHMANN_DEFINE_TYPE_INTRUSIVE_IMPL(BasicJsonType, Type, ...)  \
    friend void to_json(BasicJsonType& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    friend void from_json(const BasicJsonType& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

/*!
@brief macro to briefly define non-intrusive serialization of a given type to/from JSON
@note you can define your own specialized macroses like NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE
@def NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_IMPL
@since version 3.9.2
*/
#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_IMPL(BasicJsonType, Type, ...)  \
    inline void to_json(BasicJsonType& nlohmann_json_j, const Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_TO, __VA_ARGS__)) } \
    inline void from_json(const BasicJsonType& nlohmann_json_j, Type& nlohmann_json_t) { NLOHMANN_JSON_EXPAND(NLOHMANN_JSON_PASTE(NLOHMANN_JSON_FROM, __VA_ARGS__)) }

/*!
@brief macro to briefly define intrusive serialization of given type to/from nlohmann::json
@def NLOHMANN_DEFINE_TYPE_INTRUSIVE
@since version 3.9.0
@sa NLOHMANN_DEFINE_TYPE_INTRUSIVE_IMPL
*/
#define NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type, ...) NLOHMANN_DEFINE_TYPE_INTRUSIVE_IMPL(nlohmann::json, Type, __VA_ARGS__)

/*!
@brief macro to briefly define non-intrusive serialization of given type to/from nlohmann::json
@def NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE
@since version 3.9.0
@sa NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_IMPL
*/
#define NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(Type, ...) NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_IMPL(nlohmann::json, Type, __VA_ARGS__)

…_TYPE_NON_INTRUSIVE_2 are renamed to *_T. Also doxygen docs for them are improved

nlohmann · 2020-12-14T13:03:34Z

Thanks for checking - the other usages of nlohmann::json seem straightforward to fix.

…ce code bloat in unit-udt_macro.cpp

…RUSIVE_T after "Template declaration of NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_2 updated to match NLOHMANN_JSON_SERIALIZE_ENUM"

…ON_INTRUSIVE_T are added in unit-udt_macro.cpp. Needed to patch doctest.h to workaround doctest/doctest#447

…USIVE_IMPL are added. NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE are updated to work via XXX_IMPL verions. Also doxygen documentation is improved

YarikTH · 2020-12-14T16:29:52Z

Tests are added. PR is ready to review.

test/thirdparty/doctest/doctest.h

…define_type_macroses

…define_type_macroses # Conflicts: # test/src/unit-udt_macro.cpp

MHebes · 2021-03-09T16:11:19Z

Are there any possible use cases where just making the original macros templated would break backwards compat?

I personally think it'd be a lot cleaner if the macros were changed rather than making new ones that do the same thing. Although, I'm sure you agree and are just erring on the side of caution 🙂

Just not sure if the caution is needed necessarily.

YarikTH · 2021-03-10T06:45:02Z

Are there any possible use cases where just making the original macros templated would break backwards compat?

I personally think it'd be a lot cleaner if the macros were changed rather than making new ones that do the same thing. Although, I'm sure you agree and are just erring on the side of caution

Just not sure if the caution is needed necessarily.

There are a lot of users with different compilers. Who knows what can go wrong. A separate macro at least minimizes the chance of problems. But of course, I can just replace the content of the existing macro with my templated version, if the owner says so. Unfortunately, 3 months passed and there is still zero feedback from nlohmann about the final version.

nlohmann · 2021-04-29T15:39:30Z

@YarikTH Sorry for leaving this open for so long... Can you please update to the latest develop version. I think the only remaining todos are documentation.

YarikTH · 2021-05-11T06:26:29Z

I'm no longer interested in this PR. If someone wants to continue it, you are welcome.

Two new macroses NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN_DEFINE…

a0b170b

…_TYPE_NON_INTRUSIVE_2 added to make work with arbitrary basic_json overloads easier.

YarikTH requested a review from nlohmann as a code owner December 14, 2020 00:56

YarikTH changed the title ~~Two new macroses NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN_DEFINE…~~ Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE Dec 14, 2020

YarikTH commented Dec 14, 2020

View reviewed changes

include/nlohmann/detail/json_ref.hpp Outdated Show resolved Hide resolved

YarikTH commented Dec 14, 2020

View reviewed changes

single_include/nlohmann/json.hpp Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/develop' into feature/templated_…

95118c4

…define_type_macroses

Template declaration of NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN…

985d3ea

…_DEFINE_TYPE_NON_INTRUSIVE_2 updated to match NLOHMANN_JSON_SERIALIZE_ENUM

Two new macroses NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN_DEFINE…

8d7c38e

…_TYPE_NON_INTRUSIVE_2 are renamed to *_T. Also doxygen docs for them are improved

YarikTH added 4 commits December 14, 2020 19:08

CREATE_PERSON and CREATE_PERSON_WITH_ALPHABET macro are added to redu…

d58aed7

…ce code bloat in unit-udt_macro.cpp

Fix NLOHMANN_DEFINE_TYPE_INTRUSIVE_T and NLOHMANN_DEFINE_TYPE_NON_INT…

a9639c9

…RUSIVE_T after "Template declaration of NLOHMANN_DEFINE_TYPE_INTRUSIVE_2 and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_2 updated to match NLOHMANN_JSON_SERIALIZE_ENUM"

Tests for NLOHMANN_DEFINE_TYPE_INTRUSIVE_T and NLOHMANN_DEFINE_TYPE_N…

4f25c5c

…ON_INTRUSIVE_T are added in unit-udt_macro.cpp. Needed to patch doctest.h to workaround doctest/doctest#447

NLOHMANN_DEFINE_TYPE_INTRUSIVE_IMPL and NLOHMANN_DEFINE_TYPE_NON_INTR…

2519721

…USIVE_IMPL are added. NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE are updated to work via XXX_IMPL verions. Also doxygen documentation is improved

YarikTH mentioned this pull request Dec 18, 2020

Doctest is updated to v2.4.3 #2538

Merged

3 tasks

YarikTH commented Dec 18, 2020

View reviewed changes

test/thirdparty/doctest/doctest.h Outdated Show resolved Hide resolved

YarikTH added 2 commits December 19, 2020 00:12

Merge remote-tracking branch 'origin/develop' into feature/templated_…

ee88c8c

…define_type_macroses

Merge remote-tracking branch 'origin/develop' into feature/templated_…

c2d50d6

…define_type_macroses # Conflicts: # test/src/unit-udt_macro.cpp

YarikTH closed this May 11, 2021

kunaltyagi mentioned this pull request Jun 26, 2021

Add templated version of DEFINE_TYPE macros #2843

Closed

4 tasks

gregmarr mentioned this pull request Dec 6, 2024

Allow ordered_json in NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE macro #4528

Closed

kimci86 mentioned this pull request Jan 13, 2025

Generate template functions with NLOHMANN_DEFINE_TYPE macros #4597

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2532

Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2532

Uh oh!

YarikTH commented Dec 14, 2020 •

edited

Loading

Uh oh!

coveralls commented Dec 14, 2020 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020 •

edited

Loading

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020 •

edited

Loading

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

Uh oh!

MHebes commented Mar 9, 2021

Uh oh!

YarikTH commented Mar 10, 2021

Uh oh!

nlohmann commented Apr 29, 2021

Uh oh!

YarikTH commented May 11, 2021

Uh oh!

Uh oh!

Uh oh!

Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2532

Templated versions of NLOHMANN_DEFINE_TYPE_INTRUSIVE and NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE #2532

Uh oh!

Conversation

YarikTH commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Uh oh!

coveralls commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

nlohmann commented Dec 14, 2020

Uh oh!

YarikTH commented Dec 14, 2020

Uh oh!

Uh oh!

MHebes commented Mar 9, 2021

Uh oh!

YarikTH commented Mar 10, 2021

Uh oh!

nlohmann commented Apr 29, 2021

Uh oh!

YarikTH commented May 11, 2021

Uh oh!

Uh oh!

YarikTH commented Dec 14, 2020 •

edited

Loading

coveralls commented Dec 14, 2020 •

edited

Loading

YarikTH commented Dec 14, 2020 •

edited

Loading

YarikTH commented Dec 14, 2020 •

edited

Loading