[libcxx-commits] [libcxx] [libc++] Don't claim strong emplace guarantee for vector/deque of throwing-move types (PR #229686)
via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Oct 7 00:57:03 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Michiel van Slobbe (mvanslobbe)
<details>
<summary>Changes</summary>
`__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:
```cpp
#include <flat_map>
#include <vector>
#include <cstdio>
static bool armed = false;
struct Key {
int v;
Key(int x) : v(x) {}
Key(const Key&) = default;
Key& operator=(const Key& o) { if (armed) throw 42; v = o.v; return *this; }
bool operator<(const Key& o) const { return v < o.v; }
};
int main() {
std::vector<Key> ks{1, 3}; ks.reserve(8);
std::vector<int> vs{10, 30}; vs.reserve(8);
std::flat_map<Key, int> m(std::sorted_unique, std::move(ks), std::move(vs));
armed = true;
try { m.emplace(2, 20); } catch (int) {}
std::printf("keys=%zu values=%zu\n", m.keys().size(), m.values().size());
}
```
This prints `keys=3 values=2` (libstdc++ prints `keys=0 values=0`).
Any later iteration reads past the end of `values()`. A throwing copy
constructor during a reallocating middle insert causes the same problem
the other way around (`keys=2 values=3`), and `flat_set` ends up with
duplicate keys.
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.
Testing:
- New `flat.map.modifiers/emplace_throwing_element.pass.cpp` and
`flat.set.modifiers/emplace_throwing_element.pass.cpp`. Both fail
without this patch and pass with it, and both pass on libstdc++.
- `container_traits.compile.pass.cpp` now expects `false` for
`vector`/`deque` of `MoveOnly` and `ThrowOnMove`.
- Also drops two unused includes from `__vector/container_traits.h`.
Found with a libFuzzer harness that checks `std::flat_map` against
`std::map` while injecting exceptions. I used an AI coding assistant to
help build the harness and draft the patch; I reviewed and tested the
changes.
---
Full diff: https://github.com/llvm/llvm-project/pull/229686.diff
5 Files Affected:
- (modified) libcxx/include/__vector/container_traits.h (+6-3)
- (modified) libcxx/include/deque (+5-1)
- (modified) libcxx/test/libcxx/containers/container_traits.compile.pass.cpp (+4-4)
- (added) libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp (+136)
- (added) libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp (+88)
``````````diff
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..72c763407857d1
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/emplace_throwing_element.pass.cpp
@@ -0,0 +1,136 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 <deque>
+#include <flat_map>
+#include <functional>
+#include <type_traits>
+#include <vector>
+
+#include "test_macros.h"
+#include "../helpers.h"
+
+struct ThrowingAssign {
+ static inline bool throw_on_assign = false;
+
+ int value;
+
+ ThrowingAssign(int v) : value(v) {}
+ 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&, const ThrowingAssign&) = default;
+ friend auto operator<=>(const ThrowingAssign&, const ThrowingAssign&) = default;
+};
+
+struct ThrowingCopy {
+ static inline int copies_until_throw = -1; // -1: never throw
+
+ int value;
+
+ ThrowingCopy(int v) : value(v) {}
+ ThrowingCopy(const ThrowingCopy& other) : value(other.value) {
+ if (copies_until_throw >= 0 && 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&, const ThrowingCopy&) = default;
+ friend auto operator<=>(const ThrowingCopy&, const ThrowingCopy&) = default;
+};
+
+// n >= 0: assignments throw, and the n-th copy (counting from 0) throws. n < 0: nothing throws.
+void set_throwing(int n) {
+ ThrowingAssign::throw_on_assign = n >= 0;
+ ThrowingCopy::copies_until_throw = n;
+}
+
+template <class Container>
+Container make_container(std::initializer_list<int> il) {
+ Container c;
+ for (int v : il)
+ c.emplace_back(v);
+ 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<typename Container::value_type, 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>;
+
+ for (int n = 0; n < 6; ++n) {
+ M m(std::sorted_unique, make_container<KeyContainer>({1, 3, 5}), make_container<ValueContainer>({10, 30, 50}));
+ set_throwing(n);
+ try {
+ insert_function(m);
+ } catch (int) {
+ }
+ set_throwing(-1);
+ check_invariant(m);
+ }
+}
+
+template <class KeyContainer, class ValueContainer>
+void test() {
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.emplace(2, 20); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.emplace_hint(m.begin() + 1, 2, 20); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.try_emplace(2, 20); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.try_emplace(m.begin() + 1, 2, 20); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.insert({2, 20}); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.insert(m.begin() + 1, {2, 20}); });
+ test_one<KeyContainer, ValueContainer>([](auto& m) { m.insert_or_assign(2, 20); });
+}
+
+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..99ec437d5a173b
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.set/flat.set.modifiers/emplace_throwing_element.pass.cpp
@@ -0,0 +1,88 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 <deque>
+#include <flat_set>
+#include <functional>
+#include <vector>
+
+#include "test_macros.h"
+#include "../helpers.h"
+
+struct ThrowingAssign {
+ static inline bool throw_on_assign = false;
+
+ int value;
+
+ ThrowingAssign(int v) : value(v) {}
+ 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&, const ThrowingAssign&) = default;
+ friend auto operator<=>(const ThrowingAssign&, const ThrowingAssign&) = default;
+};
+
+template <class KeyContainer, class F>
+void test_one(F insert_function) {
+ using M = std::flat_set<ThrowingAssign, std::less<ThrowingAssign>, KeyContainer>;
+
+ 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);
+ M m(std::sorted_unique, std::move(keys));
+
+ ThrowingAssign::throw_on_assign = true;
+ try {
+ insert_function(m);
+ assert(false);
+ } catch (int) {
+ }
+ ThrowingAssign::throw_on_assign = false;
+ check_invariant(m);
+}
+
+template <class KeyContainer>
+void test() {
+ test_one<KeyContainer>([](auto& m) { m.emplace(2); });
+ test_one<KeyContainer>([](auto& m) { m.emplace_hint(m.begin() + 1, 2); });
+ test_one<KeyContainer>([](auto& m) { m.insert(ThrowingAssign(2)); });
+ test_one<KeyContainer>([](auto& m) { m.insert(m.begin() + 1, ThrowingAssign(2)); });
+}
+
+int main(int, char**) {
+ test<std::vector<ThrowingAssign>>();
+ test<std::deque<ThrowingAssign>>();
+
+ return 0;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/229686
More information about the libcxx-commits
mailing list