Fix C++20/gcc-12 issues (Part 2) (#3446)

* Add C++20 3-way comparison operator and fix broken comparisons Fixes #3207. Fixes #3409. * Fix iterators to meet (more) std::ranges requirements Fixes #3130. Related discussion: #3408 * Add note about CMake standard version selection to unit tests Document how CMake chooses which C++ standard version to use when building tests. * Update documentation * CI: add legacy discarded value comparison * Fix internal linkage errors when building a module
nlohmann · May 29, 2022 · 6b97599 · 6b97599
1 parent ede6667
commit 6b97599
Show file tree

Hide file tree

Showing 35 changed files with 1,941 additions and 668 deletions.
diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml
@@ -48,7 +48,7 @@ jobs:
     container: ghcr.io/nlohmann/json-ci:v2.3.0
     strategy:
       matrix:
-        target: [ci_test_diagnostics, ci_test_noexceptions, ci_test_noimplicitconversions]
+        target: [ci_test_diagnostics, ci_test_noexceptions, ci_test_noimplicitconversions, ci_test_legacycomparison]
     steps:
       - uses: actions/checkout@v3
       - name: cmake

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -37,13 +37,14 @@ if(${MAIN_PROJECT} AND (${CMAKE_VERSION} VERSION_EQUAL 3.13 OR ${CMAKE_VERSION}
 else()
     set(JSON_BuildTests_INIT OFF)
 endif()
-option(JSON_BuildTests          "Build the unit tests when BUILD_TESTING is enabled." ${JSON_BuildTests_INIT})
-option(JSON_CI                  "Enable CI build targets." OFF)
-option(JSON_Diagnostics         "Use extended diagnostic messages." OFF)
-option(JSON_ImplicitConversions "Enable implicit conversions." ON)
-option(JSON_Install             "Install CMake targets during install step." ${MAIN_PROJECT})
-option(JSON_MultipleHeaders     "Use non-amalgamated version of the library." OFF)
-option(JSON_SystemInclude       "Include as system headers (skip for clang-tidy)." OFF)
+option(JSON_BuildTests                     "Build the unit tests when BUILD_TESTING is enabled." ${JSON_BuildTests_INIT})
+option(JSON_CI                             "Enable CI build targets." OFF)
+option(JSON_Diagnostics                    "Use extended diagnostic messages." OFF)
+option(JSON_ImplicitConversions            "Enable implicit conversions." ON)
+option(JSON_LegacyDiscardedValueComparison "Enable legacy discarded value comparison." OFF)           
+option(JSON_Install                        "Install CMake targets during install step." ${MAIN_PROJECT})
+option(JSON_MultipleHeaders                "Use non-amalgamated version of the library." OFF)
+option(JSON_SystemInclude                  "Include as system headers (skip for clang-tidy)." OFF)
 
 if (JSON_CI)
     include(ci)
@@ -77,6 +78,10 @@ if (NOT JSON_ImplicitConversions)
     message(STATUS "Implicit conversions are disabled")
 endif()
 
+if (JSON_LegacyDiscardedValueComparison)
+    message(STATUS "Legacy discarded value comparison enabled")
+endif()
+
 if (JSON_Diagnostics)
     message(STATUS "Diagnostics enabled")
 endif()
@@ -100,8 +105,9 @@ endif()
 target_compile_definitions(
     ${NLOHMANN_JSON_TARGET_NAME}
     INTERFACE
-    JSON_USE_IMPLICIT_CONVERSIONS=$<BOOL:${JSON_ImplicitConversions}>
-    JSON_DIAGNOSTICS=$<BOOL:${JSON_Diagnostics}>
+    $<$<NOT:$<BOOL:${JSON_ImplicitConversions}>>:JSON_USE_IMPLICIT_CONVERSIONS=0>
+    $<$<BOOL:${JSON_Diagnostics}>:JSON_DIAGNOSTICS=1>
+    $<$<BOOL:${JSON_LegacyDiscardedValueComparison}>:JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1>
 )
 
 target_include_directories(

diff --git a/cmake/ci.cmake b/cmake/ci.cmake
@@ -498,6 +498,20 @@ add_custom_target(ci_test_diagnostics
     COMMENT "Compile and test with improved diagnostics enabled"
 )
 
+###############################################################################
+# Enable legacy discarded value comparison.
+###############################################################################
+
+add_custom_target(ci_test_legacycomparison
+    COMMAND CXX=${CLANG_TOOL} ${CMAKE_COMMAND}
+    -DCMAKE_BUILD_TYPE=Debug -GNinja
+    -DJSON_BuildTests=ON -DJSON_MultipleHeaders=ON -DJSON_LegacyDiscardedValueComparison=ON
+    -S${PROJECT_SOURCE_DIR} -B${PROJECT_BINARY_DIR}/build_legacycomparison
+    COMMAND ${CMAKE_COMMAND} --build ${PROJECT_BINARY_DIR}/build_legacycomparison
+    COMMAND cd ${PROJECT_BINARY_DIR}/build_legacycomparison && ${CMAKE_CTEST_COMMAND} --parallel ${N} --output-on-failure
+    COMMENT "Compile and test with legacy discarded value comparison enabled"
+)
+
 ###############################################################################
 # Coverage.
 ###############################################################################
@@ -797,8 +811,9 @@ endfunction()
 ci_get_cmake(3.1.0 CMAKE_3_1_0_BINARY)
 ci_get_cmake(3.13.0 CMAKE_3_13_0_BINARY)
 
-set(JSON_CMAKE_FLAGS_3_1_0 "JSON_Install;JSON_MultipleHeaders;JSON_ImplicitConversions;JSON_Valgrind;JSON_Diagnostics;JSON_SystemInclude")
-set(JSON_CMAKE_FLAGS_3_13_0 "JSON_BuildTests")
+set(JSON_CMAKE_FLAGS_3_1_0 JSON_Diagnostics JSON_ImplicitConversions JSON_LegacyDiscardedValueComparison 
+    JSON_Install JSON_MultipleHeaders JSON_SystemInclude JSON_Valgrind)
+set(JSON_CMAKE_FLAGS_3_13_0 JSON_BuildTests)
 
 function(ci_add_cmake_flags_targets flag min_version)
     string(TOLOWER "ci_cmake_flag_${flag}" flag_target)

diff --git a/cmake/test.cmake b/cmake/test.cmake
@@ -156,6 +156,7 @@ endfunction()
 #############################################################################
 # json_test_add_test_for(
 #     <file>
+#     [NAME <name>]
 #     MAIN <main>
 #     [CXX_STANDARDS <version_number>...] [FORCE])
 #
@@ -165,22 +166,30 @@ endfunction()
 #
 # if C++ standard <version_number> is supported by the compiler and the
 # source file contains JSON_HAS_CPP_<version_number>.
+# Use NAME <name> to override the filename-derived test name.
 # Use FORCE to create the test regardless of the file containing
 # JSON_HAS_CPP_<version_number>.
 # Test targets are linked against <main>.
 # CXX_STANDARDS defaults to "11".
 #############################################################################
 
 function(json_test_add_test_for file)
-    cmake_parse_arguments(args "FORCE" "MAIN" "CXX_STANDARDS" ${ARGN})
-
-    get_filename_component(file_basename ${file} NAME_WE)
-    string(REGEX REPLACE "unit-([^$]+)" "test-\\1" test_name ${file_basename})
+    cmake_parse_arguments(args "FORCE" "MAIN;NAME" "CXX_STANDARDS" ${ARGN})
 
     if("${args_MAIN}" STREQUAL "")
         message(FATAL_ERROR "Required argument MAIN <main> missing.")
     endif()
 
+    if("${args_NAME}" STREQUAL "")
+        get_filename_component(file_basename ${file} NAME_WE)
+        string(REGEX REPLACE "unit-([^$]+)" "test-\\1" test_name ${file_basename})
+    else()
+        set(test_name ${args_NAME})
+        if(NOT test_name MATCHES "test-[^$]+")
+            message(FATAL_ERROR "Test name must start with 'test-'.")
+        endif()
+    endif()
+
     if("${args_CXX_STANDARDS}" STREQUAL "")
         set(args_CXX_STANDARDS 11)
     endif()

diff --git a/docs/mkdocs/docs/api/basic_json/index.md b/docs/mkdocs/docs/api/basic_json/index.md
@@ -233,9 +233,10 @@ Access to the JSON value
 - [**operator==**](operator_eq.md) - comparison: equal
 - [**operator!=**](operator_ne.md) - comparison: not equal
 - [**operator<**](operator_lt.md) - comparison: less than
-- [**operator<=**](operator_le.md) - comparison: less than or equal
 - [**operator>**](operator_gt.md) - comparison: greater than
+- [**operator<=**](operator_le.md) - comparison: less than or equal
 - [**operator>=**](operator_ge.md) - comparison: greater than or equal
+- [**operator<=>**](operator_spaceship.md) - comparison: 3-way
 
 ### Serialization / Dumping
 

diff --git a/docs/mkdocs/docs/api/basic_json/operator_eq.md b/docs/mkdocs/docs/api/basic_json/operator_eq.md
@@ -1,21 +1,31 @@
 # <small>nlohmann::basic_json::</small>operator==
 
 ```cpp
-bool operator==(const_reference lhs, const_reference rhs) noexcept;
+// until C++20
+bool operator==(const_reference lhs, const_reference rhs) noexcept;   // (1)
 
 template<typename ScalarType>
-bool operator==(const_reference lhs, const ScalarType rhs) noexcept;
+bool operator==(const_reference lhs, const ScalarType rhs) noexcept;  // (2)
 
 template<typename ScalarType>
-bool operator==(ScalarType lhs, const const_reference rhs) noexcept;
+bool operator==(ScalarType lhs, const const_reference rhs) noexcept;  // (2)
+
+// since C++20
+class basic_json {
+    bool operator==(const_reference rhs) const noexcept;              // (1)
+
+    template<typename ScalarType>
+    bool operator==(ScalarType rhs) const noexcept;                   // (2)
+};
 ```
 
-Compares two JSON values for equality according to the following rules:
+1. Compares two JSON values for equality according to the following rules:
+    - Two JSON values are equal if (1) neither value is discarded, or (2) they are of the same
+      type and their stored values are the same according to their respective `operator==`.
+    - Integer and floating-point numbers are automatically converted before comparison.
 
-- Two JSON values are equal if (1) they are not discarded, (2) they are from the same type, and (3) their stored values
-  are the same according to their respective `operator==`.
-- Integer and floating-point numbers are automatically converted before comparison. Note that two NaN values are always
-  treated as unequal.
+2. Compares a JSON value and a scalar or a scalar and a JSON value for equality by converting the
+   scalar to a JSON value and comparing both JSON values according to 1.
 
 ## Template parameters
 
@@ -32,7 +42,7 @@ Compares two JSON values for equality according to the following rules:
 
 ## Return value
 
-whether the values `lhs` and `rhs` are equal
+whether the values `lhs`/`*this` and `rhs` are equal
 
 ## Exception safety
 
@@ -46,7 +56,11 @@ Linear.
 
 !!! note "Comparing special values"
 
-    - NaN values never compare equal to themselves or to other NaN values.
+    - `NaN` values are unordered within the domain of numbers.
+      The following comparisons all yield `#!cpp false`:
+        1. Comparing a `NaN` with itself.
+        2. Comparing a `NaN` with another `NaN`.
+        3. Comparing a `NaN` and any other number.
     - JSON `#!cpp null` values are all equal.
     - Discarded values never compare equal to themselves.
 
@@ -117,4 +131,5 @@ Linear.
 
 ## Version history
 
-- Added in version 1.0.0.
+1. Added in version 1.0.0. Added C++20 member functions in version 3.11.0.
+2. Added in version 1.0.0. Added C++20 member functions in version 3.11.0.
diff --git a/docs/mkdocs/docs/api/basic_json/operator_ge.md b/docs/mkdocs/docs/api/basic_json/operator_ge.md
@@ -1,17 +1,25 @@
 # <small>nlohmann::basic_json::</small>operator>=
 
 ```cpp
-bool operator>=(const_reference lhs, const_reference rhs) noexcept,
+// until C++20
+bool operator>=(const_reference lhs, const_reference rhs) noexcept;   // (1)
 
 template<typename ScalarType>
-bool operator>=(const_reference lhs, const ScalarType rhs) noexcept;
+bool operator>=(const_reference lhs, const ScalarType rhs) noexcept;  // (2)
 
 template<typename ScalarType>
-bool operator>=(ScalarType lhs, const const_reference rhs) noexcept;
+bool operator>=(ScalarType lhs, const const_reference rhs) noexcept;  // (2)
 ```
 
-Compares whether one JSON value `lhs` is greater than or equal to another JSON value `rhs` by calculating
-`#!cpp !(lhs < rhs)`.
+1. Compares whether one JSON value `lhs` is greater than or equal to another JSON value `rhs`
+   according to the following rules:
+    - The comparison always yields `#!cpp false` if (1) either operand is discarded, or (2) either
+      operand is `NaN` and the other operand is either `NaN` or any other number.
+    - Otherwise, returns the result of `#!cpp !(lhs < rhs)`.
+
+2. Compares wether a JSON value is greater than or equal to a scalar or a scalar is greater than or
+   equal to a JSON value by converting the scalar to a JSON value and comparing both JSON values
+   according to 1.
 
 ## Template parameters
 
@@ -38,6 +46,21 @@ No-throw guarantee: this function never throws exceptions.
 
 Linear.
 
+## Notes
+
+!!! note "Comparing `NaN`"
+
+    `NaN` values are unordered within the domain of numbers.
+    The following comparisons all yield `#!cpp false`:
+      1. Comparing a `NaN` with itself.
+      2. Comparing a `NaN` with another `NaN`.
+      3. Comparing a `NaN` and any other number.
+
+!!! note "Operator overload resolution"
+
+    Since C++20 overload resolution will consider the _rewritten candidate_ generated from
+    [`operator<=>`](operator_spaceship.md).
+
 ## Examples
 
 ??? example
@@ -54,6 +77,11 @@ Linear.
     --8<-- "examples/operator__greaterequal.output"
     ```
 
+## See also
+
+- [**operator<=>**](operator_spaceship.md) comparison: 3-way
+
 ## Version history
 
-- Added in version 1.0.0.
+1. Added in version 1.0.0. Conditionally removed since C++20 in version 3.11.0.
+2. Added in version 1.0.0. Conditionally removed since C++20 in version 3.11.0.
diff --git a/docs/mkdocs/docs/api/basic_json/operator_gt.md b/docs/mkdocs/docs/api/basic_json/operator_gt.md
@@ -1,16 +1,24 @@
 # <small>nlohmann::basic_json::</small>operator>
 
 ```cpp
-bool operator>(const_reference lhs, const_reference rhs) noexcept,
+// until C++20
+bool operator>(const_reference lhs, const_reference rhs) noexcept;   // (1)
 
 template<typename ScalarType>
-bool operator>(const_reference lhs, const ScalarType rhs) noexcept;
+bool operator>(const_reference lhs, const ScalarType rhs) noexcept;  // (2)
 
 template<typename ScalarType>
-bool operator>(ScalarType lhs, const const_reference rhs) noexcept;
+bool operator>(ScalarType lhs, const const_reference rhs) noexcept;  // (2)
 ```
 
-Compares whether one JSON value `lhs` is greater than another JSON value `rhs` by calculating `#!cpp !(lhs <= rhs)`.
+1. Compares whether one JSON value `lhs` is greater than another JSON value `rhs` according to the
+  following rules:
+    - The comparison always yields `#!cpp false` if (1) either operand is discarded, or (2) either
+      operand is `NaN` and the other operand is either `NaN` or any other number.
+    - Otherwise, returns the result of `#!cpp !(lhs <= rhs)`.
+
+2. Compares wether a JSON value is greater than a scalar or a scalar is greater than a JSON value by
+   converting the scalar to a JSON value and comparing both JSON values according to 1.
 
 ## Template parameters
 
@@ -37,6 +45,21 @@ No-throw guarantee: this function never throws exceptions.
 
 Linear.
 
+## Notes
+
+!!! note "Comparing `NaN`"
+
+    `NaN` values are unordered within the domain of numbers.
+    The following comparisons all yield `#!cpp false`:
+      1. Comparing a `NaN` with itself.
+      2. Comparing a `NaN` with another `NaN`.
+      3. Comparing a `NaN` and any other number.
+
+!!! note "Operator overload resolution"
+
+    Since C++20 overload resolution will consider the _rewritten candidate_ generated from
+    [`operator<=>`](operator_spaceship.md).
+
 ## Examples
 
 ??? example
@@ -53,6 +76,11 @@ Linear.
     --8<-- "examples/operator__greater.output"
     ```
 
+## See also
+
+- [**operator<=>**](operator_spaceship.md) comparison: 3-way
+
 ## Version history
 
-- Added in version 1.0.0.
+1. Added in version 1.0.0. Conditionally removed since C++20 in version 3.11.0.
+2. Added in version 1.0.0. Conditionally removed since C++20 in version 3.11.0.