[libcxx-commits] [libcxx] [libc++] Ensure strong exception guarantee for forward_list::resize (PR #131025)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Wed Mar 12 13:57:39 PDT 2025


https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/131025

>From fc7e931832bb7d44ae2d071b1057fc5a57d85236 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 12 Mar 2025 09:27:18 -0400
Subject: [PATCH] Provide strong exception guarantee for forward_list::resize

---
 libcxx/docs/Status/Cxx2cIssues.csv            |  2 +-
 libcxx/include/forward_list                   | 45 +++++++++---
 .../std/containers/exception_safety_helpers.h | 51 +++++++++++++
 .../forwardlist/exception_safety.pass.cpp     | 73 ++++++++++++++-----
 4 files changed, 139 insertions(+), 32 deletions(-)

diff --git a/libcxx/docs/Status/Cxx2cIssues.csv b/libcxx/docs/Status/Cxx2cIssues.csv
index 9bf31c417f3c9..fdf381862d87b 100644
--- a/libcxx/docs/Status/Cxx2cIssues.csv
+++ b/libcxx/docs/Status/Cxx2cIssues.csv
@@ -107,7 +107,7 @@
 "`LWG4153 <https://wg21.link/LWG4153>`__","Fix extra ""-1"" for ``philox_engine::max()``","2024-11 (Wrocław)","","",""
 "`LWG4154 <https://wg21.link/LWG4154>`__","The Mandates for ``std::packaged_task``'s constructor from a callable entity should consider decaying","2024-11 (Wrocław)","","",""
 "`LWG4157 <https://wg21.link/LWG4157>`__","The resolution of LWG3465 was damaged by P2167R3","2024-11 (Wrocław)","","20",""
-"`LWG4164 <https://wg21.link/LWG4164>`__","Missing guarantees for ``forward_list`` modifiers","2024-11 (Wrocław)","","",""
+"`LWG4164 <https://wg21.link/LWG4164>`__","Missing guarantees for ``forward_list`` modifiers","2024-11 (Wrocław)","|Complete|","21",""
 "`LWG4169 <https://wg21.link/LWG4169>`__","``std::atomic<T>``'s default constructor should be constrained","2024-11 (Wrocław)","","",""
 "`LWG4170 <https://wg21.link/LWG4170>`__","``contiguous_iterator`` should require ``to_address(I{})``","2024-11 (Wrocław)","","",""
 "","","","","",""
diff --git a/libcxx/include/forward_list b/libcxx/include/forward_list
index 7582de20995b9..6cad1f96f7019 100644
--- a/libcxx/include/forward_list
+++ b/libcxx/include/forward_list
@@ -819,6 +819,7 @@ public:
 
   template <class _InputIterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI iterator __insert_after_with_sentinel(const_iterator __p, _InputIterator __f, _Sentinel __l);
+  _LIBCPP_HIDE_FROM_ABI iterator __default_insert_after(const_iterator __p, size_type __n);
 
   _LIBCPP_HIDE_FROM_ABI iterator erase_after(const_iterator __p);
   _LIBCPP_HIDE_FROM_ABI iterator erase_after(const_iterator __f, const_iterator __l);
@@ -1128,6 +1129,36 @@ forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, const value_type& __
   return iterator(__r->__next_);
 }
 
+template <class _Tp, class _Alloc>
+typename forward_list<_Tp, _Alloc>::iterator
+forward_list<_Tp, _Alloc>::__default_insert_after(const_iterator __p, size_type __n) {
+  __begin_node_pointer __r = __p.__get_begin();
+  if (__n > 0) {
+    __node_pointer __first = this->__create_node(/* next = */ nullptr);
+    __node_pointer __last  = __first;
+#  if _LIBCPP_HAS_EXCEPTIONS
+    try {
+#  endif // _LIBCPP_HAS_EXCEPTIONS
+      for (--__n; __n != 0; --__n, __last = __last->__next_) {
+        __last->__next_ = this->__create_node(/* next = */ nullptr);
+      }
+#  if _LIBCPP_HAS_EXCEPTIONS
+    } catch (...) {
+      while (__first != nullptr) {
+        __node_pointer __next = __first->__next_;
+        this->__delete_node(__first);
+        __first = __next;
+      }
+      throw;
+    }
+#  endif // _LIBCPP_HAS_EXCEPTIONS
+    __last->__next_ = __r->__next_;
+    __r->__next_    = __first;
+    __r             = static_cast<__begin_node_pointer>(__last);
+  }
+  return iterator(__r);
+}
+
 template <class _Tp, class _Alloc>
 typename forward_list<_Tp, _Alloc>::iterator
 forward_list<_Tp, _Alloc>::insert_after(const_iterator __p, size_type __n, const value_type& __v) {
@@ -1240,12 +1271,7 @@ void forward_list<_Tp, _Alloc>::resize(size_type __n) {
   if (__i != __e)
     erase_after(__p, __e);
   else {
-    __n -= __sz;
-    if (__n > 0) {
-      for (__begin_node_pointer __ptr = __p.__get_begin(); __n > 0; --__n, __ptr = __ptr->__next_as_begin()) {
-        __ptr->__next_ = this->__create_node(/* next = */ nullptr);
-      }
-    }
+    __default_insert_after(__p, __n - __sz);
   }
 }
 
@@ -1260,12 +1286,7 @@ void forward_list<_Tp, _Alloc>::resize(size_type __n, const value_type& __v) {
   if (__i != __e)
     erase_after(__p, __e);
   else {
-    __n -= __sz;
-    if (__n > 0) {
-      for (__begin_node_pointer __ptr = __p.__get_begin(); __n > 0; --__n, __ptr = __ptr->__next_as_begin()) {
-        __ptr->__next_ = this->__create_node(/* next = */ nullptr, __v);
-      }
-    }
+    insert_after(__p, __n - __sz, __v);
   }
 }
 
diff --git a/libcxx/test/std/containers/exception_safety_helpers.h b/libcxx/test/std/containers/exception_safety_helpers.h
index da247571d303f..2d26735d9bb4e 100644
--- a/libcxx/test/std/containers/exception_safety_helpers.h
+++ b/libcxx/test/std/containers/exception_safety_helpers.h
@@ -11,6 +11,7 @@
 
 #include <cassert>
 #include <cstddef>
+#include <forward_list>
 #include <functional>
 #include <utility>
 #include "test_macros.h"
@@ -53,6 +54,33 @@ int ThrowingCopy<N>::created_by_copying = 0;
 template <int N>
 int ThrowingCopy<N>::destroyed = 0;
 
+template <int N>
+struct ThrowingDefault {
+  static bool throwing_enabled;
+  static int default_constructed;
+  static int destroyed;
+  int x = 0;
+
+  ThrowingDefault() {
+    ++default_constructed;
+    if (throwing_enabled && default_constructed == N) {
+      throw -1;
+    }
+  }
+
+  ThrowingDefault(int value) : x(value) {}
+  ThrowingDefault(const ThrowingDefault& other) = default;
+  friend bool operator==(const ThrowingDefault& lhs, const ThrowingDefault& rhs) { return lhs.x == rhs.x; }
+  friend bool operator<(const ThrowingDefault& lhs, const ThrowingDefault& rhs) { return lhs.x < rhs.x; }
+
+  static void reset() { default_constructed = destroyed = 0; }
+};
+
+template <int N>
+bool ThrowingDefault<N>::throwing_enabled = true;
+template <int N>
+int ThrowingDefault<N>::default_constructed = 0;
+
 template <int N>
 struct std::hash<ThrowingCopy<N>> {
   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) {
   }
 }
 
+template <int ThrowOn, int Size, class Func>
+void test_strong_exception_safety_throwing_copy(Func&& func) {
+  using T             = ThrowingCopy<ThrowOn>;
+  T::throwing_enabled = false;
+
+  std::forward_list<T> c0(Size);
+  for (int i = 0; i < Size; ++i)
+    c0.emplace_front(i);
+  std::forward_list<T> c = c0;
+  T in[Size];
+  T::reset();
+  T::throwing_enabled = true;
+  try {
+    func(c, in, in + Size);
+    assert(false); // The function call above should throw.
+
+  } catch (int) {
+    assert(T::created_by_copying == ThrowOn);
+    assert(T::destroyed == ThrowOn - 1); // No destructor call for the partially-constructed element.
+    assert(c == c0);                     // Strong exception guarantee
+  }
+}
+
 #endif // !defined(TEST_HAS_NO_EXCEPTIONS)
 
 #endif // SUPPORT_EXCEPTION_SAFETY_HELPERS_H
diff --git a/libcxx/test/std/containers/sequences/forwardlist/exception_safety.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/exception_safety.pass.cpp
index faf42202e951a..a6579e144e6d3 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/exception_safety.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/exception_safety.pass.cpp
@@ -35,18 +35,23 @@
 //     void assign_range(R&& rg); // C++23
 //
 // void push_front(const value_type& v);
-//  template<container-compatible-range<T> R>
+// template <class... Args> reference emplace_front(Args&&... args);  // reference in C++17
+// template<container-compatible-range<T> R>
 //    void prepend_range(R&& rg); // C++23
 //
 // iterator insert_after(const_iterator p, const value_type& v);
+// iterator insert_after(const_iterator p, value_type&& v);
 // iterator insert_after(const_iterator p, size_type n, const value_type& v);
+// iterator insert_after(const_iterator p, initializer_list<value_type> il);
 // template <class InputIterator>
 //     iterator insert_after(const_iterator p,
 //                           InputIterator first, InputIterator last);
-//  template<container-compatible-range<T> R>
+// template<container-compatible-range<T> R>
 //     iterator insert_range_after(const_iterator position, R&& rg); // C++23
-//
+// template <class... Args>
+//     iterator emplace_after(const_iterator p, Args&&... args);
 // void resize(size_type n, const value_type& v);
+// void resize(size_type n);
 
 #include <forward_list>
 
@@ -65,16 +70,33 @@ int main(int, char**) {
     using T               = ThrowingCopy<ThrowOn>;
 
     // void push_front(const value_type& v);
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
+      c.push_front(*from);
+    });
+
+    // template <class... Args> reference emplace_front(Args&&... args);
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
       c.push_front(*from);
     });
 
     // iterator insert_after(const_iterator p, const value_type& v);
-    test_exception_safety_throwing_copy</*ThrowOn=*/1, Size>([](T* from, T*) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
       c.insert_after(c.before_begin(), *from);
     });
+
+    // iterator insert_after(const_iterator p, value_type&& v);
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
+      c.insert_after(c.before_begin(), std::move(*from));
+    });
+
+    // template <class... Args>
+    //     iterator emplace_after(const_iterator p, Args&&... args);
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
+      c.emplace_after(c.before_begin(), *from);
+    });
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
+      c.emplace_after(c.before_begin(), std::move(*from));
+    });
   }
 
   {
@@ -169,40 +191,53 @@ int main(int, char**) {
 #if TEST_STD_VER >= 23
     // template<container-compatible-range<T> R>
     //   void prepend_range(R&& rg); // C++23
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
       c.prepend_range(std::ranges::subrange(from, to));
     });
 #endif
 
     // iterator insert_after(const_iterator p, size_type n, const value_type& v);
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
       c.insert_after(c.before_begin(), Size, *from);
     });
 
     // template <class InputIterator>
     //     iterator insert_after(const_iterator p,
     //                           InputIterator first, InputIterator last);
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
       c.insert_after(c.before_begin(), from, to);
     });
 
+    // iterator insert_after(const_iterator p, initializer_list<value_type> il);
+    std::initializer_list<T> il{1, 2, 3, 4, 5};
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([&](std::forward_list<T>& c, T*, T*) {
+      c.insert_after(c.before_begin(), il);
+    });
+
 #if TEST_STD_VER >= 23
     // template<container-compatible-range<T> R>
     //     iterator insert_range_after(const_iterator position, R&& rg); // C++23
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T* to) {
-      std::forward_list<T> c;
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T* to) {
       c.insert_range_after(c.before_begin(), std::ranges::subrange(from, to));
     });
 #endif
 
     // void resize(size_type n, const value_type& v);
-    test_exception_safety_throwing_copy<ThrowOn, Size>([](T* from, T*) {
-      std::forward_list<T> c;
-      c.resize(Size, *from);
-    });
+    test_strong_exception_safety_throwing_copy<ThrowOn, Size>([](std::forward_list<T>& c, T* from, T*) {
+      c.resize(Size * 3, *from);
+    });
+
+    { // void resize(size_type n);
+      using X = ThrowingDefault<ThrowOn>;
+      std::forward_list<X> c0{X{1}, X{2}, X{3}};
+      std::forward_list<X> c = c0;
+      try {
+        c.resize(3 * ThrowOn);
+        assert(false);
+      } catch (int) {
+        assert(c == c0);
+      }
+    }
   }
 
   return 0;



More information about the libcxx-commits mailing list