Skip to content

Commit ec5610c

Browse files
winner245winner245
authored
[libc++] Ensure strong exception guarantee for forward_list::resize (#131025)
The current implementation of `forward_list::resize` does not meet the strong exception safety guarantee required by [forward.list.modifiers]: If an exception is thrown by any of these member functions there is no effect on the container. This patch refactors `resize()` to provide strong exception safety and introduces additional tests to validate the strong exception guarantees for other `forward_list` modifiers. Fixes #118366.
1 parent e03044f commit ec5610c

File tree

4 files changed

+120
-40
lines changed

4 files changed

+120
-40
lines changed

‎libcxx/docs/Status/Cxx2cIssues.csv‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@
107107
"`LWG4153 <https://wg21.link/LWG4153>`__","Fix extra ""-1"" for ``philox_engine::max()``","2024-11 (Wrocław)","","",""
108108
"`LWG4154 <https://wg21.link/LWG4154>`__","The Mandates for ``std::packaged_task``'s constructor from a callable entity should consider decaying","2024-11 (Wrocław)","","",""
109109
"`LWG4157 <https://wg21.link/LWG4157>`__","The resolution of LWG3465 was damaged by P2167R3","2024-11 (Wrocław)","","20",""
110-
"`LWG4164 <https://wg21.link/LWG4164>`__","Missing guarantees for ``forward_list`` modifiers","2024-11 (Wrocław)","","",""
110+
"`LWG4164 <https://wg21.link/LWG4164>`__","Missing guarantees for ``forward_list`` modifiers","2024-11 (Wrocław)","|Complete|","21",""
111111
"`LWG4169 <https://wg21.link/LWG4169>`__","``std::atomic<T>``'s default constructor should be constrained","2024-11 (Wrocław)","","",""
112112
"`LWG4170 <https://wg21.link/LWG4170>`__","``contiguous_iterator`` should require ``to_address(I{})``","2024-11 (Wrocław)","","",""
113113
"","","","","",""

‎libcxx/include/forward_list‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,9 @@ public:
806806
}
807807
# endif // _LIBCPP_CXX03_LANG
808808
_LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, const value_type& __v);
809-
_LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, size_type __n, const value_type& __v);
809+
_LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, size_type __n, const value_type& __v) {
810+
return __insert_after(__p, __n, __v);
811+
}
810812
template <class _InputIterator, __enable_if_t<__has_input_iterator_category<_InputIterator>::value, int> = 0>
811813
_LIBCPP_HIDE_FROM_ABI iterator insert_after(const_iterator __p, _InputIterator __f, _InputIterator __l);
812814

@@ -876,6 +878,9 @@ private:
876878
template <class _Iter, class _Sent>
877879
_LIBCPP_HIDE_FROM_ABI void __assign_with_sentinel(_Iter __f, _Sent __l);
878880

881+
template <class... _Args>
882+
_LIBCPP_HIDE_FROM_ABI iterator __insert_after(const_iterator __p, size_type __n, _Args&&... __args);
883+
879884
template <class _Compare>
880885
static _LIBCPP_HIDE_FROM_ABI __node_pointer __merge(__node_pointer __f1, __node_pointer __f2, _Compare& __comp);
881886

@@ -1129,17 +1134,18 @@ forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, const value_type& __
11291134
}
11301135

11311136
template <class _Tp, class _Alloc>
1137+
template <class... _Args>
11321138
typename forward_list<_Tp, _Alloc>::iterator
1133-
forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, size_type __n, const value_type& __v) {
1139+
forward_list<_Tp, _Alloc>::__insert_after(const_iterator __p, size_type __n, _Args&&... __args) {
11341140
__begin_node_pointer __r = __p.__get_begin();
11351141
if (__n > 0) {
1136-
__node_pointer __first = this->__create_node(/* next = */ nullptr, __v);
1142+
__node_pointer __first = this->__create_node(/* next = */ nullptr, std::forward<_Args>(__args)...);
11371143
__node_pointer __last = __first;
11381144
# if _LIBCPP_HAS_EXCEPTIONS
11391145
try {
11401146
# endif // _LIBCPP_HAS_EXCEPTIONS
11411147
for (--__n; __n != 0; --__n, __last = __last->__next_) {
1142-
__last->__next_ = this->__create_node(/* next = */ nullptr, __v);
1148+
__last->__next_ = this->__create_node(/* next = */ nullptr, std::forward<_Args>(__args)...);
11431149
}
11441150
# if _LIBCPP_HAS_EXCEPTIONS
11451151
} catch (...) {
@@ -1239,14 +1245,8 @@ void forward_list<_Tp, _Alloc>::resize(size_type __n) {
12391245
;
12401246
if (__i != __e)
12411247
erase_after(__p, __e);
1242-
else {
1243-
__n -= __sz;
1244-
if (__n > 0) {
1245-
for (__begin_node_pointer __ptr = __p.__get_begin(); __n > 0; --__n, __ptr = __ptr->__next_as_begin()) {
1246-
__ptr->__next_ = this->__create_node(/* next = */ nullptr);
1247-
}
1248-
}
1249-
}
1248+
else
1249+
__insert_after(__p, __n - __sz);
12501250
}
12511251

12521252
template <class _Tp, class _Alloc>
@@ -1259,14 +1259,8 @@ void forward_list<_Tp, _Alloc>::resize(size_type __n, const value_type& __v) {
12591259
;
12601260
if (__i != __e)
12611261
erase_after(__p, __e);
1262-
else {
1263-
__n -= __sz;
1264-
if (__n > 0) {
1265-
for (__begin_node_pointer __ptr = __p.__get_begin(); __n > 0; --__n, __ptr = __ptr->__next_as_begin()) {
1266-
__ptr->__next_ = this->__create_node(/* next = */ nullptr, __v);
1267-
}
1268-
}
1269-
}
1262+
else
1263+
__insert_after(__p, __n - __sz, __v);
12701264
}
12711265

12721266
template <class _Tp, class _Alloc>

‎libcxx/test/std/containers/exception_safety_helpers.h‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include <cassert>
1313
#include <cstddef>
14+
#include <forward_list>
1415
#include <functional>
1516
#include <utility>
1617
#include "test_macros.h"
@@ -53,6 +54,33 @@ int ThrowingCopy<N>::created_by_copying = 0;
5354
template <int N>
5455
int ThrowingCopy<N>::destroyed = 0;
5556

57+
template <int N>
58+
struct ThrowingDefault {
59+
static bool throwing_enabled;
60+
static int default_constructed;
61+
static int destroyed;
62+
int x = 0;
63+
64+
ThrowingDefault() {
65+
++default_constructed;
66+
if (throwing_enabled && default_constructed == N) {
67+
throw -1;
68+
}
69+
}
70+
71+
ThrowingDefault(int value) : x(value) {}
72+
ThrowingDefault(const ThrowingDefault& other) = default;
73+
friend bool operator==(const ThrowingDefault& lhs, const ThrowingDefault& rhs) { return lhs.x == rhs.x; }
74+
friend bool operator<(const ThrowingDefault& lhs, const ThrowingDefault& rhs) { return lhs.x < rhs.x; }
75+
76+
static void reset() { default_constructed = destroyed = 0; }
77+
};
78+
79+
template <int N>
80+
bool ThrowingDefault<N>::throwing_enabled = true;
81+
template <int N>
82+
int ThrowingDefault<N>::default_constructed = 0;
83+
5684
template <int N>
5785
struct std::hash<ThrowingCopy<N>> {
5886
std::size_t operator()(const ThrowingCopy<N>& value) const { return value.x; }
@@ -95,6 +123,29 @@ void test_exception_safety_throwing_copy_container(Func&& func) {
95123
}
96124
}
97125

126+
template <int ThrowOn, int Size, class Func>
127+
void test_strong_exception_safety_throwing_copy(Func&& func) {
128+
using T = ThrowingCopy<ThrowOn>;
129+
T::throwing_enabled = false;
130+
131+
std::forward_list<T> c0(Size);
132+
for (int i = 0; i < Size; ++i)
133+
c0.emplace_front(i);
134+
std::forward_list<T> c = c0;
135+
T in[Size];
136+
T::reset();
137+
T::throwing_enabled = true;
138+
try {
139+
func(c, in, in + Size);
140+
assert(false); // The function call above should throw.
141+
142+
} catch (int) {
143+
assert(T::created_by_copying == ThrowOn);
144+
assert(T::destroyed == ThrowOn - 1); // No destructor call for the partially-constructed element.
145+
assert(c == c0); // Strong exception guarantee
146+
}
147+
}
148+
98149
#endif // !defined(TEST_HAS_NO_EXCEPTIONS)
99150

100151
#endif // SUPPORT_EXCEPTION_SAFETY_HELPERS_H

‎libcxx/test/std/containers/sequences/forwardlist/exception_safety.pass.cpp‎

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,23 @@
3535
// void assign_range(R&& rg); // C++23
3636
//
3737
// void push_front(const value_type& v);
38-
// template<container-compatible-range<T> R>
38+
// template <class... Args> reference emplace_front(Args&&... args); // reference in C++17
39+
// template<container-compatible-range<T> R>
3940
// void prepend_range(R&& rg); // C++23
4041
//
4142
// iterator insert_after(const_iterator p, const value_type& v);
43+
// iterator insert_after(const_iterator p, value_type&& v);
4244
// iterator insert_after(const_iterator p, size_type n, const value_type& v);
45+
// iterator insert_after(const_iterator p, initializer_list<value_type> il);
4346
// template <class InputIterator>
4447
// iterator insert_after(const_iterator p,
4548
// InputIterator first, InputIterator last);
46-
// template<container-compatible-range<T> R>
49+
// template<container-compatible-range<T> R>
4750
// iterator insert_range_after(const_iterator position, R&& rg); // C++23
48-
//
51+
// template <class... Args>
52+
// iterator emplace_after(const_iterator p, Args&&... args);
4953
// void resize(size_type n, const value_type& v);
54+
// void resize(size_type n);
5055

5156
#include <forward_list>
5257

@@ -65,16 +70,33 @@ int main(int, char**) {
6570
using T = ThrowingCopy<ThrowOn>;
6671

6772
// void push_front(const value_type& v);
68-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
69-
std::forward_list<T> c;
73+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
74+
c.push_front(*from);
75+
});
76+
77+
// template <class... Args> reference emplace_front(Args&&... args);
78+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
7079
c.push_front(*from);
7180
});
7281

7382
// iterator insert_after(const_iterator p, const value_type& v);
74-
test_exception_safety_throwing_copy</*ThrowOn=*/1, Size>([](T* from, T*) {
75-
std::forward_list<T> c;
83+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
7684
c.insert_after(c.before_begin(), *from);
7785
});
86+
87+
// iterator insert_after(const_iterator p, value_type&& v);
88+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
89+
c.insert_after(c.before_begin(), std::move(*from));
90+
});
91+
92+
// template <class... Args>
93+
// iterator emplace_after(const_iterator p, Args&&... args);
94+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
95+
c.emplace_after(c.before_begin(), *from);
96+
});
97+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
98+
c.emplace_after(c.before_begin(), std::move(*from));
99+
});
78100
}
79101

80102
{
@@ -169,40 +191,53 @@ int main(int, char**) {
169191
#if TEST_STD_VER >= 23
170192
// template<container-compatible-range<T> R>
171193
// void prepend_range(R&& rg); // C++23
172-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
173-
std::forward_list<T> c;
194+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
174195
c.prepend_range(std::ranges::subrange(from, to));
175196
});
176197
#endif
177198

178199
// iterator insert_after(const_iterator p, size_type n, const value_type& v);
179-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
180-
std::forward_list<T> c;
200+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
181201
c.insert_after(c.before_begin(), Size, *from);
182202
});
183203

184204
// template <class InputIterator>
185205
// iterator insert_after(const_iterator p,
186206
// InputIterator first, InputIterator last);
187-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
188-
std::forward_list<T> c;
207+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
189208
c.insert_after(c.before_begin(), from, to);
190209
});
191210

211+
// iterator insert_after(const_iterator p, initializer_list<value_type> il);
212+
std::initializer_list<T> il{1, 2, 3, 4, 5};
213+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([&](std::forward_list<T>& c, T*, T*) {
214+
c.insert_after(c.before_begin(), il);
215+
});
216+
192217
#if TEST_STD_VER >= 23
193218
// template<container-compatible-range<T> R>
194219
// iterator insert_range_after(const_iterator position, R&& rg); // C++23
195-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
196-
std::forward_list<T> c;
220+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
197221
c.insert_range_after(c.before_begin(), std::ranges::subrange(from, to));
198222
});
199223
#endif
200224

201225
// void resize(size_type n, const value_type& v);
202-
test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
203-
std::forward_list<T> c;
204-
c.resize(Size, *from);
205-
});
226+
test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
227+
c.resize(Size * 3, *from);
228+
});
229+
230+
{ // void resize(size_type n);
231+
using X = ThrowingDefault<ThrowOn>;
232+
std::forward_list<X> c0{X{1}, X{2}, X{3}};
233+
std::forward_list<X> c = c0;
234+
try {
235+
c.resize(3 * ThrowOn);
236+
assert(false);
237+
} catch (int) {
238+
assert(c == c0);
239+
}
240+
}
206241
}
207242

208243
return 0;

0 commit comments

Comments
 (0)