From 0e694b4060ed55df980eaaebc2398b0ff24530d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20DELRIEU?= Date: Thu, 14 Oct 2021 19:19:46 +0200 Subject: [PATCH] fix std::filesystem::path regression (#3073) * meta: rework is_compatible/is_constructible_string_type These type traits performed an incorrect and insufficient check. Converting to a std::filesystem::path used to work by accident thanks to these brittle constraints, but the clean-up performed in #3020 broke them. * support std::filesystem::path Fixes #3070 --- .../nlohmann/detail/conversions/from_json.hpp | 18 ++- .../nlohmann/detail/conversions/to_json.hpp | 13 ++ include/nlohmann/detail/meta/type_traits.hpp | 39 ++--- single_include/nlohmann/json.hpp | 71 +++++---- test/src/unit-regression2.cpp | 139 ++++++++++-------- 5 files changed, 161 insertions(+), 119 deletions(-) diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp index c7bd018e34..71e32aaf71 100644 --- a/include/nlohmann/detail/conversions/from_json.hpp +++ b/include/nlohmann/detail/conversions/from_json.hpp @@ -19,6 +19,10 @@ #include #include +#ifdef JSON_HAS_CPP_17 + #include +#endif + namespace nlohmann { namespace detail @@ -169,7 +173,7 @@ void from_json(const BasicJsonType& j, std::valarray& l) } template -auto from_json(const BasicJsonType& j, T (&arr)[N]) // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) +auto from_json(const BasicJsonType& j, T (&arr)[N]) // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) -> decltype(j.template get(), void()) { for (std::size_t i = 0; i < N; ++i) @@ -444,6 +448,18 @@ void from_json(const BasicJsonType& j, std::unordered_map +void from_json(const BasicJsonType& j, std::filesystem::path& p) +{ + if (JSON_HEDLEY_UNLIKELY(!j.is_string())) + { + JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j)); + } + p = *j.template get_ptr(); +} +#endif + struct from_json_fn { template diff --git a/include/nlohmann/detail/conversions/to_json.hpp b/include/nlohmann/detail/conversions/to_json.hpp index 06323a2729..79fdb3233f 100644 --- a/include/nlohmann/detail/conversions/to_json.hpp +++ b/include/nlohmann/detail/conversions/to_json.hpp @@ -9,11 +9,16 @@ #include // valarray #include // vector +#include #include #include #include #include +#ifdef JSON_HAS_CPP_17 + #include +#endif + namespace nlohmann { namespace detail @@ -386,6 +391,14 @@ void to_json(BasicJsonType& j, const T& t) to_json_tuple_impl(j, t, make_index_sequence::value> {}); } +#ifdef JSON_HAS_CPP_17 +template +void to_json(BasicJsonType& j, const std::filesystem::path& p) +{ + j = p.string(); +} +#endif + struct to_json_fn { template diff --git a/include/nlohmann/detail/meta/type_traits.hpp b/include/nlohmann/detail/meta/type_traits.hpp index ca6051e7cd..984ca19319 100644 --- a/include/nlohmann/detail/meta/type_traits.hpp +++ b/include/nlohmann/detail/meta/type_traits.hpp @@ -309,44 +309,21 @@ struct is_constructible_object_type : is_constructible_object_type_impl {}; -template -struct is_compatible_string_type_impl : std::false_type {}; - template -struct is_compatible_string_type_impl < - BasicJsonType, CompatibleStringType, - enable_if_t::value >> +struct is_compatible_string_type { static constexpr auto value = is_constructible::value; }; template -struct is_compatible_string_type - : is_compatible_string_type_impl {}; - -template -struct is_constructible_string_type_impl : std::false_type {}; - -template -struct is_constructible_string_type_impl < - BasicJsonType, ConstructibleStringType, - enable_if_t::value >> +struct is_constructible_string_type { static constexpr auto value = is_constructible::value; }; -template -struct is_constructible_string_type - : is_constructible_string_type_impl {}; - template struct is_compatible_array_type_impl : std::false_type {}; @@ -355,7 +332,10 @@ struct is_compatible_array_type_impl < BasicJsonType, CompatibleArrayType, enable_if_t < is_detected::value&& - is_iterator_traits>>::value >> + is_iterator_traits>>::value&& +// special case for types like std::filesystem::path whose iterator's value_type are themselves +// c.f. https://github.com/nlohmann/json/pull/3073 + !std::is_same>::value >> { static constexpr bool value = is_constructible::value&& is_iterator_traits>>::value&& is_detected::value&& -is_complete_type < -detected_t>::value >> +// special case for types like std::filesystem::path whose iterator's value_type are themselves +// c.f. https://github.com/nlohmann/json/pull/3073 +!std::is_same>::value&& + is_complete_type < + detected_t>::value >> { using value_type = range_value_t; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 25c6983b04..3f68f33f70 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3783,44 +3783,21 @@ struct is_constructible_object_type : is_constructible_object_type_impl {}; -template -struct is_compatible_string_type_impl : std::false_type {}; - template -struct is_compatible_string_type_impl < - BasicJsonType, CompatibleStringType, - enable_if_t::value >> +struct is_compatible_string_type { static constexpr auto value = is_constructible::value; }; template -struct is_compatible_string_type - : is_compatible_string_type_impl {}; - -template -struct is_constructible_string_type_impl : std::false_type {}; - -template -struct is_constructible_string_type_impl < - BasicJsonType, ConstructibleStringType, - enable_if_t::value >> +struct is_constructible_string_type { static constexpr auto value = is_constructible::value; }; -template -struct is_constructible_string_type - : is_constructible_string_type_impl {}; - template struct is_compatible_array_type_impl : std::false_type {}; @@ -3829,7 +3806,10 @@ struct is_compatible_array_type_impl < BasicJsonType, CompatibleArrayType, enable_if_t < is_detected::value&& - is_iterator_traits>>::value >> + is_iterator_traits>>::value&& +// special case for types like std::filesystem::path whose iterator's value_type are themselves +// c.f. https://github.com/nlohmann/json/pull/3073 + !std::is_same>::value >> { static constexpr bool value = is_constructible::value&& is_iterator_traits>>::value&& is_detected::value&& -is_complete_type < -detected_t>::value >> +// special case for types like std::filesystem::path whose iterator's value_type are themselves +// c.f. https://github.com/nlohmann/json/pull/3073 +!std::is_same>::value&& + is_complete_type < + detected_t>::value >> { using value_type = range_value_t; @@ -3967,6 +3950,10 @@ T conditional_static_cast(U value) // #include +#ifdef JSON_HAS_CPP_17 + #include +#endif + namespace nlohmann { namespace detail @@ -4117,7 +4104,7 @@ void from_json(const BasicJsonType& j, std::valarray& l) } template -auto from_json(const BasicJsonType& j, T (&arr)[N]) // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) +auto from_json(const BasicJsonType& j, T (&arr)[N]) // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) -> decltype(j.template get(), void()) { for (std::size_t i = 0; i < N; ++i) @@ -4392,6 +4379,18 @@ void from_json(const BasicJsonType& j, std::unordered_map +void from_json(const BasicJsonType& j, std::filesystem::path& p) +{ + if (JSON_HEDLEY_UNLIKELY(!j.is_string())) + { + JSON_THROW(type_error::create(302, "type must be string, but is " + std::string(j.type_name()), j)); + } + p = *j.template get_ptr(); +} +#endif + struct from_json_fn { template @@ -4425,6 +4424,8 @@ constexpr const auto& from_json = detail::static_const::va #include // valarray #include // vector +// #include + // #include @@ -4625,6 +4626,10 @@ class tuple_element> // #include +#ifdef JSON_HAS_CPP_17 + #include +#endif + namespace nlohmann { namespace detail @@ -4997,6 +5002,14 @@ void to_json(BasicJsonType& j, const T& t) to_json_tuple_impl(j, t, make_index_sequence::value> {}); } +#ifdef JSON_HAS_CPP_17 +template +void to_json(BasicJsonType& j, const std::filesystem::path& p) +{ + j = p.string(); +} +#endif + struct to_json_fn { template diff --git a/test/src/unit-regression2.cpp b/test/src/unit-regression2.cpp index cf74297825..b5a6ca0a46 100644 --- a/test/src/unit-regression2.cpp +++ b/test/src/unit-regression2.cpp @@ -37,16 +37,17 @@ SOFTWARE. using json = nlohmann::json; using ordered_json = nlohmann::ordered_json; -#include #include +#include #include #include -#if (defined(__cplusplus) && __cplusplus >= 201703L) || (defined(_HAS_CXX17) && _HAS_CXX17 == 1) // fix for issue #464 +#if (defined(__cplusplus) && __cplusplus >= 201703L) || (defined(_HAS_CXX17) && _HAS_CXX17 == 1) // fix for issue #464 #define JSON_HAS_CPP_17 #endif #ifdef JSON_HAS_CPP_17 + #include #include #endif @@ -69,14 +70,19 @@ using float_json = nlohmann::basic_json +template<> struct adl_serializer { - static NonDefaultFromJsonStruct from_json (json const& /*unused*/) noexcept + static NonDefaultFromJsonStruct from_json(json const& /*unused*/) noexcept { return {}; } }; -} // namespace nlohmann +} // namespace nlohmann ///////////////////////////////////////////////////////////////////// // for #1805 @@ -138,28 +147,29 @@ struct NotSerializableData float myfloat; }; - ///////////////////////////////////////////////////////////////////// // for #2574 ///////////////////////////////////////////////////////////////////// struct NonDefaultConstructible { - explicit NonDefaultConstructible (int a) : x(a) { } + explicit NonDefaultConstructible(int a) + : x(a) + {} int x; }; namespace nlohmann { -template <> +template<> struct adl_serializer { - static NonDefaultConstructible from_json (json const& j) + static NonDefaultConstructible from_json(json const& j) { return NonDefaultConstructible(j.get()); } }; -} // namespace nlohmann +} // namespace nlohmann ///////////////////////////////////////////////////////////////////// // for #2824 @@ -168,11 +178,13 @@ struct adl_serializer class sax_no_exception : public nlohmann::detail::json_sax_dom_parser { public: - explicit sax_no_exception(json& j) : nlohmann::detail::json_sax_dom_parser(j, false) {} + explicit sax_no_exception(json& j) + : nlohmann::detail::json_sax_dom_parser(j, false) + {} static bool parse_error(std::size_t /*position*/, const std::string& /*last_token*/, const json::exception& ex) { - error_string = new std::string(ex.what()); // NOLINT(cppcoreguidelines-owning-memory) + error_string = new std::string(ex.what()); // NOLINT(cppcoreguidelines-owning-memory) return false; } @@ -296,22 +308,26 @@ TEST_CASE("regression tests 2") using it_type = decltype(p1.begin()); std::set_difference( - p1.begin(), p1.end(), - p2.begin(), p2.end(), - std::inserter(diffs, diffs.end()), [&](const it_type & e1, const it_type & e2) -> bool + p1.begin(), + p1.end(), + p2.begin(), + p2.end(), + std::inserter(diffs, diffs.end()), + [&](const it_type & e1, const it_type & e2) -> bool { - using comper_pair = std::pair; // Trying to avoid unneeded copy - return comper_pair(e1.key(), e1.value()) < comper_pair(e2.key(), e2.value()); // Using pair comper + using comper_pair = std::pair; // Trying to avoid unneeded copy + return comper_pair(e1.key(), e1.value()) < comper_pair(e2.key(), e2.value()); // Using pair comper }); - CHECK(diffs.size() == 1); // Note the change here, was 2 + CHECK(diffs.size() == 1); // Note the change here, was 2 } #ifdef JSON_HAS_CPP_17 SECTION("issue #1292 - Serializing std::variant causes stack overflow") { static_assert( - !std::is_constructible>::value, ""); + !std::is_constructible>::value, + ""); } #endif @@ -376,24 +392,7 @@ TEST_CASE("regression tests 2") nlohmann::json dump_test; const std::array data = { - { - 109, 108, 103, 125, -122, -53, 115, - 18, 3, 0, 102, 19, 1, 15, - -110, 13, -3, -1, -81, 32, 2, - 0, 0, 0, 0, 0, 0, 0, - 8, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, -80, 2, - 0, 0, 96, -118, 46, -116, 46, - 109, -84, -87, 108, 14, 109, -24, - -83, 13, -18, -51, -83, -52, -115, - 14, 6, 32, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, - 64, 3, 0, 0, 0, 35, -74, - -73, 55, 57, -128, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, - 0, 0, 33, 0, 0, 0, -96, - -54, -28, -26 - } + {109, 108, 103, 125, -122, -53, 115, 18, 3, 0, 102, 19, 1, 15, -110, 13, -3, -1, -81, 32, 2, 0, 0, 0, 0, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -80, 2, 0, 0, 96, -118, 46, -116, 46, 109, -84, -87, 108, 14, 109, -24, -83, 13, -18, -51, -83, -52, -115, 14, 6, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 64, 3, 0, 0, 0, 35, -74, -73, 55, 57, -128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 33, 0, 0, 0, -96, -54, -28, -26} }; std::string s; for (int i : data) @@ -453,7 +452,7 @@ TEST_CASE("regression tests 2") SECTION("string array") { - const std::array input = {{ 'B', 0x00 }}; + const std::array input = {{'B', 0x00}}; json cbor = json::from_cbor(input, true, false); CHECK(cbor.is_discarded()); } @@ -491,12 +490,11 @@ TEST_CASE("regression tests 2") const std::array data = {{0x81, 0xA4, 0x64, 0x61, 0x74, 0x61, 0xC4, 0x0F, 0x33, 0x30, 0x30, 0x32, 0x33, 0x34, 0x30, 0x31, 0x30, 0x37, 0x30, 0x35, 0x30, 0x31, 0x30}}; json j = json::from_msgpack(data.data(), data.size()); CHECK_NOTHROW( - j.dump(4, // Indent - ' ', // Indent char - false, // Ensure ascii + j.dump(4, // Indent + ' ', // Indent char + false, // Ensure ascii json::error_handler_t::strict // Error - ) - ); + )); } SECTION("PR #2181 - regression bug with lvalue") @@ -512,7 +510,16 @@ TEST_CASE("regression tests 2") { std::vector data = { - 0x7B, 0x6F, 0x62, 0x6A, 0x65, 0x63, 0x74, 0x20, 0x4F, 0x42 + 0x7B, + 0x6F, + 0x62, + 0x6A, + 0x65, + 0x63, + 0x74, + 0x20, + 0x4F, + 0x42 }; json result = json::from_cbor(data, true, false); CHECK(result.is_discarded()); @@ -562,11 +569,10 @@ TEST_CASE("regression tests 2") SECTION("std::array") { { - json j = { 7, 4 }; + json j = {7, 4}; auto arr = j.get>(); CHECK(arr[0].x == 7); CHECK(arr[1].x == 4); - } { @@ -578,21 +584,21 @@ TEST_CASE("regression tests 2") SECTION("std::pair") { { - json j = { 3, 8 }; + json j = {3, 8}; auto p = j.get>(); CHECK(p.first.x == 3); CHECK(p.second.x == 8); } { - json j = { 4, 1 }; + json j = {4, 1}; auto p = j.get>(); CHECK(p.first == 4); CHECK(p.second.x == 1); } { - json j = { 6, 7 }; + json j = {6, 7}; auto p = j.get>(); CHECK(p.first.x == 6); CHECK(p.second == 7); @@ -607,16 +613,16 @@ TEST_CASE("regression tests 2") SECTION("std::tuple") { { - json j = { 9 }; + json j = {9}; auto t = j.get>(); CHECK(std::get<0>(t).x == 9); } { - json j = { 9, 8, 7 }; + json j = {9, 8, 7}; auto t = j.get>(); CHECK(std::get<0>(t).x == 9); - CHECK(std::get<1>(t) == 8); + CHECK(std::get<1>(t) == 8); CHECK(std::get<2>(t).x == 7); } @@ -658,9 +664,9 @@ TEST_CASE("regression tests 2") json j; sax_no_exception sax(j); - CHECK (!json::sax_parse("xyz", &sax)); + CHECK(!json::sax_parse("xyz", &sax)); CHECK(*sax_no_exception::error_string == "[json.exception.parse_error.101] parse error at line 1, column 1: syntax error while parsing value - invalid literal; last read: 'x'"); - delete sax_no_exception::error_string; // NOLINT(cppcoreguidelines-owning-memory) + delete sax_no_exception::error_string; // NOLINT(cppcoreguidelines-owning-memory) } SECTION("issue #2825 - Properly constrain the basic_json conversion operator") @@ -695,6 +701,17 @@ TEST_CASE("regression tests 2") json k = json::from_cbor(my_vector); CHECK(j == k); } + +#ifdef JSON_HAS_CPP_17 + SECTION("issue #3070 - Version 3.10.3 breaks backward-compatibility with 3.10.2 ") + { + std::filesystem::path text_path("/tmp/text.txt"); + json j(text_path); + + const auto j_path = j.get(); + CHECK(j_path == text_path); + } +#endif } DOCTEST_CLANG_SUPPRESS_WARNING_POP