Fix constraints on from_json() for strings (#3427)

Constrain from_json() overload for StringType to not accept json_ref and require it to be assignable, instead of constructible, from basic_json::string_t. Re-enable C++14 tests on Clang <4.0. Fixes #3171. Fixes #3267. Fixes #3312. Fixes #3384.
nlohmann · Apr 8, 2022 · 261cc4e · 261cc4e
1 parent 15fa6a3
commit 261cc4e
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 17 deletions.
diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp
@@ -105,13 +105,12 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
 }
 
 template <
-    typename BasicJsonType, typename ConstructibleStringType,
+    typename BasicJsonType, typename StringType,
     enable_if_t <
-        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
-        !std::is_same<typename BasicJsonType::string_t,
-                      ConstructibleStringType>::value,
-        int > = 0 >
-void from_json(const BasicJsonType& j, ConstructibleStringType& s)
+        std::is_assignable<StringType&, const typename BasicJsonType::string_t>::value
+        && !std::is_same<typename BasicJsonType::string_t, StringType>::value
+        && !is_json_ref<StringType>::value, int > = 0 >
+void from_json(const BasicJsonType& j, StringType& s)
 {
     if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
     {

diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
@@ -3928,13 +3928,12 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
 }
 
 template <
-    typename BasicJsonType, typename ConstructibleStringType,
+    typename BasicJsonType, typename StringType,
     enable_if_t <
-        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
-        !std::is_same<typename BasicJsonType::string_t,
-                      ConstructibleStringType>::value,
-        int > = 0 >
-void from_json(const BasicJsonType& j, ConstructibleStringType& s)
+        std::is_assignable<StringType&, const typename BasicJsonType::string_t>::value
+        && !std::is_same<typename BasicJsonType::string_t, StringType>::value
+        && !is_json_ref<StringType>::value, int > = 0 >
+void from_json(const BasicJsonType& j, StringType& s)
 {
     if (JSON_HEDLEY_UNLIKELY(!j.is_string()))
     {

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -11,11 +11,6 @@ include(test)
 # override standard support
 #############################################################################
 
-# compiling json.hpp in C++14 mode fails with Clang <4.0
-if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.0)
-    unset(compiler_supports_cpp_14)
-endif()
-
 # Clang only supports C++17 starting from Clang 5.0
 if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5.0)
     unset(compiler_supports_cpp_17)

diff --git a/test/src/unit-regression2.cpp b/test/src/unit-regression2.cpp
@@ -240,6 +240,50 @@ inline void from_json(const nlohmann::json& j, FooBar& fb)
     j.at("value").get_to(fb.foo.value);
 }
 
+/////////////////////////////////////////////////////////////////////
+// for #3171
+/////////////////////////////////////////////////////////////////////
+
+struct for_3171_base // NOLINT(cppcoreguidelines-special-member-functions)
+{
+    for_3171_base(const std::string& /*unused*/ = {}) {}
+    virtual ~for_3171_base() = default;
+
+    virtual void _from_json(const json& j)
+    {
+        j.at("str").get_to(str);
+    }
+
+    std::string str{};
+};
+
+struct for_3171_derived : public for_3171_base
+{
+    for_3171_derived() = default;
+    explicit for_3171_derived(const std::string& /*unused*/) { }
+};
+
+inline void from_json(const json& j, for_3171_base& tb)
+{
+    tb._from_json(j);
+}
+
+/////////////////////////////////////////////////////////////////////
+// for #3312
+/////////////////////////////////////////////////////////////////////
+
+#ifdef JSON_HAS_CPP_20
+struct for_3312
+{
+    std::string name;
+};
+
+inline void from_json(const json& j, for_3312& obj)
+{
+    j.at("name").get_to(obj.name);
+}
+#endif
+
 TEST_CASE("regression tests 2")
 {
     SECTION("issue #1001 - Fix memory leak during parser callback")
@@ -791,6 +835,31 @@ TEST_CASE("regression tests 2")
         CHECK(jit->first == ojit->first);
         CHECK(jit->second.get<std::string>() == ojit->second.get<std::string>());
     }
+
+    SECTION("issue #3171 - if class is_constructible from std::string wrong from_json overload is being selected, compilation failed")
+    {
+        json j{{ "str", "value"}};
+
+        // failed with: error: no match for ‘operator=’ (operand types are ‘for_3171_derived’ and ‘const nlohmann::basic_json<>::string_t’
+        //                                               {aka ‘const std::__cxx11::basic_string<char>’})
+        //                  s = *j.template get_ptr<const typename BasicJsonType::string_t*>();
+        auto td = j.get<for_3171_derived>();
+
+        CHECK(td.str == "value");
+    }
+
+#ifdef JSON_HAS_CPP_20
+    SECTION("issue #3312 - Parse to custom class from unordered_json breaks on G++11.2.0 with C++20")
+    {
+        // see test for #3171
+        ordered_json j = {{"name", "class"}};
+        for_3312 obj{};
+
+        j.get_to(obj);
+
+        CHECK(obj.name == "class");
+    }
+#endif
 }
 
 DOCTEST_CLANG_SUPPRESS_WARNING_POP