Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON_BuildTests fail when JSON_DisableEnumSerialization is set to ON #4384

Closed
1 of 2 tasks
madamsrc opened this issue May 29, 2024 · 3 comments · Fixed by #4504
Closed
1 of 2 tasks

JSON_BuildTests fail when JSON_DisableEnumSerialization is set to ON #4384

madamsrc opened this issue May 29, 2024 · 3 comments · Fixed by #4504
Assignees
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation

Comments

@madamsrc
Copy link

Description

Setting JSON_DisableEnumSerialization to ON when building JSON for Modern C++ with testing enabled (JSON_BuildTests set also set to ON), the tests do not build and the build fails. Tests that rely on that setting should not be built for library clients who do not want EnumSerialization and the build should succeed.

Reproduction steps

From a fresh clone of nlohmann/json, I ran the following commands to configure and build the library using CMake:

/usr/bin/cmake -DJSON_DisableEnumSerialization=ON -DJSON_BuildTests=On -DJSON_Install=ON -DJSON_SystemInclude=ON -DCMAKE_INSTALL_PREFIX=/workspaces/json/install/gcc12-release -DCMAKE_EXPORT_COMPILE_COMMANDS=true -DCMAKE_BUILD_TYPE=Release -DCMAKE_CXX_COMPILER:FILEPATH=/usr/bin/g++-12 -S/workspaces/json -B/workspaces/json/build/gcc12-release -DCMAKE_BUILD_TYPE:STRING=Release -DCMAKE_C_COMPILER:FILEPATH=/usr/bin/gcc-12 -DCMAKE_CXX_COMPILER:FILEPATH=/usr/bin/g++-12 -DCMAKE_CXX_STANDARD:STRING=20 -DCMAKE_CXX_STANDARD_REQUIRED:BOOL=ON -DCMAKE_CXX_EXTENSIONS:BOOL=OFF -DCMAKE_CXX_FLAGS_INIT:STRING='-fPIC -O3 -mavx2 -mavx'
/usr/bin/cmake --build /workspaces/json/build/gcc12-release --parallel 17 --target all

Expected vs. actual results

I expect successful building and linking of all executable tests with no errors or warnings. In practice, the compiler produces errors and warnings as detailed below in the Error messages field in this form.

Minimal code example

N/A - just building the tests of the library

Error messages

/workspaces/json/tests/src/unit-conversions.cpp:1272:53: error: expected primary-expression before ‘)’ token
 1272 |         CHECK(json(cpp_enum::value_1).get<cpp_enum>() == cpp_enum::value_1);
      |                                                     ^
/workspaces/json/tests/thirdparty/doctest/doctest.h:2427:24: note: in definition of macro ‘DOCTEST_ASSERT_IMPLEMENT_1’
 2427 |                     << __VA_ARGS__) DOCTEST_CLANG_SUPPRESS_WARNING_POP
      |                        ^~~~~~~~~~~
/workspaces/json/tests/thirdparty/doctest/doctest.h:2969:20: note: in expansion of macro ‘DOCTEST_CHECK’
 2969 | #define CHECK(...) DOCTEST_CHECK(__VA_ARGS__)
      |                    ^~~~~~~~~~~~~
/workspaces/json/tests/src/unit-conversions.cpp:1272:9: note: in expansion of macro ‘CHECK’
 1272 |         CHECK(json(cpp_enum::value_1).get<cpp_enum>() == cpp_enum::value_1);
      |         ^~~~~
gmake[2]: *** [tests/CMakeFiles/test-conversions_cpp11.dir/build.make:76: tests/CMakeFiles/test-conversions_cpp11.dir/src/unit-conversions.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1763: tests/CMakeFiles/test-conversions_cpp11.dir/all] Error 2
gmake: *** [Makefile:146: all] Error

Compiler and operating system

gcc12, Ubuntu 22.04.1 x86_64

Library version

3.11.2

Validation

@nlohmann
Copy link
Owner

That's a very specific use case, but the error should be easily fixable. I will have a look.

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Nov 20, 2024
@nlohmann nlohmann added this to the Release 3.11.4 milestone Nov 20, 2024
@nlohmann
Copy link
Owner

I propagated the JSON_DisableEnumSerialization flag to the test cases to exclude those that rely on enum conversion. Please take a look @madamsrc @EfesX

@madamsrc
Copy link
Author

madamsrc commented Dec 3, 2024

I propagated the JSON_DisableEnumSerialization flag to the test cases to exclude those that rely on enum conversion. Please take a look @madamsrc @EfesX

Thanks @nlohmann - this fix did the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants