Repository navigation
Support zero-member types in NLOHMANN_DEFINE_TYPE_* macros (#4041) - #5272
Conversation
NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type) and its 11 sibling macros produced broken code for types with no members to serialize. Invoking a variadic macro so __VA_ARGS__ is empty is only standard-conforming since C++20, so a plain __VA_OPT__ fix (as tried in #5142) breaks every pre-C++20 build under -pedantic. Instead, make all 12 macros purely variadic and dispatch on argument count using a sentinel-padded extension of the existing NLOHMANN_JSON_GET_MACRO idiom, giving full C++11-C++26 support with no feature-test gate. Verified against real GCC 16 and Clang at -std=c++11/14/17/20 with -pedantic -Werror -Wvariadic-macros: zero regressions in the existing unit-udt_macro.cpp suite plus 12 new zero-member test cases. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Three issues surfaced on PR #5272's real CI that weren't caught by local testing against a narrower flag set: - GCC -Werror=noexcept: the four truly-empty from_json bodies (plain INTRUSIVE/NON_INTRUSIVE, with and without _WITH_DEFAULT) provably never throw but weren't declared noexcept; mark them noexcept explicitly. to_json and the derived-type from_json overloads are left alone since they genuinely can throw (object assignment / delegating to the base class's from_json). - clang-tidy bugprone-macro-parentheses: false positive on the same 8 zero-member bodies (Type/BaseType used purely as declarator types); suppressed with NOLINTNEXTLINE comments in the same style already used elsewhere in this file (see NLOHMANN_JSON_SERIALIZE_ENUM). - MSVC's traditional preprocessor doesn't fully expand NLOHMANN_JSON_CAT(prefix, NLOHMANN_JSON_TYPE_TAG(...))(...) in one pass, which broke a pre-existing one-member usage in unit-regression2.cpp with syntax errors. Wrap all 12 public dispatcher macros in an extra outer NLOHMANN_JSON_EXPAND(...), matching the pattern NLOHMANN_JSON_PASTE already uses for the same MSVC quirk. Re-verified against real GCC 16 and Clang at -std=c++11/14/17/20 with -pedantic -Werror -Wvariadic-macros -Wnoexcept, including the exact files that failed in CI (unit-udt_macro.cpp, unit-regression2.cpp), against both the modular headers and the re-amalgamated single header. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The four zero-member ONLY_SERIALIZE test objects are only ever read (via to_json), never mutated, so mark them const per clang-tidy. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
3f8c58a to
4dcd45b
Compare
|
This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions! |
nlohmann
left a comment
There was a problem hiding this comment.
Caution
Blocking — not ready to merge as-is. GitHub does not allow a formal Request changes review on your own PR, so this is filed as a review comment; treat it as blocking.
The approach is sound — pure argument-count dispatch reusing the existing NLOHMANN_JSON_GET_MACRO idiom is the right call, and the reasoning for rejecting __VA_OPT__ and ##__VA_ARGS__ holds up. Zero-member coverage is thorough (all 12 macros × json/ordered_json), the amalgamation is in sync, and the new NLOHMANN_JSON_CAT helpers don't collide with anything.
One blocking issue though: NLOHMANN_JSON_DERIVED_TYPE_TAG silently reduces the maximum member count for all six NLOHMANN_DEFINE_DERIVED_TYPE_* macros from 63 to 62, which is a public API break that also contradicts the shipped docs. Details and a tested fix inline.
Two smaller points, non-blocking:
- Zero-member
from_jsonaccepts any JSON value. It is a no-op, sojson(42).get<Zero>(),"str",[1,2]andnullall succeed, whereas the 1+-member version throwstype_error.304on a non-object. Defensible — there is nothing to read — but worth stating explicitly in the docs rather than leaving implicit. - API docs not updated.
docs/mkdocs/docs/features/arbitrary_types.mdgot the new note, but the threedocs/mkdocs/docs/api/macros/*.mdpages, where the parameter contract is actually specified, say nothing about zero members.
This review was written by Claude Code.
NLOHMANN_JSON_GET_MACRO resolves 64 positional arguments, with NAME at position 65. NLOHMANN_JSON_TYPE_TAG dispatches on Type plus the member list, so it resolves correctly up to the 63 members NLOHMANN_JSON_PASTE supports. NLOHMANN_JSON_DERIVED_TYPE_TAG dispatched on the two-token Type,BaseType prefix plus the member list, running out one slot early: at 63 members, position 65 landed on the last member name instead of a sentinel and NLOHMANN_JSON_CAT built an undefined identifier such as NLOHMANN_JSON_DEFINE_DERIVED_TYPE_INTRUSIVE_m63, with the compiler reporting "unknown type name 'm1'" once per member and nothing pointing at an argument-count limit. That silently reduced all six NLOHMANN_DEFINE_DERIVED_TYPE_* macros from 63 members to 62, contradicting the "up to 63 members" contract in docs/mkdocs/docs/api/macros/nlohmann_define_derived_type.md. Drop the leading Type and defer to NLOHMANN_JSON_TYPE_TAG so the tag is computed from BaseType plus the member list, which fits the available slots. The zero-own-member derived bodies are therefore selected by tag 1 rather than 2, and the sentinel table for the derived tag is no longer needed. Add a regression test at the documented maximum for both the plain and the derived macros; it fails to compile against the previous dispatch. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The dispatch tag was the literal token 1 or N, pasted onto a macro prefix to select the zero-member or member-carrying body. For the derived-type macros that reads wrong: their tag is computed after dropping the leading Type, so the zero-member body was named _1 while taking two parameters (Type, BaseType). Emit EMPTY and MEMBERS instead. The mechanism is unchanged -- the tag is still a token pasted onto the prefix by NLOHMANN_JSON_CAT -- but the body names now say what they are rather than encoding an argument count that only lines up for half of the macros. Collapse the four duplicated zero-member bodies while here: with no members there is nothing to default, so each _WITH_DEFAULT_EMPTY body was a byte-for-byte copy of its plain counterpart. They are now one-line aliases, leaving a single definition of what an empty object serializes to per intrusive/non-intrusive and base/derived combination. No functional change: for both zero-member and member-carrying types the preprocessed to_json/from_json output is token-for-token identical, and the arity limits are unchanged (63 members, base and derived). Signed-off-by: Niels Lohmann <mail@nlohmann.me>
docs/mkdocs/docs/features/arbitrary_types.md already gained a note, but the three api/macros pages are where the parameter contract is actually specified and they still described member as a non-empty list. State that the list may be empty on each page, and add a note showing what the zero-member case generates: an empty JSON object for the plain macros, and base-type-only serialization for the derived ones. Both notes record that the WITH_NAMES variants do not support this. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
|
This pull request has been marked as stale because it has had no activity for 30 days. While we won’t close it automatically, we encourage you to update or comment if it is still relevant. Keeping pull requests active and up-to-date helps us review and merge changes more efficiently. Thank you for your contributions! |
The dispatch produced the bare token EMPTY or MEMBERS and pasted it onto the macro prefix afterwards. In between, the token was rescanned, so a user macro with either name replaced it: with `#define MEMBERS x` in scope, even NLOHMANN_DEFINE_TYPE_INTRUSIVE(A, member) -- which compiled before -- expanded to garbage, and `#define EMPTY` broke the zero-member form. Paste the suffix onto the prefix directly in the GET_MACRO slot table instead. Operands of ## are not macro-expanded, so the selected body name is formed before any user macro can interfere. NLOHMANN_JSON_TYPE_TAG and NLOHMANN_JSON_DERIVED_TYPE_TAG become NLOHMANN_JSON_TYPE_BODY and NLOHMANN_JSON_DERIVED_TYPE_BODY, taking the prefix as their first argument; NLOHMANN_JSON_CAT is no longer needed. The body macro names are unchanged, and so is the generated code. Add a regression test that defines EMPTY and MEMBERS around plain and derived types, with and without members. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Summary
Fixes #4041:
NLOHMANN_DEFINE_TYPE_INTRUSIVE(Type)and its 11 sibling macros (_WITH_DEFAULT,_ONLY_SERIALIZE,NON_INTRUSIVEequivalents, and theDERIVED_TYPEfamily) produced broken generated code for types with no member variables to serialize.Three prior PRs attempted this (#5142, #5099, #4994); all are stale and fail CI. This is a fresh implementation, not based on any of them:
__VA_OPT__approach breaks every pre-C++20 build (error C3861: '__VA_OPT__': identifier not foundon MSVC's C++11 test project) because it isn't gated behind a C++20 check.__VA_ARGS__is empty is only standard-conforming since C++20 — GCC/Clang reject it under-pedanticpre-C++20 regardless of what the macro body does (verified directly against real GCC 16 / Clang with this repo's actual-pedantic -Werror -Wvariadic-macrosflags). So a__VA_OPT__-only fix can, at best, only ever cover C++20+.##__VA_ARGS__GNU-extension attempt doesn't avoid this either (also verified directly) and only special-cases GCC.Instead, this PR makes all 12 macros purely variadic (no fixed leading parameter) and dispatches on argument count, using a sentinel-padded extension of the existing
NLOHMANN_JSON_GET_MACROpositional-dispatch idiom already used byNLOHMANN_JSON_PASTE1..64in this file.NLOHMANN_JSON_TYPE_BODY(Prefix, ...)yields the macro namePrefix##EMPTYfor a loneTypeandPrefix##MEMBERSotherwise. The suffix is pasted inside the slot table itself, so it is never macro-expanded and user macros namedEMPTYorMEMBERScannot interfere with the dispatch. The derived-type macros drop their leadingTypeand reuse the same dispatch, which keeps the dispatch insideNLOHMANN_JSON_GET_MACRO's 64 positional slots and so preserves the full 63-member limit for them too. This gives full C++11 through C++26 support with no feature-test gate at all.WITH_NAMESvariants are intentionally out of scope, matching issue #4041 and all three prior PRs.Changes
include/nlohmann/detail/macro_scope.hpp: newNLOHMANN_JSON_TYPE_BODY/NLOHMANN_JSON_DERIVED_TYPE_BODYdispatch helpers; each of the 12 macros split into a_MEMBERSbody (byte-identical to the previous public macro body) and a new_EMPTYbody, with the public macro name becoming a 1-line dispatcher. The four_WITH_DEFAULT_EMPTYbodies are one-line aliases of their plain counterparts, since with no members there is nothing to default.single_include/nlohmann/json.hpp: re-amalgamated (make amalgamate, astyle 3.4.13).tests/src/unit-udt_macro.cpp: 12 new zero-member test cases (all 6 base macro variants + all 6 derived variants, the latter using a real base class to confirm base-class delegation still works with zero own members).tests/src/unit-udt_macro.cpp: a regression test that defines user macros namedEMPTYandMEMBERSaround plain and derived types, with and without members, to make sure they don't leak into the dispatch.tests/src/unit-udt_macro.cpp: a max-arity regression test (max_members,max_members_derived) pinning the documented 63-member limit for both the plain and the derived macros.docs/mkdocs/docs/features/arbitrary_types.md: new "Zero-member types" note with an example.docs/mkdocs/docs/api/macros/nlohmann_define_type_intrusive.md,..._non_intrusive.md,nlohmann_define_derived_type.md: thememberparameter is documented as allowing an empty list, with a note on what the zero-member case generates.API compatibility
No breaking changes. The change is purely additive: argument lists that compiled before produce byte-identical generated code (verified by comparing preprocessed token streams), and the documented 63-member limit is unchanged for both the plain and the derived macros. The only new behaviour is that a previously ill-formed invocation — a macro with no member arguments — now compiles.
Test plan
unit-udt_macro.cppsuite passes unmodified — zero regressions from the macro rename/dispatch refactor; the 12_MEMBERSbodies are byte-identical to the previous public macro bodies.nlohmann::jsonandnlohmann::ordered_json) pass.g++-16andclang++at-std=c++11/14/17/20with-pedantic -Werror -Wvariadic-macros(matchingcmake/gcc_flags.cmake).make check-amalgamationclean.unit-udt.cpp) still compiles/passes to rule out macro-name collisions from the new helpers.This PR description was written by Claude Code.