Skip to content

Conversation

@winner245
Copy link
Contributor

@winner245 winner245 commented Mar 6, 2025

LWG3088 requires that forward_list::merge() is a no-op when passed *this, which aligns with the behavior of list::merge. Although libc++'s implementation of forward_list::merge() already meets this requirement, there were no tests to verify this behavior. This patch adds the necessary tests to ensure that self-merging remains a no-op and prevents any future regressions on this.

Closes #104942.

@winner245 winner245 marked this pull request as ready for review March 6, 2025 04:23
@winner245 winner245 requested a review from a team as a code owner March 6, 2025 04:23
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

LWG3088 requires that forward_list::merge() is a no-op when passed *this, which aligns with the behavior of list::merge. Although libc++'s implementation of forward_list::merge() already meets this requirement, there were no tests to verify this behavior. This patch adds the necessary tests to ensure that self-merging remains a no-op and prevents any future regressions.

Closes #104942.


Full diff: https://github.com/llvm/llvm-project/pull/129985.diff

5 Files Affected:

  • (modified) libcxx/docs/Status/Cxx23Issues.csv (+1-1)
  • (modified) libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.pass.cpp (+8)
  • (modified) libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp (+8-1)
  • (modified) libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp (+8)
  • (modified) libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.pass.cpp (+8)
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 9ae6322b7fdbe..c0245035f60ae 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -137,7 +137,7 @@
 "`LWG3593 <https://wg21.link/LWG3593>`__","Several iterators' ``base() const &`` and ``lazy_split_view::outer-iterator::value_type::end()`` missing ``noexcept``","2021-10 (Virtual)","","",""
 "`LWG3595 <https://wg21.link/LWG3595>`__","Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","2021-10 (Virtual)","|Complete|","14",""
 "","","","","",""
-"`LWG3088 <https://wg21.link/LWG3088>`__","``forward_list::merge`` behaviour unclear when passed ``*this``","2022-02 (Virtual)","","",""
+"`LWG3088 <https://wg21.link/LWG3088>`__","``forward_list::merge`` behaviour unclear when passed ``*this``","2022-02 (Virtual)","|Complete|","21",""
 "`LWG3471 <https://wg21.link/LWG3471>`__","``polymorphic_allocator::allocate`` does not satisfy ``Cpp17Allocator`` requirements","2022-02 (Virtual)","","",""
 "`LWG3525 <https://wg21.link/LWG3525>`__","``uses_allocator_construction_args`` fails to handle types convertible to ``pair``","2022-02 (Virtual)","","",""
 "`LWG3598 <https://wg21.link/LWG3598>`__","``system_category().default_error_condition(0)`` is underspecified","2022-02 (Virtual)","","",""
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.pass.cpp
index 6e73b2fd73726..541a5417db4e5 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue.pass.cpp
@@ -9,6 +9,7 @@
 // <forward_list>
 
 // void merge(forward_list& x);
+// If (addressof(x) == this) does nothing; otherwise ...
 
 #include <forward_list>
 #include <iterator>
@@ -109,5 +110,12 @@ int main(int, char**) {
   }
 #endif
 
+  { // Make sure self-merging does nothing.
+    int a[] = {1, 2, 3, 4, 5};
+    std::forward_list<int> c(a, a + sizeof(a) / sizeof(a[0]));
+    c.merge(c);
+    assert((c == std::forward_list<int>(a, a + sizeof(a) / sizeof(a[0]))));
+  }
+
   return 0;
 }
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp
index fddf9f9dc0f46..22acb2836076b 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_lvalue_pred.pass.cpp
@@ -9,6 +9,7 @@
 // <forward_list>
 
 // template <class Compare> void merge(forward_list& x, Compare comp);
+// If (addressof(x) == this) does nothing; otherwise ...
 
 #include <forward_list>
 #include <iterator>
@@ -91,7 +92,6 @@ int main(int, char**) {
     C c3(std::begin(t3), std::end(t3));
     assert(c1 == c3);
   }
-
 #if TEST_STD_VER >= 11
   { // Test with a different allocator.
     typedef int T;
@@ -110,5 +110,12 @@ int main(int, char**) {
   }
 #endif
 
+  { // Make sure self-merging does nothing.
+    int a[] = {5, 4, 3, 2, 1};
+    std::forward_list<int> c(a, a + sizeof(a) / sizeof(a[0]));
+    c.merge(c, std::greater<int>());
+    assert((c == std::forward_list<int>(a, a + sizeof(a) / sizeof(a[0]))));
+  }
+
   return 0;
 }
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp
index d5084eccd98da..995762e8a04e3 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue.pass.cpp
@@ -11,6 +11,7 @@
 // <forward_list>
 
 // void merge(forward_list&& x);
+// If (addressof(x) == this) does nothing; otherwise ...
 
 #include <forward_list>
 #include <functional>
@@ -102,5 +103,12 @@ int main(int, char**) {
     assert(c1 == c3);
   }
 
+  { // Make sure self-merging does nothing.
+    int a[] = {1, 2, 3, 4, 5};
+    std::forward_list<int> c(a, a + sizeof(a) / sizeof(a[0]));
+    c.merge(std::move(c));
+    assert((c == std::forward_list<int>(a, a + sizeof(a) / sizeof(a[0]))));
+  }
+
   return 0;
 }
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.pass.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.pass.cpp
index 235707c65370d..0ec466cefce2b 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.pass.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.ops/merge_rvalue_pred.pass.cpp
@@ -11,6 +11,7 @@
 // <forward_list>
 
 // template <class Compare> void merge(forward_list&& x, Compare comp);
+// If (addressof(x) == this) does nothing; otherwise ...
 
 #include <forward_list>
 #include <functional>
@@ -103,5 +104,12 @@ int main(int, char**) {
     assert(c1 == c3);
   }
 
+  { // Make sure self-merging does nothing.
+    int a[] = {5, 4, 3, 2, 1};
+    std::forward_list<int> c(a, a + sizeof(a) / sizeof(a[0]));
+    c.merge(std::move(c), std::greater<int>());
+    assert((c == std::forward_list<int>(a, a + sizeof(a) / sizeof(a[0]))));
+  }
+
   return 0;
 }

@ldionne ldionne merged commit b350bc2 into llvm:main Mar 19, 2025
13 of 19 checks passed
@winner245 winner245 deleted the no-op-self-merge branch March 19, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LWG3088: forward_list::merge behaviour unclear when passed *this

5 participants