nlohmann
Add proper C++20 module support
- [x] The changes are described in detail, both the what and why.
- [x] If applicable, an existing issue is referenced.
- [ ] The Code coverage remained at 100%. A test case for every new line of code.
- [x] If applicable, the documentation is updated.
- [ ] The source code is amalgamated by running
make amalgamate.
This pull request adds proper support through CMake for a module for the nlohmann-json library, which is exported by the module nlohmann.json. It also updates the existing test (created from this pull request) to use that module (the original name in that was json, however nlohmann.json is probably a more appropriate name as it is less likely to cause naming conflicts). The module must be built using NLOHMANN_JSON_BUILD_MODULES in CMake, and this is not enabled by default.
I have also gone ahead to update the website and documentation to make note of this change.
coverage: 99.19%. remained the same when pulling b2d76be1adbdbf334bd068b2a27915d361c99308 on mikomikotaishi:develop into d70e46bc65a21a8eb1ef0e3e9ba707b7b9b78378 on nlohmann:develop.
The CI test for modules does not pass but I am afraid of making changes to the CI configuration that could break something. However, compiling the library manually seems to be fine, by just using cmake -S . -B build -G Ninja and cmake --build build and having the macro enabled.
@nlohmann Hi, so I believe I have addressed all of your concerns (at least the ones you have posted up to this point). Is there anything else you need me to address, and if not, can these changes now be merged?
Please check the CI issues.
I have re-signed my commits, but the check workflow requires approval.
@nlohmann Hi, so it seems that some of the tests (specifically ci_static_analysis_clang (ci_test_clang) and ci_cmake_options (ci_test_legacycomparison)) are failing due to files that I have not modified, and I do not know how to resolve these CI issues, as I have very little experience working with GitHub's CI system. If you have any idea what is happening, could you help with resolving those issues? I have tried to resolve specifically the CI test that corresponds to the changes I made (ci_module_cpp20 (silkeh/clang:latest)), however it takes almost 50 minutes for the GitHub CI to begin those tests.
Regardless, I can confirm that the C++ modules function correctly when compiling myself. The steps I have taken are:
- Run
cmake -S . -B build -G Ninja -DNLOHMANN_JSON_BUILD_MODULES=ON - Run
cmake --build buildAnd I confirm that this works on CMake 4.0.2 on Clang 19.1.7. (I am aware that there are apparently issues compiling this library on MSVC, however as I am on Linux I have no ability to test MSVC.)
I have a fix for ci_static_analysis_clang in #4801. Once this is merged, please rebase and check Ubuntu / ci_module_cpp20 (gcc:latest) (pull_request).
Hmm, I had a look and I think I might understand what the issue was with the modules test, it appears I had to modify the test in ubuntu.yml rather than in codeql-analysis.yml. Let's see what happens
#4801 is merged. Please rebase.
Rebased and addressed issues except for modules/ being in the project root, I am not sure if you have a better location in mind
That should be all concerns addressed, also, I was thinking that potentially in the future if anything from namespace nlohmann::detail:: needed to be used by the client, an additional module nlohmann.json.detail could be created, which may or may not be exported by the main module nlohmann.json
What did you think of the idea for the future of exporting implementation contents (nlohmann::detail::) in nlohmann.json.detail?
The detail namespace is not meant to be used by client code. It is allowed to be changed or removed between releases without warning.
@nlohmann Will there be an update on the status of this any time soon?