Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 19, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
319d569
Refactor tests for std::{copy, move, fill} algorithms
winner245 Dec 22, 2024
b467191
Apply philnik777's suggestions
winner245 Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"

class PaddedBase {
public:
Expand Down Expand Up @@ -81,7 +82,7 @@ TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) {
}

TEST_CONSTEXPR_CXX20 bool test() {
types::for_each(types::cpp17_input_iterator_list<int*>(), TestInIters());
types::for_each(types::cpp17_input_iterator_list<const int*>(), TestInIters());

{ // Make sure that padding bits aren't copied
Derived src(1, 2, 3);
Expand All @@ -91,7 +92,6 @@ TEST_CONSTEXPR_CXX20 bool test() {
assert(dst.b_ == 2);
assert(dst.c_ == 6);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick, but this newline should probably stay.

{ // 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);
Expand Down
62 changes: 25 additions & 37 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_backward.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"
#include "user_defined_integral.h"

class PaddedBase {
Expand All @@ -36,21 +37,29 @@ class Derived : public PaddedBase {
std::int8_t c_;
};

template <class InIter, class OutIter>
TEST_CONSTEXPR_CXX20 void test_copy_backward() {
{
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_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]);
struct TestIterators {
template <class InIter>
TEST_CONSTEXPR_CXX20 void operator()() {
types::for_each(types::bidirectional_iterator_list<int*>(), TestImpl<InIter>());
}
}

template <class InIter>
struct TestImpl {
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};

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]);
}
};
};

TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) {
std::vector<bool> in(N, false);
Expand All @@ -70,31 +79,10 @@ TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t N) {
}

return true;
};
}

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
types::for_each(types::bidirectional_iterator_list<const int*>(), TestIterators());

{ // Make sure that padding bits aren't copied
Derived src(1, 2, 3);
Expand Down
83 changes: 28 additions & 55 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_if.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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*> >();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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*> > >(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

        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.

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;
Expand Down
104 changes: 41 additions & 63 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include "test_macros.h"
#include "test_iterators.h"
#include "type_algorithms.h"
#include "user_defined_integral.h"

typedef UserDefinedIntegral<unsigned> UDI;
Expand All @@ -37,37 +38,31 @@ class Derived : public PaddedBase {
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]);
struct TestIterators {
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*> > >(),
TestImpl<InIter>());
}

{ // Make sure that padding bits aren't copied
Derived src(1, 2, 3);
Derived dst(4, 5, 6);
std::copy_n(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_n(a + 3, 7, a);
int expected[] = {4, 5, 6, 7, 8, 9, 10, 8, 9, 10};
assert(std::equal(a, a + 10, expected));
}
}
template <class InIter>
struct TestImpl {
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};

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 bool test_vector_bool(std::size_t N) {
std::vector<bool> in(N, false);
Expand All @@ -90,40 +85,23 @@ TEST_CONSTEXPR_CXX20 bool test_vector_bool(std::size_t 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*>();

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*>();

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*>();

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*>();
types::for_each(types::cpp17_input_iterator_list<const int*>(), TestIterators());

{ // Make sure that padding bits aren't copied
Derived src(1, 2, 3);
Derived dst(4, 5, 6);
std::copy_n(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_n(a + 3, 7, a);
int expected[] = {4, 5, 6, 7, 8, 9, 10, 8, 9, 10};
assert(std::equal(a, a + 10, expected));
}

{ // Test vector<bool>::iterator optimization
assert(test_vector_bool(8));
Expand Down
1 change: 1 addition & 0 deletions libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
3 changes: 2 additions & 1 deletion libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/pstl.copy_n.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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&) {
Expand Down
Loading