Further constrain from_json() for strings

Constrain from_json() overload for ConstructibleStringType to not accept json_ref and require assignability. Re-enable C++14 tests on Clang <4.0. Fixes nlohmann#3171, nlohmann#3312, nlohmann#3384. Maybe fixes nlohmann#3267.
falbrechtskirchinger · Apr 6, 2022 · 67cedc0 · 67cedc0
1 parent 52e16a9
commit 67cedc0
Show file tree

Hide file tree

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

diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp
@@ -3930,9 +3930,11 @@ void from_json(const BasicJsonType& j, typename BasicJsonType::string_t& s)
 template <
     typename BasicJsonType, typename ConstructibleStringType,
     enable_if_t <
-        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value&&
-        !std::is_same<typename BasicJsonType::string_t,
-                      ConstructibleStringType>::value,
+        is_constructible_string_type<BasicJsonType, ConstructibleStringType>::value
+        && !is_json_ref<ConstructibleStringType>::value
+        && std::is_assignable<ConstructibleStringType&, const typename BasicJsonType::string_t>::value
+        && !std::is_same<typename BasicJsonType::string_t,
+                         ConstructibleStringType>::value,
         int > = 0 >
 void from_json(const BasicJsonType& j, ConstructibleStringType& s)
 {

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
+/////////////////////////////////////////////////////////////////////
+
+// NOLINT(cppcoreguidelines-special-member-functions)
+struct for_3171_base
+{
+    for_3171_base(const std::string& /*unused*/ = {}) {}
+    virtual ~for_3171_base() = default;
+
+    virtual void _from_json(const json& j)
+    {
+        str = j.at("str");
+    }
+
+    std::string str{};
+};
+
+struct for_3171_derived : public for_3171_base
+{
+    for_3171_derived() = default;
+    explicit for_3171_derived(const std::string& /*unused*/) { }
+};
+
+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;
+};
+
+void from_json(const nlohmann::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 ‘test_derived_3171’ 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