From 261cc4e509c51d53c57d0c266abd4a78f134e6a4 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Fri, 8 Apr 2022 10:22:47 +0200 Subject: [PATCH] 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/detail/conversions/from_json.hpp | 11 ++- single_include/nlohmann/json.hpp | 11 ++- test/CMakeLists.txt | 5 -- test/src/unit-regression2.cpp | 69 +++++++++++++++++++ 4 files changed, 79 insertions(+), 17 deletions(-) diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp index 207d3e302..cc06f198b 100644 --- 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::value&& - !std::is_same::value, - int > = 0 > -void from_json(const BasicJsonType& j, ConstructibleStringType& s) + std::is_assignable::value + && !std::is_same::value + && !is_json_ref::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 index 8e2863501..c1f545b07 100644 --- 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::value&& - !std::is_same::value, - int > = 0 > -void from_json(const BasicJsonType& j, ConstructibleStringType& s) + std::is_assignable::value + && !std::is_same::value + && !is_json_ref::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 index 90c325861..2f06def50 100644 --- 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 index dd2edb4ff..55bbcac65 100644 --- 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() == ojit->second.get()); } + + 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’}) + // s = *j.template get_ptr(); + auto td = j.get(); + + 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