Repository navigation
New macros for the named JSON convertor generation - #4563
Conversation
the correct maximum number of member variables is 63
|
I see several PRs that can potentially conflict with this one (#4528, #4560), and I'd rather not do extra work in maintaining this patch, if it's going to be pointless anyway. @nlohmann can you at least tell what are the chances of accepting this PR? There is an unsuccessful check in the static code analysis, but first, it's a false positive, it says that the three fields are not used, when they most definitely are, and second, it's in example files, which shouldn't be statically analysed in the first place. Examples should be demonstrative and clear, not statically correct. |
Signed-off-by: George Sedov <radist.morse@gmail.com>
|
@nlohmann Do you think this is close enough to ready to have it in the next release? It seems like a good complement to what's in there now for these macros. |
Sure - I'm in no rush for the release. And by starting with the release notes one gets an idea about the theme of the release, and yes, the named macros make sense to be added. |
|
Can you please rebase to the latest develop branch? |
Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com> Signed-off-by: George Sedov <radist.morse@gmail.com>
nlohmann
left a comment
There was a problem hiding this comment.
Please check the DTO requirements and the issue below.
|
The sign-off requirement is not that easy to fulfil retroactively. When I opened this PR there was no such requirement, and now after several merges from develop the simple |
|
I'm fine with ignoring DCO. I'll give a final review later this week. |
|
Thanks a lot! |
* Add new macros for named conversions * Unit tests for the named conversion macros * Update the docs to include the new macros * Fix the documentation for the macros the correct maximum number of member variables is 63 * Fix CI tests * update the named macros * move the example files * update the explicit macros expansion * update documentation * fix documentation hiccups * astyle changes * add static analysis exceptions * change md header to explicit html to fit the length * Small corrections to docs Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com> Signed-off-by: George Sedov <radist.morse@gmail.com> --------- Signed-off-by: George Sedov <radist.morse@gmail.com> Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com> Signed-off-by: Vaibhav Sharma <48472541+GhostVaibhav@users.noreply.github.com>
* Add new macros for named conversions * Unit tests for the named conversion macros * Update the docs to include the new macros * Fix the documentation for the macros the correct maximum number of member variables is 63 * Fix CI tests * update the named macros * move the example files * update the explicit macros expansion * update documentation * fix documentation hiccups * astyle changes * add static analysis exceptions * change md header to explicit html to fit the length * Small corrections to docs Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com> Signed-off-by: George Sedov <radist.morse@gmail.com> --------- Signed-off-by: George Sedov <radist.morse@gmail.com> Co-authored-by: Niels Lohmann <niels.lohmann@gmail.com>
tools/macro_builder only emitted NLOHMANN_JSON_EXPAND..PASTE64, so two other tables that scale with the same max_args stayed hand-maintained with nothing checking them: NLOHMANN_JSON_DOUBLE_PASTE (added by hand in #4563 for the *_WITH_NAMES macros) and the 64-slot dispatch table of NLOHMANN_JSON_TYPE_BODY (the #4041 zero-member/one-or-more-member switch). Both tables pass one macro name per slot to the same NLOHMANN_JSON_GET_MACRO dispatch as PASTE, so they can drift out of sync with max_args exactly the way _dp did in the ABI tag list fixed by #5544. Extend main.cpp with build_double_paste_code() (same recursive-doubling shape as build_paste_code(), but DOUBLE_PASTE consumes two arguments per member, so an even slot index falls back to the next lower odd DOUBLE_PASTE<N>) and build_type_body_table() (max_args - 1 MEMBERS slots and one trailing EMPTY slot, 8 per line, matching how it is written by hand today). Add a "type_body" argument that selects the TYPE_BODY block, since it lives at a separate location in macro_scope.hpp from the EXPAND..DOUBLE_PASTE63 block; plain invocation is unchanged apart from covering the extended range. No longer emit the tool's old trailing blank line, so its output is directly diffable without post-processing. Verified with c++ -std=c++11: running the tool (with and without "type_body") and piping the raw output through the pinned astyle reproduces both blocks of the current macro_scope.hpp byte for byte. tests/src/unit-udt_macro.cpp (all NLOHMANN_DEFINE_TYPE_*/_WITH_NAMES/ zero-member variants) passes unchanged under -std=c++11 and -std=c++17 with -fsanitize=address,undefined. Add a "macro_builder_check" Makefile target that builds main.cpp, regenerates both blocks into a scratch directory inside the repository (astyle's --project lookup needs the target files under the same tree as .astylerc, unlike an external /tmp directory), and diffs them against the corresponding ranges of macro_scope.hpp; wire it into check-amalgamation next to the natvis check. Wire the same regeneration into check_amalgamation.yml, splicing the (still unindented) generated blocks back into the PR's own macro_scope.hpp before the existing astyle/amalgamation step runs, so that step's own tree-wide astyle pass both indents them and folds any drift into the amalgamation patch/diff the workflow already produces. Unlike amalgamate.py and generate_natvis.py, this step builds tools/macro_builder/main.cpp from the pull request's own checkout ($MAIN_DIR) rather than a separate checkout of tools/ at develop: this tool has no independent source of truth to regenerate against (its README documents that it must reproduce macro_scope.hpp byte for byte), so a develop-pinned copy would only reproduce the generate_natvis.py trap fixed in a previous commit on this branch, where a PR that teaches the tool to cover more of the file fails its own CI until that PR merges and updates the develop copy. Add tools/macro_builder/README.md documentation for both new tables and the two-invocation usage, and a short pointer comment above NLOHMANN_JSON_TYPE_BODY (the EXPAND pointer already covered the first block; extended its wording to include DOUBLE_PASTE63). Closes #5717 item 5 in full, completing what the documentation-only "Document tools/macro_builder..." commit already on this branch left open (that commit's README/pointer-comment half stands; it also covers item 7). Signed-off-by: Niels Lohmann <mail@nlohmann.me>
…er (#5735) * Remove dead doctest help entry and pretty_format target from Makefile The top-level Makefile still carried three leftovers: - The help text listed a "doctest" target that was removed in #4560, so "make doctest" fails with "No rule to make target". The example check now runs as "make check_output -C docs". - "pretty_format" ran clang-format on all sources, but .clang-format was deleted in #4573, so the target reformatted everything in the default LLVM style, against the Artistic Style formatting that "make pretty" applies and CI enforces. - "clean" removed benchmarks/files/numbers/*.json, a directory that no longer exists since the benchmarks moved to tests/benchmarks (#3462). Only maintainer tooling changes; the library is not affected. Part of #5717 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Document tools/macro_builder and tidy up serve_header.py tools/macro_builder generates the NLOHMANN_JSON_EXPAND, NLOHMANN_JSON_GET_MACRO and NLOHMANN_JSON_PASTE* macros in macro_scope.hpp, but nothing referred to it. Add a README that explains what it generates, how to run it and where the output goes, and which dependent tables (NLOHMANN_JSON_DOUBLE_PASTE, NLOHMANN_JSON_TYPE_BODY) are maintained by hand. Point to it from a comment above NLOHMANN_JSON_EXPAND. The generator itself is unchanged; following the README reproduces the header byte for byte. In serve_header.py, drop the LGTM suppression (LGTM.com shut down in 2022), replace the """.""" placeholder docstrings with real ones, and import socket and ssl at module level. DualStackServer.server_bind uses socket, which was only imported under __main__; when the module was imported instead, the NameError was swallowed and IPV6_V6ONLY was not cleared. The header change is a comment only; behavior, API and ABI are unchanged. Part of #5717 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Hash every release_files artifact, not a hardcoded subset The `release` target signed and copied json_fwd.hpp into release_files alongside json.hpp, but the shasum line that writes hashes.txt only listed json.hpp, include.zip and json.tar.xz. Users could not verify the published json_fwd.hpp against hashes.txt. Hash every file in release_files except the .asc signatures instead of naming files by hand, so a newly shipped header (such as the json_literals.hpp that #5610 adds to this target) cannot be missed again. Only affects the generated hashes.txt release artifact; the library itself is unaffected. Overlaps #5610, which touches the same lines to add json_literals.hpp to the release target. #5717 item 1 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Remove the broken fuzz_testing* Makefile targets fuzz_testing and fuzz_testing_{bon8,bson,cbor,msgpack,ubjson} seeded fuzz-testing/testcases from tests/data, which was removed in dbf1a1f (2020) when the test data moved to the external json_test_data repo. The find command found nothing, but the pipeline's exit status was that of xargs, so the recipe still reported success with an empty corpus, and the printed afl-fuzz command would refuse to start. The recipes were also six near-identical copies with unquoted -name patterns, used the legacy CXX=afl-clang++, and fuzzing-start/stop were missing from both the help output and .PHONY. tests/fuzzing.md already documents the working flow (download json_test_data, then `make -C tests fuzzers`), so replace the six broken targets and their help lines with a single pointer to that document instead of trying to keep six copies of a fragile shell pipeline in sync. This does not affect OSS-Fuzz, which builds through tests/Makefile. Overlaps #5621, which adds a seventh copy of the same broken line for fuzz_testing_json_view. #5717 item 2 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Remove stale Travis comment above check-amalgamation check-amalgamation carried "Note: this target is called by Travis", left over from before the project switched off Travis CI. The prior Makefile cleanup commit removed the other stale Travis-era leftovers (the doctest help entry, pretty_format, and the benchmarks/ path in clean) but missed this comment. #5717 item 4 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix stale install/usage instructions in the vendored amalgamate README tools/amalgamate/README.md is the unmodified upstream text and no longer matches how the tool is used here: - It named a Bitbucket origin that no longer exists; CHANGES.md already tracks the GitHub mirror commit this copy is based on. - It asked for Python 2.7, but CI and the Makefile run the script with python3. - It told readers to run ./test.sh (not vendored) and install to /usr/local/bin; in this repository the tool runs through `make amalgamate`. - Its usage synopsis showed `-v` taking no argument, but the script's own argparser requires `choices=["yes", "no"]`, so that form fails with "argument -v/--verbose: expected one argument". The Makefile calls it as `--verbose=yes`. - It pointed at test/source.c.json and test/include.h.json, which are not vendored; the configs actually used are config_json.json and config_json_fwd.json. Rewrote only the Installing and Using sections to match; left the "Here be dragons" caveats and the rest of the vendored code untouched to avoid diverging further from upstream. Overlaps #5615, which edits amalgamate.py, this README and CHANGES.md. #5717 item 6 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Derive generate_natvis.py's ABI tag list and version from abi_macros.hpp generate_natvis.py hard-coded abi_tags = ['_diag', '_ldvcmp', '_dp', '_bics', '_psp', '_snul'] and required --version on the command line. The source of truth is include/nlohmann/detail/abi_macros.hpp: the NLOHMANN_JSON_ABI_TAG_* defines, the argument order of NLOHMANN_JSON_ABI_TAGS_CONCAT, and NLOHMANN_JSON_VERSION_MAJOR/MINOR/ PATCH. Nothing checked that the copies stayed in sync, and they have drifted apart before: _dp was added in #4517 but missed here until #5544, and 3.11.3 shipped json_abi_v3_11_2 namespaces (#4340). Parse the tag list (in NLOHMANN_JSON_ABI_TAGS_CONCAT order) and the version from abi_macros.hpp instead of hard-coding them. Make --version optional (falling back to the parsed version) and default the output directory to the repository root the script lives in. Add a "natvis" Makefile target that runs the script, and extend check-amalgamation to regenerate nlohmann_json.natvis and fail on a diff, the same way it already does for the amalgamated headers and BUILD.bazel. Wire the same regeneration into check_amalgamation.yml, using the tool copy checked out from develop (as the workflow already does for amalgamate.py) and installing jinja2 from tools/generate_natvis/requirements.txt. Update the tool's README to say it must be re-run after adding an ABI tag or bumping the version. Verified: a run against develop produces no diff (with either the default or an explicit --version 3.12.0); adding a dummy NLOHMANN_JSON_ABI_TAG_* without a matching #define makes the script fail loudly instead of silently omitting the tag; xmllint --noout passes on the regenerated file; and running the script from a directory other than the one being checked (simulating the workflow's separate tool checkout) against this repository root also produces no diff. Overlaps #5600, which added _ekmo to the same hand-written abi_tags line and regenerated the file. #5717 item 3 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Strip the leading "./" find(1) prefix from release hashes.txt entries bc6e7db (#5717 item 1) switched the release target's shasum line from naming files by hand to $$(find . -type f -not -name '*.asc' | sort), so a newly shipped header is hashed automatically. Run from inside release_files, that find prints paths as "./json.hpp" instead of "json.hpp", so hashes.txt lists "./json.hpp" etc. instead of the plain filenames it always used. shasum -c still verifies "./json.hpp" fine, but it is a needless cosmetic regression for anyone reading the file or matching it against release notes. Strip the "./" prefix with sed before sorting, keeping the filenames exactly as before while still hashing every artifact automatically. Review fix for #5717 item 1 (PR #5735). Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix check_amalgamation.yml: pass --version to generate_natvis.py bff45f1 (#5717 item 3) made --version optional in tools/generate_natvis/generate_natvis.py and wired the workflow's new "Regenerate nlohmann_json.natvis" step to call it without --version, relying on the script deriving the version from abi_macros.hpp itself. But NATVIS_TOOL_DIR is checked out from develop, the same way TOOL_DIR already is for amalgamate.py, precisely so an in-flight PR's tooling changes cannot mark themselves clean. Until this PR (or an equivalent) merges to develop, that checkout is the old generate_natvis.py, whose --version argument is still required=True. The new step's invocation of "generate_natvis.py $MAIN_DIR" (no --version) then fails argparse on this PR's own CI run with "the following arguments are required: --version", before the check ever gets to compare output. Extract the version from $MAIN_DIR's own abi_macros.hpp in the workflow and always pass it as --version. That satisfies the old script's required argument and is accepted as an explicit override by the new one, so the step behaves the same whether NATVIS_TOOL_DIR holds the pre- or post-merge tool, and stays correct for later PRs that bump the version. Verified by running the workflow step's shell logic locally against both the pre-#5717 generate_natvis.py (checked out at 633de8e) and the new one: both produce the identical nlohmann_json.natvis as the committed file. Review fix for #5717 item 3 (PR #5735). Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Extend tools/macro_builder to also generate DOUBLE_PASTE and TYPE_BODY tools/macro_builder only emitted NLOHMANN_JSON_EXPAND..PASTE64, so two other tables that scale with the same max_args stayed hand-maintained with nothing checking them: NLOHMANN_JSON_DOUBLE_PASTE (added by hand in #4563 for the *_WITH_NAMES macros) and the 64-slot dispatch table of NLOHMANN_JSON_TYPE_BODY (the #4041 zero-member/one-or-more-member switch). Both tables pass one macro name per slot to the same NLOHMANN_JSON_GET_MACRO dispatch as PASTE, so they can drift out of sync with max_args exactly the way _dp did in the ABI tag list fixed by #5544. Extend main.cpp with build_double_paste_code() (same recursive-doubling shape as build_paste_code(), but DOUBLE_PASTE consumes two arguments per member, so an even slot index falls back to the next lower odd DOUBLE_PASTE<N>) and build_type_body_table() (max_args - 1 MEMBERS slots and one trailing EMPTY slot, 8 per line, matching how it is written by hand today). Add a "type_body" argument that selects the TYPE_BODY block, since it lives at a separate location in macro_scope.hpp from the EXPAND..DOUBLE_PASTE63 block; plain invocation is unchanged apart from covering the extended range. No longer emit the tool's old trailing blank line, so its output is directly diffable without post-processing. Verified with c++ -std=c++11: running the tool (with and without "type_body") and piping the raw output through the pinned astyle reproduces both blocks of the current macro_scope.hpp byte for byte. tests/src/unit-udt_macro.cpp (all NLOHMANN_DEFINE_TYPE_*/_WITH_NAMES/ zero-member variants) passes unchanged under -std=c++11 and -std=c++17 with -fsanitize=address,undefined. Add a "macro_builder_check" Makefile target that builds main.cpp, regenerates both blocks into a scratch directory inside the repository (astyle's --project lookup needs the target files under the same tree as .astylerc, unlike an external /tmp directory), and diffs them against the corresponding ranges of macro_scope.hpp; wire it into check-amalgamation next to the natvis check. Wire the same regeneration into check_amalgamation.yml, splicing the (still unindented) generated blocks back into the PR's own macro_scope.hpp before the existing astyle/amalgamation step runs, so that step's own tree-wide astyle pass both indents them and folds any drift into the amalgamation patch/diff the workflow already produces. Unlike amalgamate.py and generate_natvis.py, this step builds tools/macro_builder/main.cpp from the pull request's own checkout ($MAIN_DIR) rather than a separate checkout of tools/ at develop: this tool has no independent source of truth to regenerate against (its README documents that it must reproduce macro_scope.hpp byte for byte), so a develop-pinned copy would only reproduce the generate_natvis.py trap fixed in a previous commit on this branch, where a PR that teaches the tool to cover more of the file fails its own CI until that PR merges and updates the develop copy. Add tools/macro_builder/README.md documentation for both new tables and the two-invocation usage, and a short pointer comment above NLOHMANN_JSON_TYPE_BODY (the EXPAND pointer already covered the first block; extended its wording to include DOUBLE_PASTE63). Closes #5717 item 5 in full, completing what the documentation-only "Document tools/macro_builder..." commit already on this branch left open (that commit's README/pointer-comment half stands; it also covers item 7). Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This patch adds several new
NLOHMANN_DEFINE_<***>_WITH_NAMESmacros. They behave the same way as the ones withoutWITH_NAMES, but require explicit JSON names. Useful for the situation when the fields in the class are following the naming convention that you do not want to expose to JSON, e.g.or if the name in JSON cannot be used as the field name because it is reserved, e.g.
Also, this patch includes the unit tests for the new macros, and the update for the documentation.
It also fixes a small error in the docs where the
DEFINE_TYPEmacros were said to support up to 64 member variables, when in reality it is 63.This is the update to the #4092 which didn't pass the amalgamation test because I had an old version of astyle. I updated the patch to include the new
DERIVEDvariations as well asONLY_SERIALIZE. Hopefully this one will be better.Pull request checklist
Read the Contribution Guidelines for detailed information.
include/nlohmanndirectory, runmake amalgamateto create the single-header filessingle_include/nlohmann/json.hppandsingle_include/nlohmann/json_fwd.hpp. The whole process is described here.