mirror of
https://github.com/nlohmann/json.git
synced 2026-11-02 19:05:35 +00:00
insert(): strong guarantee for single elements, basic guarantee for the rest (#5996)
* Make basic_json::insert() provide its documented strong exception guarantee (#5986) A failed allocation while the new elements were copied left the value changed, and with JSON_DIAGNOSTICS stale parent pointers behind. The extra work now happens almost only on the failure path: - Array insert(pos, cnt, val) and insert(pos, first, last) append the copies at the end and rotate them into place. If a copy throws, the appended elements are erased again (noexcept) and the parents are set. Moving a basic_json cannot throw, so reallocation is safe. This is no slower than vector::insert (faster for numbers), needs no temporary array, and keeps the elements that libstdc++ lost on reallocation. - Array insert(pos, val) copies a non-scalar value first and inserts it by move, except at the end, where vector::insert is strong. Scalars keep the direct path. - The object range insert(first, last) into an empty object uses the container's own range insertion and clears the object on an exception. Into a non-empty object it inserts member by member and logs one bit per source member (a new member is detected by the size change; no allocation for the first 256 members); on an exception the members added are erased by walking the source range. No assumption that key_type is string_t. - The parents are set when a failed insertion is undone. The regression test fails every allocation of every insert() overload in turn, for json and ordered_json, and checks the value and its parents. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix clang-tidy and -Wnrvo findings in insert rollback helpers Declare the rollback helpers non-movable, replace the nested conditional and C array in object_insert_rollback, silence the intentional copy in the allocator test, and avoid the missed NRVO in the test factory lambda. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N5NasAgbDGG28MqXjQvLqp Signed-off-by: Claude <noreply@anthropic.com> * insert(): strong guarantee for single elements, basic for the rest Rework of the previous two commits, following the discussion in #5996: the standard containers give the strong guarantee only for a single element (std::vector: at the end, or if the exception does not come from copying T; std::map: single-element insert), and copying a basic_json allocates. Rebuilding a rollback for the count, range and object-range overloads on top of array_t/object_t made the code verbose and slower. - insert(pos, const basic_json&): at end() forward as before (the standard covers it); elsewhere copy first and insert by move, as libstdc++ does internally (libc++ shifts the tail before copying). - insert_iterator() and object insert(first, last) set the parents in a scope guard, so a failed insertion leaves a valid value with JSON_DIAGNOSTICS too (the next assert_invariant() aborted before). - Docs: strong guarantee for (1), basic guarantee for (2)-(5); the argument checks throw before anything is changed. - Test: allocation-failure sweep checks the strong guarantee for (1) and validity, type and parents for the other overloads. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * insert(): document the exception safety of overloads 2 to 5 once The four basic-guarantee statements were identical up to array/object. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * insert(): simplify the wording of the basic guarantee Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me> Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
bdc737db6d
commit
494a74c242
4 files changed
+298
-9
No files matched your search
@@ -65,7 +65,12 @@ the same index, which is now a different value.
|
||||
|
||||
## Exception safety
|
||||
|
||||
Strong exception safety: if an exception occurs, the original value stays intact.
|
||||
Overload (1) gives the strong guarantee: if an exception is thrown, there are no changes in the JSON value.
|
||||
|
||||
Overloads (2) to (5) give the basic guarantee: if an exception is thrown while the elements or members are copied
|
||||
(for example, `std::bad_alloc`), the JSON value may be partially modified, but it stays a valid array or object.
|
||||
|
||||
The exceptions listed below are thrown before the JSON value is changed.
|
||||
|
||||
## Exceptions
|
||||
|
||||
|
||||
@@ -4917,6 +4917,30 @@ public:
|
||||
return {it, res.second};
|
||||
}
|
||||
|
||||
private:
|
||||
/// Sets the parents of a value when it goes out of scope, also when an
|
||||
/// exception is thrown: a container insertion that throws may already
|
||||
/// have added elements, whose parents must be set for the value to stay
|
||||
/// valid (basic exception guarantee). Does nothing without JSON_DIAGNOSTICS.
|
||||
class set_parents_on_exit
|
||||
{
|
||||
public:
|
||||
explicit set_parents_on_exit(basic_json* v) noexcept : m_value(v) {}
|
||||
set_parents_on_exit(const set_parents_on_exit&) = delete;
|
||||
set_parents_on_exit& operator=(const set_parents_on_exit&) = delete;
|
||||
set_parents_on_exit(set_parents_on_exit&&) = delete;
|
||||
set_parents_on_exit& operator=(set_parents_on_exit&&) = delete;
|
||||
|
||||
~set_parents_on_exit()
|
||||
{
|
||||
m_value->set_parents();
|
||||
}
|
||||
|
||||
private:
|
||||
basic_json* m_value;
|
||||
};
|
||||
|
||||
public:
|
||||
/// Helper for insertion of an iterator
|
||||
/// @note: This uses std::distance to support GCC 4.8,
|
||||
/// see https://github.com/nlohmann/json/pull/1257
|
||||
@@ -4926,6 +4950,7 @@ public:
|
||||
iterator result(this);
|
||||
JSON_ASSERT(m_data.m_value.array != nullptr);
|
||||
|
||||
const set_parents_on_exit parents(this);
|
||||
auto insert_pos = std::distance(m_data.m_value.array->begin(), pos.m_it.array_iterator);
|
||||
m_data.m_value.array->insert(pos.m_it.array_iterator, std::forward<Args>(args)...);
|
||||
result.m_it.array_iterator = m_data.m_value.array->begin() + insert_pos;
|
||||
@@ -4934,7 +4959,6 @@ public:
|
||||
// result.m_it.array_iterator = m_data.m_value.array->insert(pos.m_it.array_iterator, cnt, val);
|
||||
// but the return value of insert is missing in GCC 4.8, so it is written this way instead.
|
||||
|
||||
set_parents();
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -4954,8 +4978,16 @@ public:
|
||||
throw_iterator_from_other_value(this);
|
||||
}
|
||||
|
||||
// insert to array and return iterator
|
||||
return insert_iterator(pos, val);
|
||||
// vector::insert has no effects if copying a single element to the
|
||||
// end throws; elsewhere, it may already have shifted the elements, so
|
||||
// copy first and insert by move, which cannot throw but by allocating
|
||||
if (pos.m_it.array_iterator == m_data.m_value.array->end())
|
||||
{
|
||||
return insert_iterator(pos, val);
|
||||
}
|
||||
|
||||
basic_json tmp(val);
|
||||
return insert_iterator(pos, std::move(tmp));
|
||||
}
|
||||
|
||||
/// @brief inserts element into array
|
||||
@@ -5087,8 +5119,8 @@ public:
|
||||
detail::throw_literal<invalid_iterator>(detail::exception_id::iterator_from_other_value, "iterators first and last must point to objects", this);
|
||||
}
|
||||
|
||||
const set_parents_on_exit parents(this);
|
||||
m_data.m_value.object->insert(first.m_it.object_iterator, last.m_it.object_iterator);
|
||||
set_parents();
|
||||
}
|
||||
|
||||
/// @brief updates a JSON object from another object, overwriting existing keys
|
||||
|
||||
Generated
+36
-4
@@ -42747,6 +42747,30 @@ public:
|
||||
return {it, res.second};
|
||||
}
|
||||
|
||||
private:
|
||||
/// Sets the parents of a value when it goes out of scope, also when an
|
||||
/// exception is thrown: a container insertion that throws may already
|
||||
/// have added elements, whose parents must be set for the value to stay
|
||||
/// valid (basic exception guarantee). Does nothing without JSON_DIAGNOSTICS.
|
||||
class set_parents_on_exit
|
||||
{
|
||||
public:
|
||||
explicit set_parents_on_exit(basic_json* v) noexcept : m_value(v) {}
|
||||
set_parents_on_exit(const set_parents_on_exit&) = delete;
|
||||
set_parents_on_exit& operator=(const set_parents_on_exit&) = delete;
|
||||
set_parents_on_exit(set_parents_on_exit&&) = delete;
|
||||
set_parents_on_exit& operator=(set_parents_on_exit&&) = delete;
|
||||
|
||||
~set_parents_on_exit()
|
||||
{
|
||||
m_value->set_parents();
|
||||
}
|
||||
|
||||
private:
|
||||
basic_json* m_value;
|
||||
};
|
||||
|
||||
public:
|
||||
/// Helper for insertion of an iterator
|
||||
/// @note: This uses std::distance to support GCC 4.8,
|
||||
/// see https://github.com/nlohmann/json/pull/1257
|
||||
@@ -42756,6 +42780,7 @@ public:
|
||||
iterator result(this);
|
||||
JSON_ASSERT(m_data.m_value.array != nullptr);
|
||||
|
||||
const set_parents_on_exit parents(this);
|
||||
auto insert_pos = std::distance(m_data.m_value.array->begin(), pos.m_it.array_iterator);
|
||||
m_data.m_value.array->insert(pos.m_it.array_iterator, std::forward<Args>(args)...);
|
||||
result.m_it.array_iterator = m_data.m_value.array->begin() + insert_pos;
|
||||
@@ -42764,7 +42789,6 @@ public:
|
||||
// result.m_it.array_iterator = m_data.m_value.array->insert(pos.m_it.array_iterator, cnt, val);
|
||||
// but the return value of insert is missing in GCC 4.8, so it is written this way instead.
|
||||
|
||||
set_parents();
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -42784,8 +42808,16 @@ public:
|
||||
throw_iterator_from_other_value(this);
|
||||
}
|
||||
|
||||
// insert to array and return iterator
|
||||
return insert_iterator(pos, val);
|
||||
// vector::insert has no effects if copying a single element to the
|
||||
// end throws; elsewhere, it may already have shifted the elements, so
|
||||
// copy first and insert by move, which cannot throw but by allocating
|
||||
if (pos.m_it.array_iterator == m_data.m_value.array->end())
|
||||
{
|
||||
return insert_iterator(pos, val);
|
||||
}
|
||||
|
||||
basic_json tmp(val);
|
||||
return insert_iterator(pos, std::move(tmp));
|
||||
}
|
||||
|
||||
/// @brief inserts element into array
|
||||
@@ -42917,8 +42949,8 @@ public:
|
||||
detail::throw_literal<invalid_iterator>(detail::exception_id::iterator_from_other_value, "iterators first and last must point to objects", this);
|
||||
}
|
||||
|
||||
const set_parents_on_exit parents(this);
|
||||
m_data.m_value.object->insert(first.m_it.object_iterator, last.m_it.object_iterator);
|
||||
set_parents();
|
||||
}
|
||||
|
||||
/// @brief updates a JSON object from another object, overwriting existing keys
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
#include <nlohmann/json.hpp>
|
||||
using nlohmann::json;
|
||||
|
||||
#include <functional>
|
||||
#include <valarray>
|
||||
#if JSON_HAS_RANGES
|
||||
#include <ranges>
|
||||
@@ -804,3 +805,222 @@ TEST_CASE("a failed allocation leaves the value unchanged")
|
||||
#endif
|
||||
}
|
||||
#endif
|
||||
|
||||
// the no-exceptions CI job has no std::bad_alloc to catch; with iterator
|
||||
// debugging (MSVC STL debug builds) containers allocate inside noexcept
|
||||
// functions, so failing an allocation there terminates the program
|
||||
#if !defined(JSON_NOEXCEPTION) && !(defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL > 0)
|
||||
namespace
|
||||
{
|
||||
// whether every child of j names j as its parent (always true without
|
||||
// JSON_DIAGNOSTICS, where no parents are stored)
|
||||
template<class BasicJsonType>
|
||||
bool children_name_their_parent(const BasicJsonType& j)
|
||||
{
|
||||
#if JSON_DIAGNOSTICS
|
||||
if (j.is_array())
|
||||
{
|
||||
for (const auto& element : *j.m_data.m_value.array)
|
||||
{
|
||||
if (element.m_parent != &j)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
else if (j.is_object())
|
||||
{
|
||||
for (const auto& element : *j.m_data.m_value.object)
|
||||
{
|
||||
if (element.second.m_parent != &j)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
#else
|
||||
static_cast<void>(j);
|
||||
#endif
|
||||
return true;
|
||||
}
|
||||
|
||||
enum class guarantee
|
||||
{
|
||||
basic, // the value stays valid and keeps its type
|
||||
strong // the value is unchanged
|
||||
};
|
||||
|
||||
// Runs op on a fresh value make() with the 0th, 1st, 2nd, ... allocation made
|
||||
// by the library failing in turn, until op completes. Every failed attempt
|
||||
// must have thrown std::bad_alloc and left the value valid, with all parent
|
||||
// pointers intact (copying a value asserts the invariant), and for the strong
|
||||
// guarantee exactly as it was.
|
||||
template<class BasicJsonType>
|
||||
void check_exception_guarantee(guarantee g,
|
||||
const std::function<BasicJsonType()>& make,
|
||||
const std::function<void(BasicJsonType&)>& op)
|
||||
{
|
||||
fail_at_alloc_call = -1;
|
||||
|
||||
bool completed = false;
|
||||
std::size_t failures = 0;
|
||||
for (std::size_t n = 0; !completed; ++n)
|
||||
{
|
||||
CAPTURE(n)
|
||||
REQUIRE(n < 1000);
|
||||
|
||||
BasicJsonType j = make();
|
||||
const std::string before = j.dump();
|
||||
const auto type_before = j.type();
|
||||
|
||||
alloc_call_count = 0;
|
||||
fail_at_alloc_call = static_cast<long>(n);
|
||||
try
|
||||
{
|
||||
op(j);
|
||||
completed = true;
|
||||
}
|
||||
catch (const std::bad_alloc&)
|
||||
{
|
||||
++failures;
|
||||
}
|
||||
fail_at_alloc_call = -1;
|
||||
|
||||
if (!completed)
|
||||
{
|
||||
CHECK((j.type() == type_before));
|
||||
CHECK(children_name_their_parent(j));
|
||||
const BasicJsonType copy(j); // NOLINT(performance-unnecessary-copy-initialization): the copy is the point, it allocates
|
||||
CHECK(copy.dump() == j.dump());
|
||||
if (g == guarantee::strong)
|
||||
{
|
||||
CHECK(j.dump() == before);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// the operation allocates, so at least one attempt must have failed
|
||||
CHECK(failures > 0);
|
||||
}
|
||||
|
||||
template<class BasicJsonType>
|
||||
void check_insert_exception_guarantees()
|
||||
{
|
||||
using json_t = BasicJsonType;
|
||||
using make_t = std::function<json_t()>;
|
||||
|
||||
// strings of this length do not fit into the small string buffer, so
|
||||
// copying them allocates
|
||||
const json_t val = std::string(64, 'x');
|
||||
const json_t source = json_t::array({std::string(64, 'p'), std::string(64, 'q'), std::string(64, 'r')});
|
||||
|
||||
for (const bool spare_capacity :
|
||||
{
|
||||
false, true
|
||||
})
|
||||
{
|
||||
CAPTURE(spare_capacity)
|
||||
|
||||
// [ "aaa...", "bbb...", "ccc..." ], with a spare slot if requested
|
||||
const make_t make = [spare_capacity]()
|
||||
{
|
||||
json_t j = json_t::array({std::string(64, 'a'), std::string(64, 'b'), std::string(64, 'c')});
|
||||
if (spare_capacity)
|
||||
{
|
||||
j.push_back(std::string(64, 'd'));
|
||||
j.erase(3);
|
||||
}
|
||||
return json_t(std::move(j));
|
||||
};
|
||||
|
||||
for (const bool at_end :
|
||||
{
|
||||
false, true
|
||||
})
|
||||
{
|
||||
CAPTURE(at_end)
|
||||
|
||||
SECTION("insert(pos, const basic_json&)")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::strong, make, [&](json_t& j)
|
||||
{
|
||||
j.insert(at_end ? j.end() : j.begin() + 1, val);
|
||||
});
|
||||
}
|
||||
|
||||
SECTION("insert(pos, basic_json&&)")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::strong, make, [&](json_t& j)
|
||||
{
|
||||
j.insert(at_end ? j.end() : j.begin() + 1, json_t(val));
|
||||
});
|
||||
}
|
||||
|
||||
SECTION("insert(pos, size_type, const basic_json&)")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::basic, make, [&](json_t& j)
|
||||
{
|
||||
j.insert(at_end ? j.end() : j.begin() + 1, 3, val);
|
||||
});
|
||||
}
|
||||
|
||||
SECTION("insert(pos, const_iterator, const_iterator)")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::basic, make, [&](json_t& j)
|
||||
{
|
||||
j.insert(at_end ? j.end() : j.begin() + 1, source.begin(), source.end());
|
||||
});
|
||||
}
|
||||
|
||||
SECTION("insert(pos, initializer_list_t)")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::basic, make, [&](json_t& j)
|
||||
{
|
||||
j.insert(at_end ? j.end() : j.begin() + 1, {val, val});
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
SECTION("insert(const_iterator, const_iterator) for objects")
|
||||
{
|
||||
const json_t members = {{"m", std::string(64, 'm')}, {"n", std::string(64, 'n')}, {"o", std::string(64, 'o')}};
|
||||
|
||||
SECTION("no member exists yet")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::basic, []()
|
||||
{
|
||||
return json_t({{"a", 1}});
|
||||
}, [&](json_t& j)
|
||||
{
|
||||
j.insert(members.begin(), members.end());
|
||||
});
|
||||
}
|
||||
|
||||
SECTION("some members exist already")
|
||||
{
|
||||
check_exception_guarantee<json_t>(guarantee::basic, []()
|
||||
{
|
||||
return json_t({{"a", 1}, {"n", std::string(64, 'x')}});
|
||||
}, [&](json_t& j)
|
||||
{
|
||||
j.insert(members.begin(), members.end());
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
} // namespace
|
||||
|
||||
TEST_CASE("insert() provides its documented exception guarantees (#5986)")
|
||||
{
|
||||
SECTION("std::map-backed object_t")
|
||||
{
|
||||
check_insert_exception_guarantees<nlohmann::json::with_allocator_t<nth_alloc_fails_allocator>>();
|
||||
}
|
||||
|
||||
SECTION("ordered_map-backed object_t")
|
||||
{
|
||||
check_insert_exception_guarantees<nlohmann::ordered_json::with_allocator_t<nth_alloc_fails_allocator>>();
|
||||
}
|
||||
}
|
||||
#endif
|
||||
Reference in new issue
Block a user