make sure values are overwritten in from_json overloads

Caused unexpected behaviors when using get_to with values previously
set.

Fixes !1511
This commit is contained in:
Théo DELRIEU 2019-04-01 13:09:38 +02:00
parent b21c04c938
commit 2806b201a8
No known key found for this signature in database
GPG key ID: 7D6E00D1DF01DEAF
4 changed files with 206 additions and 41 deletions

View file

@ -136,6 +136,7 @@ void from_json(const BasicJsonType& j, std::forward_list<T, Allocator>& l)
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
l.clear();
std::transform(j.rbegin(), j.rend(), std::transform(j.rbegin(), j.rend(),
std::front_inserter(l), [](const BasicJsonType & i) std::front_inserter(l), [](const BasicJsonType & i)
{ {
@ -182,14 +183,16 @@ auto from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr, p
{ {
using std::end; using std::end;
arr.reserve(j.size()); ConstructibleArrayType ret;
ret.reserve(j.size());
std::transform(j.begin(), j.end(), std::transform(j.begin(), j.end(),
std::inserter(arr, end(arr)), [](const BasicJsonType & i) std::inserter(ret, end(ret)), [](const BasicJsonType & i)
{ {
// get<BasicJsonType>() returns *this, this won't call a from_json // get<BasicJsonType>() returns *this, this won't call a from_json
// method when value_type is BasicJsonType // method when value_type is BasicJsonType
return i.template get<typename ConstructibleArrayType::value_type>(); return i.template get<typename ConstructibleArrayType::value_type>();
}); });
arr = std::move(ret);
} }
template <typename BasicJsonType, typename ConstructibleArrayType> template <typename BasicJsonType, typename ConstructibleArrayType>
@ -198,14 +201,16 @@ void from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr,
{ {
using std::end; using std::end;
ConstructibleArrayType ret;
std::transform( std::transform(
j.begin(), j.end(), std::inserter(arr, end(arr)), j.begin(), j.end(), std::inserter(ret, end(ret)),
[](const BasicJsonType & i) [](const BasicJsonType & i)
{ {
// get<BasicJsonType>() returns *this, this won't call a from_json // get<BasicJsonType>() returns *this, this won't call a from_json
// method when value_type is BasicJsonType // method when value_type is BasicJsonType
return i.template get<typename ConstructibleArrayType::value_type>(); return i.template get<typename ConstructibleArrayType::value_type>();
}); });
arr = std::move(ret);
} }
template <typename BasicJsonType, typename ConstructibleArrayType, template <typename BasicJsonType, typename ConstructibleArrayType,
@ -239,15 +244,17 @@ void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
JSON_THROW(type_error::create(302, "type must be object, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be object, but is " + std::string(j.type_name())));
} }
ConstructibleObjectType ret;
auto inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>(); auto inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
using value_type = typename ConstructibleObjectType::value_type; using value_type = typename ConstructibleObjectType::value_type;
std::transform( std::transform(
inner_object->begin(), inner_object->end(), inner_object->begin(), inner_object->end(),
std::inserter(obj, obj.begin()), std::inserter(ret, ret.begin()),
[](typename BasicJsonType::object_t::value_type const & p) [](typename BasicJsonType::object_t::value_type const & p)
{ {
return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>()); return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
}); });
obj = std::move(ret);
} }
// overload for arithmetic types, not chosen for basic_json template arguments // overload for arithmetic types, not chosen for basic_json template arguments
@ -319,6 +326,7 @@ void from_json(const BasicJsonType& j, std::map<Key, Value, Compare, Allocator>&
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
m.clear();
for (const auto& p : j) for (const auto& p : j)
{ {
if (JSON_UNLIKELY(not p.is_array())) if (JSON_UNLIKELY(not p.is_array()))
@ -338,6 +346,7 @@ void from_json(const BasicJsonType& j, std::unordered_map<Key, Value, Hash, KeyE
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
m.clear();
for (const auto& p : j) for (const auto& p : j)
{ {
if (JSON_UNLIKELY(not p.is_array())) if (JSON_UNLIKELY(not p.is_array()))

View file

@ -192,10 +192,19 @@ struct is_constructible_object_type_impl <
using object_t = typename BasicJsonType::object_t; using object_t = typename BasicJsonType::object_t;
static constexpr bool value = static constexpr bool value =
(std::is_constructible<typename ConstructibleObjectType::key_type, typename object_t::key_type>::value and (std::is_default_constructible<ConstructibleObjectType>::value and
std::is_same<typename object_t::mapped_type, typename ConstructibleObjectType::mapped_type>::value) or (std::is_move_assignable<ConstructibleObjectType>::value or
(has_from_json<BasicJsonType, typename ConstructibleObjectType::mapped_type>::value or std::is_copy_assignable<ConstructibleObjectType>::value) and
has_non_default_from_json<BasicJsonType, typename ConstructibleObjectType::mapped_type >::value); (std::is_constructible<typename ConstructibleObjectType::key_type,
typename object_t::key_type>::value and
std::is_same <
typename object_t::mapped_type,
typename ConstructibleObjectType::mapped_type >::value)) or
(has_from_json<BasicJsonType,
typename ConstructibleObjectType::mapped_type>::value or
has_non_default_from_json <
BasicJsonType,
typename ConstructibleObjectType::mapped_type >::value);
}; };
template <typename BasicJsonType, typename ConstructibleObjectType> template <typename BasicJsonType, typename ConstructibleObjectType>
@ -278,20 +287,24 @@ struct is_constructible_array_type_impl <
BasicJsonType, ConstructibleArrayType, BasicJsonType, ConstructibleArrayType,
enable_if_t<not std::is_same<ConstructibleArrayType, enable_if_t<not std::is_same<ConstructibleArrayType,
typename BasicJsonType::value_type>::value and typename BasicJsonType::value_type>::value and
is_detected<value_type_t, ConstructibleArrayType>::value and std::is_default_constructible<ConstructibleArrayType>::value and
is_detected<iterator_t, ConstructibleArrayType>::value and (std::is_move_assignable<ConstructibleArrayType>::value or
is_complete_type< std::is_copy_assignable<ConstructibleArrayType>::value) and
detected_t<value_type_t, ConstructibleArrayType>>::value >> is_detected<value_type_t, ConstructibleArrayType>::value and
is_detected<iterator_t, ConstructibleArrayType>::value and
is_complete_type<
detected_t<value_type_t, ConstructibleArrayType>>::value >>
{ {
static constexpr bool value = static constexpr bool value =
// This is needed because json_reverse_iterator has a ::iterator type, // This is needed because json_reverse_iterator has a ::iterator type,
// furthermore, std::back_insert_iterator (and other iterators) have a base class `iterator`... // furthermore, std::back_insert_iterator (and other iterators) have a
// Therefore it is detected as a ConstructibleArrayType. // base class `iterator`... Therefore it is detected as a
// The real fix would be to have an Iterable concept. // ConstructibleArrayType. The real fix would be to have an Iterable
not is_iterator_traits < // concept.
iterator_traits<ConstructibleArrayType >>::value and not is_iterator_traits<iterator_traits<ConstructibleArrayType>>::value and
(std::is_same<typename ConstructibleArrayType::value_type, typename BasicJsonType::array_t::value_type>::value or (std::is_same<typename ConstructibleArrayType::value_type,
typename BasicJsonType::array_t::value_type>::value or
has_from_json<BasicJsonType, has_from_json<BasicJsonType,
typename ConstructibleArrayType::value_type>::value or typename ConstructibleArrayType::value_type>::value or
has_non_default_from_json < has_non_default_from_json <

View file

@ -1059,10 +1059,19 @@ struct is_constructible_object_type_impl <
using object_t = typename BasicJsonType::object_t; using object_t = typename BasicJsonType::object_t;
static constexpr bool value = static constexpr bool value =
(std::is_constructible<typename ConstructibleObjectType::key_type, typename object_t::key_type>::value and (std::is_default_constructible<ConstructibleObjectType>::value and
std::is_same<typename object_t::mapped_type, typename ConstructibleObjectType::mapped_type>::value) or (std::is_move_assignable<ConstructibleObjectType>::value or
(has_from_json<BasicJsonType, typename ConstructibleObjectType::mapped_type>::value or std::is_copy_assignable<ConstructibleObjectType>::value) and
has_non_default_from_json<BasicJsonType, typename ConstructibleObjectType::mapped_type >::value); (std::is_constructible<typename ConstructibleObjectType::key_type,
typename object_t::key_type>::value and
std::is_same <
typename object_t::mapped_type,
typename ConstructibleObjectType::mapped_type >::value)) or
(has_from_json<BasicJsonType,
typename ConstructibleObjectType::mapped_type>::value or
has_non_default_from_json <
BasicJsonType,
typename ConstructibleObjectType::mapped_type >::value);
}; };
template <typename BasicJsonType, typename ConstructibleObjectType> template <typename BasicJsonType, typename ConstructibleObjectType>
@ -1145,20 +1154,24 @@ struct is_constructible_array_type_impl <
BasicJsonType, ConstructibleArrayType, BasicJsonType, ConstructibleArrayType,
enable_if_t<not std::is_same<ConstructibleArrayType, enable_if_t<not std::is_same<ConstructibleArrayType,
typename BasicJsonType::value_type>::value and typename BasicJsonType::value_type>::value and
is_detected<value_type_t, ConstructibleArrayType>::value and std::is_default_constructible<ConstructibleArrayType>::value and
is_detected<iterator_t, ConstructibleArrayType>::value and (std::is_move_assignable<ConstructibleArrayType>::value or
is_complete_type< std::is_copy_assignable<ConstructibleArrayType>::value) and
detected_t<value_type_t, ConstructibleArrayType>>::value >> is_detected<value_type_t, ConstructibleArrayType>::value and
is_detected<iterator_t, ConstructibleArrayType>::value and
is_complete_type<
detected_t<value_type_t, ConstructibleArrayType>>::value >>
{ {
static constexpr bool value = static constexpr bool value =
// This is needed because json_reverse_iterator has a ::iterator type, // This is needed because json_reverse_iterator has a ::iterator type,
// furthermore, std::back_insert_iterator (and other iterators) have a base class `iterator`... // furthermore, std::back_insert_iterator (and other iterators) have a
// Therefore it is detected as a ConstructibleArrayType. // base class `iterator`... Therefore it is detected as a
// The real fix would be to have an Iterable concept. // ConstructibleArrayType. The real fix would be to have an Iterable
not is_iterator_traits < // concept.
iterator_traits<ConstructibleArrayType >>::value and not is_iterator_traits<iterator_traits<ConstructibleArrayType>>::value and
(std::is_same<typename ConstructibleArrayType::value_type, typename BasicJsonType::array_t::value_type>::value or (std::is_same<typename ConstructibleArrayType::value_type,
typename BasicJsonType::array_t::value_type>::value or
has_from_json<BasicJsonType, has_from_json<BasicJsonType,
typename ConstructibleArrayType::value_type>::value or typename ConstructibleArrayType::value_type>::value or
has_non_default_from_json < has_non_default_from_json <
@ -1411,6 +1424,7 @@ void from_json(const BasicJsonType& j, std::forward_list<T, Allocator>& l)
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
l.clear();
std::transform(j.rbegin(), j.rend(), std::transform(j.rbegin(), j.rend(),
std::front_inserter(l), [](const BasicJsonType & i) std::front_inserter(l), [](const BasicJsonType & i)
{ {
@ -1457,14 +1471,16 @@ auto from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr, p
{ {
using std::end; using std::end;
arr.reserve(j.size()); ConstructibleArrayType ret;
ret.reserve(j.size());
std::transform(j.begin(), j.end(), std::transform(j.begin(), j.end(),
std::inserter(arr, end(arr)), [](const BasicJsonType & i) std::inserter(ret, end(ret)), [](const BasicJsonType & i)
{ {
// get<BasicJsonType>() returns *this, this won't call a from_json // get<BasicJsonType>() returns *this, this won't call a from_json
// method when value_type is BasicJsonType // method when value_type is BasicJsonType
return i.template get<typename ConstructibleArrayType::value_type>(); return i.template get<typename ConstructibleArrayType::value_type>();
}); });
arr = std::move(ret);
} }
template <typename BasicJsonType, typename ConstructibleArrayType> template <typename BasicJsonType, typename ConstructibleArrayType>
@ -1473,14 +1489,16 @@ void from_json_array_impl(const BasicJsonType& j, ConstructibleArrayType& arr,
{ {
using std::end; using std::end;
ConstructibleArrayType ret;
std::transform( std::transform(
j.begin(), j.end(), std::inserter(arr, end(arr)), j.begin(), j.end(), std::inserter(ret, end(ret)),
[](const BasicJsonType & i) [](const BasicJsonType & i)
{ {
// get<BasicJsonType>() returns *this, this won't call a from_json // get<BasicJsonType>() returns *this, this won't call a from_json
// method when value_type is BasicJsonType // method when value_type is BasicJsonType
return i.template get<typename ConstructibleArrayType::value_type>(); return i.template get<typename ConstructibleArrayType::value_type>();
}); });
arr = std::move(ret);
} }
template <typename BasicJsonType, typename ConstructibleArrayType, template <typename BasicJsonType, typename ConstructibleArrayType,
@ -1514,15 +1532,17 @@ void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
JSON_THROW(type_error::create(302, "type must be object, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be object, but is " + std::string(j.type_name())));
} }
ConstructibleObjectType ret;
auto inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>(); auto inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
using value_type = typename ConstructibleObjectType::value_type; using value_type = typename ConstructibleObjectType::value_type;
std::transform( std::transform(
inner_object->begin(), inner_object->end(), inner_object->begin(), inner_object->end(),
std::inserter(obj, obj.begin()), std::inserter(ret, ret.begin()),
[](typename BasicJsonType::object_t::value_type const & p) [](typename BasicJsonType::object_t::value_type const & p)
{ {
return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>()); return value_type(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
}); });
obj = std::move(ret);
} }
// overload for arithmetic types, not chosen for basic_json template arguments // overload for arithmetic types, not chosen for basic_json template arguments
@ -1594,6 +1614,7 @@ void from_json(const BasicJsonType& j, std::map<Key, Value, Compare, Allocator>&
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
m.clear();
for (const auto& p : j) for (const auto& p : j)
{ {
if (JSON_UNLIKELY(not p.is_array())) if (JSON_UNLIKELY(not p.is_array()))
@ -1613,6 +1634,7 @@ void from_json(const BasicJsonType& j, std::unordered_map<Key, Value, Hash, KeyE
{ {
JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name()))); JSON_THROW(type_error::create(302, "type must be array, but is " + std::string(j.type_name())));
} }
m.clear();
for (const auto& p : j) for (const auto& p : j)
{ {
if (JSON_UNLIKELY(not p.is_array())) if (JSON_UNLIKELY(not p.is_array()))

View file

@ -140,6 +140,54 @@ TEST_CASE("value conversion")
} }
} }
SECTION("get an object (explicit, get_to)")
{
json::object_t o_reference = {{"object", json::object()},
{"array", {1, 2, 3, 4}},
{"number", 42},
{"boolean", false},
{"null", nullptr},
{"string", "Hello world"}
};
json j(o_reference);
SECTION("json::object_t")
{
json::object_t o = {{"previous", "value"}};
j.get_to(o);
CHECK(json(o) == j);
}
SECTION("std::map<json::string_t, json>")
{
std::map<json::string_t, json> o{{"previous", "value"}};
j.get_to(o);
CHECK(json(o) == j);
}
SECTION("std::multimap<json::string_t, json>")
{
std::multimap<json::string_t, json> o{{"previous", "value"}};
j.get_to(o);
CHECK(json(o) == j);
}
SECTION("std::unordered_map<json::string_t, json>")
{
std::unordered_map<json::string_t, json> o{{"previous", "value"}};
j.get_to(o);
CHECK(json(o) == j);
}
SECTION("std::unordered_multimap<json::string_t, json>")
{
std::unordered_multimap<json::string_t, json> o{{"previous", "value"}};
j.get_to(o);
CHECK(json(o) == j);
}
}
SECTION("get an object (implicit)") SECTION("get an object (implicit)")
{ {
json::object_t o_reference = {{"object", json::object()}, json::object_t o_reference = {{"object", json::object()},
@ -226,11 +274,6 @@ TEST_CASE("value conversion")
#if not defined(JSON_NOEXCEPTION) #if not defined(JSON_NOEXCEPTION)
SECTION("reserve is called on containers that supports it") SECTION("reserve is called on containers that supports it")
{ {
// making the call to from_json throw in order to check capacity
std::vector<float> v;
CHECK_THROWS_AS(nlohmann::from_json(j, v), json::type_error&);
CHECK(v.capacity() == j.size());
// make sure all values are properly copied // make sure all values are properly copied
std::vector<int> v2 = json({1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); std::vector<int> v2 = json({1, 2, 3, 4, 5, 6, 7, 8, 9, 10});
CHECK(v2.size() == 10); CHECK(v2.size() == 10);
@ -302,6 +345,55 @@ TEST_CASE("value conversion")
} }
} }
SECTION("get an array (explicit, get_to)")
{
json::array_t a_reference{json(1), json(1u), json(2.2),
json(false), json("string"), json()};
json j(a_reference);
SECTION("json::array_t")
{
json::array_t a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
SECTION("std::valarray<json>")
{
std::valarray<json> a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
SECTION("std::list<json>")
{
std::list<json> a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
SECTION("std::forward_list<json>")
{
std::forward_list<json> a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
SECTION("std::vector<json>")
{
std::vector<json> a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
SECTION("std::deque<json>")
{
std::deque<json> a{"previous", "value"};
j.get_to(a);
CHECK(json(a) == j);
}
}
SECTION("get an array (implicit)") SECTION("get an array (implicit)")
{ {
json::array_t a_reference{json(1), json(1u), json(2.2), json::array_t a_reference{json(1), json(1u), json(2.2),
@ -433,6 +525,35 @@ TEST_CASE("value conversion")
#endif #endif
} }
SECTION("get a string (explicit, get_to)")
{
json::string_t s_reference{"Hello world"};
json j(s_reference);
SECTION("string_t")
{
json::string_t s = "previous value";
j.get_to(s);
CHECK(json(s) == j);
}
SECTION("std::string")
{
std::string s = "previous value";
j.get_to(s);
CHECK(json(s) == j);
}
#if defined(JSON_HAS_CPP_17)
SECTION("std::string_view")
{
std::string s = "previous value";
std::string_view sv = s;
j.get_to(sv);
CHECK(json(sv) == j);
}
#endif
}
SECTION("get null (explicit)") SECTION("get null (explicit)")
{ {
std::nullptr_t n = nullptr; std::nullptr_t n = nullptr;