[libc++][test] Refactor tests for std::{copy, move, fill} algorithms #120909

winner245 · 2024-12-22T16:34:26Z

This refactor includes the following changes:

Refactor similar tests using types::for_each to remove redundant code;
Explicitly include the missing header type_algorithms.h in some test files;
Some tests scattered in different test functions with ad-hoc names (e.g., test5(), test6()) but belong to the same kind are now grouped into one function (test_struct_array()).

llvmbot · 2024-12-22T16:46:52Z

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

This refactor includes the following changes:

Rewirte similar tests using type_algorithms.h utilities to reduce reduandant code;
Factor out common headers;
Avoid transitive inclusion of type_algorithms.h

The purpose of this refactoring is to facilitate subsequent work to optimize the algorithms for vector<bool>::iterator.

Patch is 37.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/120909.diff

14 Files Affected:

(added) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h (+30)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp (+23-30)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp (+31-56)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp (+28-55)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp (+35-70)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp (+1)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp (+2-1)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp (+1)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp (+95-118)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.fill.pass.cpp (+1)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.fill_n.pass.cpp (+1)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move.pass.cpp (+49-60)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.move/move_backward.pass.cpp (+52-66)
(modified) libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.move.pass.cpp (+1)

diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h
new file mode 100644
index 00000000000000..562ae5121e4287
--- /dev/null
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h
@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_STD_ALGORITHMS_ALG_MODIFYING_OPERATIONS_ALG_COPY_COMMON_H
+#define TEST_STD_ALGORITHMS_ALG_MODIFYING_OPERATIONS_ALG_COPY_COMMON_H
+
+#include <cstdint>
+#include "test_macros.h"
+
+class PaddedBase {
+public:
+  TEST_CONSTEXPR PaddedBase(std::int16_t a, std::int8_t b) : a_(a), b_(b) {}
+
+  std::int16_t a_;
+  std::int8_t b_;
+};
+
+class Derived : public PaddedBase {
+public:
+  TEST_CONSTEXPR Derived(std::int16_t a, std::int8_t b, std::int8_t c) : PaddedBase(a, b), c_(c) {}
+
+  std::int8_t c_;
+};
+
+#endif
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp
index b5f0a32b986a03..e781feacb8bd93 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp
@@ -15,23 +15,30 @@
 #include <algorithm>
 #include <cassert>
 
+#include "common.h"
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 
-class PaddedBase {
-public:
-  TEST_CONSTEXPR PaddedBase(std::int16_t a, std::int8_t b) : a_(a), b_(b) {}
-
-  std::int16_t a_;
-  std::int8_t b_;
-};
-
-class Derived : public PaddedBase {
-public:
-  TEST_CONSTEXPR Derived(std::int16_t a, std::int8_t b, std::int8_t c) : PaddedBase(a, b), c_(c) {}
+TEST_CONSTEXPR_CXX20 void test_padding() {
+  { // Make sure that padding bits aren't copied
+    Derived src(1, 2, 3);
+    Derived dst(4, 5, 6);
+    std::copy(static_cast<PaddedBase*>(&src), static_cast<PaddedBase*>(&src) + 1, static_cast<PaddedBase*>(&dst));
+    assert(dst.a_ == 1);
+    assert(dst.b_ == 2);
+    assert(dst.c_ == 6);
+  }
+}
 
-  std::int8_t c_;
-};
+TEST_CONSTEXPR_CXX20 void test_overlapping() {
+  { // Make sure that overlapping ranges can be copied
+    int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+    std::copy(a + 3, a + 10, a);
+    int expected[] = {4, 5, 6, 7, 8, 9, 10, 8, 9, 10};
+    assert(std::equal(a, a + 10, expected));
+  }
+}
 
 template <class InIter>
 struct Test {
@@ -60,23 +67,9 @@ struct TestInIters {
 };
 
 TEST_CONSTEXPR_CXX20 bool test() {
-  types::for_each(types::cpp17_input_iterator_list<int*>(), TestInIters());
-
-  { // Make sure that padding bits aren't copied
-    Derived src(1, 2, 3);
-    Derived dst(4, 5, 6);
-    std::copy(static_cast<PaddedBase*>(&src), static_cast<PaddedBase*>(&src) + 1, static_cast<PaddedBase*>(&dst));
-    assert(dst.a_ == 1);
-    assert(dst.b_ == 2);
-    assert(dst.c_ == 6);
-  }
-
-  { // Make sure that overlapping ranges can be copied
-    int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
-    std::copy(a + 3, a + 10, a);
-    int expected[] = {4, 5, 6, 7, 8, 9, 10, 8, 9, 10};
-    assert(std::equal(a, a + 10, expected));
-  }
+  types::for_each(types::cpp17_input_iterator_list<const int*>(), TestInIters());
+  test_padding();
+  test_overlapping();
 
   return true;
 }
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
index 928903de1ade2d..3629da78cbcc6a 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
@@ -16,69 +16,30 @@
 #include <algorithm>
 #include <cassert>
 
+#include "common.h"
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 #include "user_defined_integral.h"
 
-class PaddedBase {
-public:
-  TEST_CONSTEXPR PaddedBase(std::int16_t a, std::int8_t b) : a_(a), b_(b) {}
-
-  std::int16_t a_;
-  std::int8_t b_;
-};
-
-class Derived : public PaddedBase {
-public:
-  TEST_CONSTEXPR Derived(std::int16_t a, std::int8_t b, std::int8_t c) : PaddedBase(a, b), c_(c) {}
-
-  std::int8_t c_;
-};
-
-template <class InIter, class OutIter>
-TEST_CONSTEXPR_CXX20 void
-test_copy_backward()
-{
-  {
+template <class InIter>
+struct TestOutIters {
+  template <class OutIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
     const unsigned N = 1000;
-    int ia[N] = {};
+    int ia[N]        = {};
     for (unsigned i = 0; i < N; ++i)
-        ia[i] = i;
+      ia[i] = i;
     int ib[N] = {0};
 
-    OutIter r = std::copy_backward(InIter(ia), InIter(ia+N), OutIter(ib+N));
+    OutIter r = std::copy_backward(InIter(ia), InIter(ia + N), OutIter(ib + N));
     assert(base(r) == ib);
     for (unsigned i = 0; i < N; ++i)
-        assert(ia[i] == ib[i]);
+      assert(ia[i] == ib[i]);
   }
-}
-
-TEST_CONSTEXPR_CXX20 bool
-test()
-{
-    test_copy_backward<bidirectional_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_backward<bidirectional_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_backward<bidirectional_iterator<const int*>, int*>();
-
-    test_copy_backward<random_access_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_backward<random_access_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_backward<random_access_iterator<const int*>, int*>();
-
-    test_copy_backward<const int*, bidirectional_iterator<int*> >();
-    test_copy_backward<const int*, random_access_iterator<int*> >();
-    test_copy_backward<const int*, int*>();
-
-#if TEST_STD_VER > 17
-    test_copy_backward<contiguous_iterator<const int*>, bidirectional_iterator<int*>>();
-    test_copy_backward<contiguous_iterator<const int*>, random_access_iterator<int*>>();
-    test_copy_backward<contiguous_iterator<const int*>, int*>();
-
-    test_copy_backward<bidirectional_iterator<const int*>, contiguous_iterator<int*>>();
-    test_copy_backward<random_access_iterator<const int*>, contiguous_iterator<int*>>();
-    test_copy_backward<contiguous_iterator<const int*>, contiguous_iterator<int*>>();
-    test_copy_backward<const int*, contiguous_iterator<int*>>();
-#endif
+};
 
+TEST_CONSTEXPR_CXX20 void test_padding() {
   { // Make sure that padding bits aren't copied
     Derived src(1, 2, 3);
     Derived dst(4, 5, 6);
@@ -88,23 +49,37 @@ test()
     assert(dst.b_ == 2);
     assert(dst.c_ == 6);
   }
+}
 
+TEST_CONSTEXPR_CXX20 void test_overlapping() {
   { // Make sure that overlapping ranges can be copied
     int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
     std::copy_backward(a, a + 7, a + 10);
     int expected[] = {1, 2, 3, 1, 2, 3, 4, 5, 6, 7};
     assert(std::equal(a, a + 10, expected));
   }
+}
+
+struct TestInIters {
+  template <class InIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
+    types::for_each(types::bidirectional_iterator_list<int*>(), TestOutIters<InIter>());
+  }
+};
+
+TEST_CONSTEXPR_CXX20 bool test() {
+  types::for_each(types::bidirectional_iterator_list<const int*>(), TestInIters());
+  test_padding();
+  test_overlapping();
 
-    return true;
+  return true;
 }
 
-int main(int, char**)
-{
-    test();
+int main(int, char**) {
+  test();
 
 #if TEST_STD_VER > 17
-    static_assert(test());
+  static_assert(test());
 #endif
 
   return 0;
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp
index 57214e65455b45..3bee77738e3423 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp
@@ -19,75 +19,48 @@
 
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 
-struct Pred
-{
-    TEST_CONSTEXPR_CXX14 bool operator()(int i) {return i % 3 == 0;}
+struct Pred {
+  TEST_CONSTEXPR_CXX14 bool operator()(int i) { return i % 3 == 0; }
 };
 
-template <class InIter, class OutIter>
-TEST_CONSTEXPR_CXX20 void
-test_copy_if()
-{
+template <class InIter>
+struct TestOutIters {
+  template <class OutIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
     const unsigned N = 1000;
-    int ia[N] = {};
+    int ia[N]        = {};
     for (unsigned i = 0; i < N; ++i)
-        ia[i] = i;
+      ia[i] = i;
     int ib[N] = {0};
 
-    OutIter r = std::copy_if(InIter(ia), InIter(ia+N), OutIter(ib), Pred());
-    assert(base(r) == ib+N/3+1);
-    for (unsigned i = 0; i < N/3+1; ++i)
-        assert(ib[i] % 3 == 0);
-}
-
-TEST_CONSTEXPR_CXX20 bool
-test()
-{
-    test_copy_if<cpp17_input_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_if<cpp17_input_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_if<cpp17_input_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_if<cpp17_input_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_if<cpp17_input_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_if<cpp17_input_iterator<const int*>, int*>();
-
-    test_copy_if<forward_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, int*>();
-
-    test_copy_if<bidirectional_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_if<bidirectional_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_if<bidirectional_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_if<bidirectional_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_if<bidirectional_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_if<bidirectional_iterator<const int*>, int*>();
-
-    test_copy_if<random_access_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_if<random_access_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_if<random_access_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_if<random_access_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_if<random_access_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_if<random_access_iterator<const int*>, int*>();
+    OutIter r = std::copy_if(InIter(ia), InIter(ia + N), OutIter(ib), Pred());
+    assert(base(r) == ib + N / 3 + 1);
+    for (unsigned i = 0; i < N / 3 + 1; ++i)
+      assert(ib[i] % 3 == 0);
+  }
+};
 
-    test_copy_if<const int*, cpp17_output_iterator<int*> >();
-    test_copy_if<const int*, cpp17_input_iterator<int*> >();
-    test_copy_if<const int*, forward_iterator<int*> >();
-    test_copy_if<const int*, bidirectional_iterator<int*> >();
-    test_copy_if<const int*, random_access_iterator<int*> >();
-    test_copy_if<const int*, int*>();
+struct TestInIters {
+  template <class InIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
+    types::for_each(
+        types::concatenate_t<types::cpp17_input_iterator_list<int*>, types::type_list<cpp17_output_iterator<int*> > >(),
+        TestOutIters<InIter>());
+  }
+};
 
+TEST_CONSTEXPR_CXX20 bool test() {
+  types::for_each(types::cpp17_input_iterator_list<const int*>(), TestInIters());
   return true;
 }
 
-int main(int, char**)
-{
-    test();
+int main(int, char**) {
+  test();
 
 #if TEST_STD_VER > 17
-    static_assert(test());
+  static_assert(test());
 #endif
 
   return 0;
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp
index b0acc1060101c2..28362f4f990997 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp
@@ -15,44 +15,15 @@
 #include <algorithm>
 #include <cassert>
 
+#include "common.h"
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 #include "user_defined_integral.h"
 
 typedef UserDefinedIntegral<unsigned> UDI;
 
-class PaddedBase {
-public:
-  TEST_CONSTEXPR PaddedBase(std::int16_t a, std::int8_t b) : a_(a), b_(b) {}
-
-  std::int16_t a_;
-  std::int8_t b_;
-};
-
-class Derived : public PaddedBase {
-public:
-  TEST_CONSTEXPR Derived(std::int16_t a, std::int8_t b, std::int8_t c) : PaddedBase(a, b), c_(c) {}
-
-  std::int8_t c_;
-};
-
-template <class InIter, class OutIter>
-TEST_CONSTEXPR_CXX20 void
-test_copy_n()
-{
-  {
-    const unsigned N = 1000;
-    int ia[N] = {};
-    for (unsigned i = 0; i < N; ++i)
-        ia[i] = i;
-    int ib[N] = {0};
-
-    OutIter r = std::copy_n(InIter(ia), UDI(N/2), OutIter(ib));
-    assert(base(r) == ib+N/2);
-    for (unsigned i = 0; i < N/2; ++i)
-        assert(ia[i] == ib[i]);
-  }
-
+TEST_CONSTEXPR_CXX20 void test_padding() {
   { // Make sure that padding bits aren't copied
     Derived src(1, 2, 3);
     Derived dst(4, 5, 6);
@@ -61,7 +32,9 @@ test_copy_n()
     assert(dst.b_ == 2);
     assert(dst.c_ == 6);
   }
+}
 
+TEST_CONSTEXPR_CXX20 void test_overlapping() {
   { // Make sure that overlapping ranges can be copied
     int a[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
     std::copy_n(a + 3, 7, a);
@@ -70,53 +43,45 @@ test_copy_n()
   }
 }
 
-TEST_CONSTEXPR_CXX20 bool
-test()
-{
-    test_copy_n<cpp17_input_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_n<cpp17_input_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_n<cpp17_input_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_n<cpp17_input_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_n<cpp17_input_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_n<cpp17_input_iterator<const int*>, int*>();
-
-    test_copy_n<forward_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_n<forward_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_n<forward_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_n<forward_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_n<forward_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_n<forward_iterator<const int*>, int*>();
+template <class InIter>
+struct TestOutIters {
+  template <class OutIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
+    const unsigned N = 1000;
+    int ia[N]        = {};
+    for (unsigned i = 0; i < N; ++i)
+      ia[i] = i;
+    int ib[N] = {0};
 
-    test_copy_n<bidirectional_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_n<bidirectional_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_n<bidirectional_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_n<bidirectional_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_n<bidirectional_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_n<bidirectional_iterator<const int*>, int*>();
+    OutIter r = std::copy_n(InIter(ia), UDI(N / 2), OutIter(ib));
+    assert(base(r) == ib + N / 2);
+    for (unsigned i = 0; i < N / 2; ++i)
+      assert(ia[i] == ib[i]);
+  }
+};
 
-    test_copy_n<random_access_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_n<random_access_iterator<const int*>, cpp17_input_iterator<int*> >();
-    test_copy_n<random_access_iterator<const int*>, forward_iterator<int*> >();
-    test_copy_n<random_access_iterator<const int*>, bidirectional_iterator<int*> >();
-    test_copy_n<random_access_iterator<const int*>, random_access_iterator<int*> >();
-    test_copy_n<random_access_iterator<const int*>, int*>();
+struct TestInIters {
+  template <class InIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
+    types::for_each(
+        types::concatenate_t<types::cpp17_input_iterator_list<int*>, types::type_list<cpp17_output_iterator<int*> > >(),
+        TestOutIters<InIter>());
+  }
+};
 
-    test_copy_n<const int*, cpp17_output_iterator<int*> >();
-    test_copy_n<const int*, cpp17_input_iterator<int*> >();
-    test_copy_n<const int*, forward_iterator<int*> >();
-    test_copy_n<const int*, bidirectional_iterator<int*> >();
-    test_copy_n<const int*, random_access_iterator<int*> >();
-    test_copy_n<const int*, int*>();
+TEST_CONSTEXPR_CXX20 bool test() {
+  types::for_each(types::cpp17_input_iterator_list<const int*>(), TestInIters());
+  test_padding();
+  test_overlapping();
 
   return true;
 }
 
-int main(int, char**)
-{
-    test();
+int main(int, char**) {
+  test();
 
 #if TEST_STD_VER > 17
-    static_assert(test());
+  static_assert(test());
 #endif
 
   return 0;
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp
index bee1ef9bcec332..6229aac733a9c4 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp
@@ -23,6 +23,7 @@
 #include "test_macros.h"
 #include "test_execution_policies.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 
 EXECUTION_POLICY_SFINAE_TEST(copy);
 
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp
index 128108ac13811b..7208be75c70d03 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp
@@ -23,6 +23,7 @@
 #include "test_macros.h"
 #include "test_execution_policies.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 
 EXECUTION_POLICY_SFINAE_TEST(copy_n);
 
@@ -58,7 +59,7 @@ struct TestIteratorsInt {
 };
 
 struct CopiedToTester {
-  bool copied_to = false;
+  bool copied_to   = false;
   CopiedToTester() = default;
   CopiedToTester(const CopiedToTester&) {}
   CopiedToTester& operator=(const CopiedToTester&) {
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
index 619dc7242a3660..46b33d1727a716 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill.pass.cpp
@@ -21,6 +21,7 @@
 
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 
 template <class Iter, class Container>
 TEST_CONSTEXPR_CXX20 void
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
index 7d6770de702bf3..10ec4fee6315ba 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp
@@ -14,160 +14,137 @@
 //   fill_n(Iter first, Size n, const T& value);
 
 #include <algorithm>
+#include <array>
 #include <cassert>
 
 #include "test_macros.h"
 #include "test_iterators.h"
+#include "type_algorithms.h"
 #include "user_defined_integral.h"
 
-#if TEST_STD_VER > 17
-TEST_CONSTEXPR bool test_constexpr() {
-    const std::size_t N = 5;
-    int ib[] = {0, 0, 0, 0, 0, 0}; // one bigger than N
-
-    auto it = std::fill_n(std::begin(ib), N, 5);
-    return it == (std::begin(ib) + N)
-        && std::all_of(std::begin(ib), it, [](int a) {return a == 5; })
-        && *it == 0 // don't overwrite the last value in the output array
-        ;
-    }
-#endif
-
 typedef UserDefinedIntegral<unsigned> UDI;
 
-template <class Iter>
-void
-test_char()
-{
-    char a[4] = {};
-    Iter it...
[truncated]

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp

github-actions · 2025-02-09T20:41:12Z

✅ With the latest revision this PR passed the C/C++ code formatter.

ldionne

Thanks a lot for the refactoring!

ldionne · 2025-02-19T17:20:52Z

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp

@@ -91,7 +92,6 @@ TEST_CONSTEXPR_CXX20 bool test() {
    assert(dst.b_ == 2);
    assert(dst.c_ == 6);
  }
-


Nitpick, but this newline should probably stay.

ldionne · 2025-02-19T17:30:06Z

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp

-    test_copy_if<cpp17_input_iterator<const int*>, int*>();
-
-    test_copy_if<forward_iterator<const int*>, cpp17_output_iterator<int*> >();
-    test_copy_if<forward_iterator<const int*>, cpp17_input_iterator<int*> >();


This was a pre-existing condition in the test, but I think it's not valid to instantiate this test with cpp17_input_iterator as the output iterator. It happens to work because our cpp17_input_iterator returns a proper reference from operator*, but my reading of https://en.cppreference.com/w/cpp/named_req/InputIterator is that that's not guaranteed.

In other words, this input iterator "archetype" is too specific, and that has allowed us to write code that depends on more than the strict requirements of an input iterator. This would be worth trying to fix in a follow-up patch. Maybe returning a prvalue from cpp17_input_iterator ::operator* would be sufficient to catch 90% of the issues here, and that would be really simple.

ldionne · 2025-02-19T17:30:51Z

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp

+  template <class InIter>
+  TEST_CONSTEXPR_CXX20 void operator()() {
+    types::for_each(
+        types::concatenate_t<types::cpp17_input_iterator_list<int*>, types::type_list<cpp17_output_iterator<int*> > >(),


types::type_list<int*, cpp17_output_iterator<int*> >(),

I think that's the only thing we're really allowed to test here. This makes me realize that we probably have a similar problem in almost all of our tests?

Since this seems to be so widespread, I actually suggest not making any change as part of this patch. That way, we'll keep our tests incorrect but at least they will be consistently incorrect. We can fix this issue across all of our tests in a separate patch.

ldionne · 2025-02-19T17:42:09Z

libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/fill_n.pass.cpp

  test_int_array();
-  test_int_array_struct_source();
  test_struct_array();
-
-  test5();
-  test6();
-
+  test_int_array_struct_source();
  test_bititer_with_custom_sized_types();


FWIW I think it would be really acceptable if you inlined those functions here. But I'm not requesting you do, I think your current refactoring is already a large improvement.

ldionne · 2025-02-19T17:44:08Z

I'm going to merge this because I am fine with the patch and all of my comments are nitpicks. If you want, you could address them in a follow-up NFC pull request.

This patch enhances test readability by inlining standalone tests, eliminating unnecessary navigation. Additionally, several classes with ad-hoc names have been renamed for better clarity: - `A` -> `CharWrapper` as it wraps a char - `B -> CharTransformer` as it accepts a char `xc` but stores `xc + 1` - `Storage -> CharUnionStorage` as it stores a union of 2 `char`s. This patch addresses a follow-up comment from #120909 to inline tests.

This patch enhances test readability by inlining standalone tests, eliminating unnecessary navigation. Additionally, several classes with ad-hoc names have been renamed for better clarity: - `A` -> `CharWrapper` as it wraps a char - `B -> CharTransformer` as it accepts a char `xc` but stores `xc + 1` - `Storage -> CharUnionStorage` as it stores a union of 2 `char`s. This patch addresses a follow-up comment from llvm#120909 to inline tests.

winner245 force-pushed the refactor-copy-move-tests branch from 1bdd846 to 84de62b Compare December 22, 2024 16:45

winner245 marked this pull request as ready for review December 22, 2024 16:46

winner245 requested a review from a team as a code owner December 22, 2024 16:46

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 22, 2024

winner245 force-pushed the refactor-copy-move-tests branch from 84de62b to a718cf1 Compare December 22, 2024 17:06

philnik777 requested changes Dec 22, 2024

View reviewed changes

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/common.h Outdated Show resolved Hide resolved

libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp Outdated Show resolved Hide resolved

winner245 force-pushed the refactor-copy-move-tests branch 4 times, most recently from 2e22e58 to fc86a84 Compare December 23, 2024 02:50

This was referenced Jan 20, 2025

[libc++] Improve test coverage for copy/move ctors for vector<bool> #120132

Merged

[libc++] Fix UB in bitwise logic of {std, ranges}::{fill, fill_n} algorithms #122410

Merged

Refactor tests for std::{copy, move, fill} algorithms

319d569

winner245 force-pushed the refactor-copy-move-tests branch from fc86a84 to 338f002 Compare February 9, 2025 20:37

Apply philnik777's suggestions

b467191

winner245 force-pushed the refactor-copy-move-tests branch from 338f002 to b467191 Compare February 9, 2025 21:03

ldionne approved these changes Feb 19, 2025

View reviewed changes

ldionne merged commit ad87d5f into llvm:main Feb 19, 2025
78 checks passed

winner245 deleted the refactor-copy-move-tests branch February 20, 2025 23:42

joaosaffran mentioned this pull request Feb 26, 2025

[libc++] Set feature-test macro __cpp_lib_atomic_float (#127559) joaosaffran/llvm-project#1

Closed

winner245 mentioned this pull request Mar 31, 2025

[libc++][NFC] Improve test readability for std::fill_n #133771

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++][test] Refactor tests for std::{copy, move, fill} algorithms #120909

[libc++][test] Refactor tests for std::{copy, move, fill} algorithms #120909

winner245 commented Dec 22, 2024 •

edited

Loading

llvmbot commented Dec 22, 2024

github-actions bot commented Feb 9, 2025 •

edited

Loading

ldionne left a comment

ldionne Feb 19, 2025

ldionne Feb 19, 2025

ldionne Feb 19, 2025

ldionne Feb 19, 2025

ldionne commented Feb 19, 2025

[libc++][test] Refactor tests for std::{copy, move, fill} algorithms #120909

[libc++][test] Refactor tests for std::{copy, move, fill} algorithms #120909

Conversation

winner245 commented Dec 22, 2024 • edited Loading

llvmbot commented Dec 22, 2024

github-actions bot commented Feb 9, 2025 • edited Loading

ldionne left a comment

Choose a reason for hiding this comment

ldionne Feb 19, 2025

Choose a reason for hiding this comment

ldionne Feb 19, 2025

Choose a reason for hiding this comment

ldionne Feb 19, 2025

Choose a reason for hiding this comment

ldionne Feb 19, 2025

Choose a reason for hiding this comment

ldionne commented Feb 19, 2025

winner245 commented Dec 22, 2024 •

edited

Loading

github-actions bot commented Feb 9, 2025 •

edited

Loading