[libcxx-commits] [libcxx] 272757d - [libc++] Don't claim strong emplace guarantee for vector/deque of throwing-move types (#229686)
via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Oct 18 04:21:24 PDT 2026
Author: Michiel van Slobbe
Date: 2026-10-18T12:21:18+01:00
New Revision: 272757d928047af326705f349d469ac02a8a3dca
URL: https://github.com/llvm/llvm-project/commit/272757d928047af326705f349d469ac02a8a3dca
DIFF: https://github.com/llvm/llvm-project/commit/272757d928047af326705f349d469ac02a8a3dca.diff
LOG: [libc++] Don't claim strong emplace guarantee for vector/deque of throwing-move types (#229686)
`__container_traits<vector>` and `__container_traits<deque>` set
`__emplacement_has_strong_exception_safety_guarantee` whenever `T` is
Cpp17CopyInsertable. [vector.modifiers]/2 and [deque.modifiers]/3 only
guarantee "no effects" for an insertion at the end (or at either end for
deque). An insertion in the middle move-constructs and move-assigns
existing elements, and exceptions from those operations are excluded
from the guarantee.
The flat containers use this trait in `__emplace_exact_pos` to decide
whether to clear themselves when an emplacement throws. Because the
trait
is too optimistic, they skip the clear and are left in a state that
breaks their invariants
This patch reports the strong guarantee only when `T` is nothrow move
constructible and nothrow move assignable. In that case the only
operations that can throw (allocation and constructing the new element)
run before the container is modified, so "no effects" really holds.
Behavior change: for element types without nothrow moves (copy-only
types, types with a non-noexcept move, `std::pmr::string`), a flat
container is now cleared when an insertion throws, including for
`std::bad_alloc`. Previously such an insertion could either leave the
container unchanged or corrupt it, depending on where the exception came
from. Clearing is what [flat.map.overview] permits. The non-throwing
path
is unchanged.
An AI coding assistant was used to help build the fuzzer harness which discovered the issue and draft the patch.
Added:
libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp
libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp
Modified:
libcxx/include/__vector/container_traits.h
libcxx/include/deque
libcxx/test/libcxx/containers/container_traits.compile.pass.cpp
Removed:
################################################################################
diff --git a/libcxx/include/__vector/container_traits.h b/libcxx/include/__vector/container_traits.h
index 20f2b9ef042cbd..16711682e08fff 100644
--- a/libcxx/include/__vector/container_traits.h
+++ b/libcxx/include/__vector/container_traits.h
@@ -11,9 +11,8 @@
#include <__config>
#include <__fwd/vector.h>
-#include <__memory/allocator_traits.h>
#include <__type_traits/container_traits.h>
-#include <__type_traits/disjunction.h>
+#include <__type_traits/is_nothrow_assignable.h>
#include <__type_traits/is_nothrow_constructible.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -30,8 +29,12 @@ struct __container_traits<vector<_Tp, _Allocator> > {
// inserting a single element at the end and T is Cpp17CopyInsertable or is_nothrow_move_constructible_v<T> is true,
// there are no effects. Otherwise, if an exception is thrown by the move constructor of a non-Cpp17CopyInsertable T,
// the effects are unspecified.
+ //
+ // The "no effects" guarantee for an insertion at the end does not cover an insertion in the middle, which
+ // move-constructs and move-assigns existing elements. Emplacement at an arbitrary position therefore has the
+ // strong guarantee only when neither of those operations can throw.
static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
- is_nothrow_move_constructible<_Tp>::value || __is_cpp17_copy_insertable_v<_Allocator>;
+ is_nothrow_move_constructible<_Tp>::value && is_nothrow_move_assignable<_Tp>::value;
static _LIBCPP_CONSTEXPR const bool __reservable = true;
};
diff --git a/libcxx/include/deque b/libcxx/include/deque
index ba4d00f81e95e7..c42e285f30f14d 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -2525,8 +2525,12 @@ struct __container_traits<deque<_Tp, _Allocator> > {
// assignment operator of T, there are no effects. If an exception is thrown while inserting a single element at
// either end, there are no effects. Otherwise, if an exception is thrown by the move constructor of a
// non-Cpp17CopyInsertable T, the effects are unspecified.
+ //
+ // The "no effects" guarantee for an insertion at the end does not cover an insertion in the middle, which
+ // move-constructs and move-assigns existing elements. Emplacement at an arbitrary position therefore has the
+ // strong guarantee only when neither of those operations can throw.
static _LIBCPP_CONSTEXPR const bool __emplacement_has_strong_exception_safety_guarantee =
- is_nothrow_move_constructible<_Tp>::value || __is_cpp17_copy_insertable_v<_Allocator>;
+ is_nothrow_move_constructible<_Tp>::value && is_nothrow_move_assignable<_Tp>::value;
static _LIBCPP_CONSTEXPR const bool __reservable = false;
};
diff --git a/libcxx/test/libcxx/containers/container_traits.compile.pass.cpp b/libcxx/test/libcxx/containers/container_traits.compile.pass.cpp
index 1452bfbaf3960e..ba1894a33cb4ce 100644
--- a/libcxx/test/libcxx/containers/container_traits.compile.pass.cpp
+++ b/libcxx/test/libcxx/containers/container_traits.compile.pass.cpp
@@ -77,14 +77,14 @@ void test() {
check<true, std::deque<int> >();
check<true, std::deque<int, test_allocator<int> > >();
- check<true, std::deque<MoveOnly> >();
- check<true, std::deque<ThrowOnMove> >();
+ check<false, std::deque<MoveOnly> >();
+ check<false, std::deque<ThrowOnMove> >();
check<false, std::deque<NonCopyThrowOnMove> >();
check<true, std::vector<int> >();
check<true, std::vector<int, test_allocator<int> > >();
- check<true, std::vector<MoveOnly> >();
- check<true, std::vector<ThrowOnMove> >();
+ check<false, std::vector<MoveOnly> >();
+ check<false, std::vector<ThrowOnMove> >();
check<false, std::vector<NonCopyThrowOnMove> >();
check<true, std::set<int> >();
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp
new file mode 100644
index 00000000000000..66d5bd1f1c8d3d
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp
@@ -0,0 +1,164 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: no-exceptions
+
+// <flat_map>
+
+// Test that the invariants are restored when inserting an element in the middle of the
+// underlying containers throws from the element's assignment operator or, during
+// reallocation, from its copy constructor. Neither has a "no effects" guarantee for an
+// insertion in the middle of a vector or deque, even if the element type is
+// Cpp17CopyInsertable.
+
+#include <cassert>
+#include <compare>
+#include <deque>
+#include <flat_map>
+#include <functional>
+#include <type_traits>
+#include <vector>
+
+#include "test_macros.h"
+#include "../helpers.h"
+
+enum class ThrowMode { Never, OnAssign, OnNthCopy };
+
+struct ThrowState {
+ ThrowMode mode = ThrowMode::Never;
+ int copies_until_throw = 0; // Only used with ThrowMode::OnNthCopy.
+};
+
+struct ThrowingAssign {
+ static constexpr ThrowMode throw_mode = ThrowMode::OnAssign;
+
+ int value;
+ ThrowState* state;
+
+ ThrowingAssign(int v, ThrowState& s) : value(v), state(&s) {}
+ ThrowingAssign(const ThrowingAssign&) = default;
+ ThrowingAssign(ThrowingAssign&&) noexcept = default;
+ ThrowingAssign& operator=(const ThrowingAssign& other) {
+ if (state->mode == ThrowMode::OnAssign)
+ throw 42;
+ value = other.value;
+ return *this;
+ }
+ ThrowingAssign& operator=(ThrowingAssign&& other) {
+ if (state->mode == ThrowMode::OnAssign)
+ throw 42;
+ value = other.value;
+ return *this;
+ }
+
+ friend bool operator==(const ThrowingAssign& x, const ThrowingAssign& y) { return x.value == y.value; }
+ friend auto operator<=>(const ThrowingAssign& x, const ThrowingAssign& y) { return x.value <=> y.value; }
+};
+
+struct ThrowingCopy {
+ static constexpr ThrowMode throw_mode = ThrowMode::OnNthCopy;
+
+ int value;
+ ThrowState* state;
+
+ ThrowingCopy(int v, ThrowState& s) : value(v), state(&s) {}
+ ThrowingCopy(const ThrowingCopy& other) : value(other.value), state(other.state) {
+ if (state->mode == ThrowMode::OnNthCopy && state->copies_until_throw-- == 0)
+ throw 42;
+ }
+ ThrowingCopy(ThrowingCopy&& other) : ThrowingCopy(other) {}
+ ThrowingCopy& operator=(const ThrowingCopy&) noexcept = default;
+ ThrowingCopy& operator=(ThrowingCopy&&) noexcept = default;
+
+ friend bool operator==(const ThrowingCopy& x, const ThrowingCopy& y) { return x.value == y.value; }
+ friend auto operator<=>(const ThrowingCopy& x, const ThrowingCopy& y) { return x.value <=> y.value; }
+};
+
+template <class T>
+T make_element(int v, ThrowState& state) {
+ if constexpr (std::is_same_v<T, int>)
+ return v;
+ else
+ return T(v, state);
+}
+
+template <class Container>
+Container make_container(std::initializer_list<int> il, ThrowState& state) {
+ using T = typename Container::value_type;
+ Container c;
+ for (int v : il)
+ c.push_back(make_element<T>(v, state));
+ if constexpr (requires { c.reserve(8); }) {
+ // ThrowingAssign: spare capacity makes vector insert in place.
+ // ThrowingCopy: no spare capacity makes vector reallocate.
+ if constexpr (std::is_same_v<T, ThrowingCopy>)
+ c.shrink_to_fit();
+ else
+ c.reserve(8);
+ }
+ return c;
+}
+
+template <class KeyContainer, class ValueContainer, class F>
+void test_one(F insert_function) {
+ using Key = typename KeyContainer::value_type;
+ using Value = typename ValueContainer::value_type;
+ using M = std::flat_map<Key, Value, std::less<Key>, KeyContainer, ValueContainer>;
+ using Throwing = std::conditional_t<std::is_same_v<Key, int>, Value, Key>;
+
+ // For ThrowingCopy, throw on each of the first few copies in turn.
+ for (int n = 0; n < 6; ++n) {
+ ThrowState state;
+ M m(std::sorted_unique,
+ make_container<KeyContainer>({1, 3, 5}, state),
+ make_container<ValueContainer>({10, 30, 50}, state));
+ Key key = make_element<Key>(2, state);
+ Value value = make_element<Value>(20, state);
+
+ state.mode = Throwing::throw_mode;
+ state.copies_until_throw = n;
+ try {
+ insert_function(m, key, value);
+ } catch (int) {
+ }
+ state.mode = ThrowMode::Never;
+ check_invariant(m);
+ }
+}
+
+template <class KeyContainer, class ValueContainer>
+void test() {
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) { m.emplace(k, v); });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) {
+ m.emplace_hint(m.begin() + 1, k, v);
+ });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) { m.try_emplace(k, v); });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) {
+ m.try_emplace(m.begin() + 1, k, v);
+ });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) { m.insert({k, v}); });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) {
+ m.insert(m.begin() + 1, {k, v});
+ });
+ test_one<KeyContainer, ValueContainer>([](auto& m, const auto& k, const auto& v) { m.insert_or_assign(k, v); });
+}
+
+int main(int, char**) {
+ // Throw while inserting the key.
+ test<std::vector<ThrowingAssign>, std::vector<int>>();
+ test<std::deque<ThrowingAssign>, std::vector<int>>();
+ // Throw while inserting the mapped value.
+ test<std::vector<int>, std::vector<ThrowingAssign>>();
+ test<std::vector<int>, std::deque<ThrowingAssign>>();
+ // Throw while reallocating.
+ test<std::vector<ThrowingCopy>, std::vector<int>>();
+ test<std::vector<int>, std::vector<ThrowingCopy>>();
+
+ return 0;
+}
diff --git a/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp
new file mode 100644
index 00000000000000..d78ea8a911dada
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp
@@ -0,0 +1,89 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: no-exceptions
+
+// <flat_set>
+
+// Test that the invariants are restored when inserting an element in the middle of the
+// underlying container throws from the element's assignment operator. An insertion in the
+// middle of a vector or deque has no "no effects" guarantee in that case, even if the
+// element type is Cpp17CopyInsertable.
+
+#include <cassert>
+#include <compare>
+#include <deque>
+#include <flat_set>
+#include <functional>
+#include <vector>
+
+#include "test_macros.h"
+#include "../helpers.h"
+
+struct ThrowingAssign {
+ int value;
+ bool* throw_on_assign;
+
+ ThrowingAssign(int v, bool& t) : value(v), throw_on_assign(&t) {}
+ ThrowingAssign(const ThrowingAssign&) = default;
+ ThrowingAssign(ThrowingAssign&&) noexcept = default;
+ ThrowingAssign& operator=(const ThrowingAssign& other) {
+ if (*throw_on_assign)
+ throw 42;
+ value = other.value;
+ return *this;
+ }
+ ThrowingAssign& operator=(ThrowingAssign&& other) {
+ if (*throw_on_assign)
+ throw 42;
+ value = other.value;
+ return *this;
+ }
+
+ friend bool operator==(const ThrowingAssign& x, const ThrowingAssign& y) { return x.value == y.value; }
+ friend auto operator<=>(const ThrowingAssign& x, const ThrowingAssign& y) { return x.value <=> y.value; }
+};
+
+template <class KeyContainer, class F>
+void test_one(F insert_function) {
+ using M = std::flat_set<ThrowingAssign, std::less<ThrowingAssign>, KeyContainer>;
+
+ bool throw_on_assign = false;
+ KeyContainer keys;
+ // Spare capacity makes vector insert in place instead of reallocating.
+ if constexpr (requires { keys.reserve(8); })
+ keys.reserve(8);
+ for (int v : {1, 3, 5})
+ keys.emplace_back(v, throw_on_assign);
+ M m(std::sorted_unique, std::move(keys));
+
+ throw_on_assign = true;
+ try {
+ insert_function(m, throw_on_assign);
+ assert(false);
+ } catch (int) {
+ }
+ throw_on_assign = false;
+ check_invariant(m);
+}
+
+template <class KeyContainer>
+void test() {
+ test_one<KeyContainer>([](auto& m, bool& t) { m.emplace(2, t); });
+ test_one<KeyContainer>([](auto& m, bool& t) { m.emplace_hint(m.begin() + 1, 2, t); });
+ test_one<KeyContainer>([](auto& m, bool& t) { m.insert(ThrowingAssign(2, t)); });
+ test_one<KeyContainer>([](auto& m, bool& t) { m.insert(m.begin() + 1, ThrowingAssign(2, t)); });
+}
+
+int main(int, char**) {
+ test<std::vector<ThrowingAssign>>();
+ test<std::deque<ThrowingAssign>>();
+
+ return 0;
+}
More information about the libcxx-commits
mailing list