Skip to content

Commit f1db578

Browse files
[libc++][test] Fix assumptions that std::array iterators are pointers (llvm#74430)
Found while running libc++'s tests with MSVC's STL, where `std::array` iterators are never pointers. Most of these changes are reasonably self-explanatory (the `std::array`s are right there, and the sometimes-slightly-wrapped raw pointer types are a short distance away). A couple of changes are less obvious: In `libcxx/test/std/containers/from_range_helpers.h`, `wrap_input()` is called with `Iter` types that are constructible from raw pointers. It's also sometimes called with an `array` as the `input`, so the first overload was implicitly assuming that `array` iterators are pointers. We can fix this assumption by providing a dedicated overload for `array`, just like the one for `vector` immediately below. Finally, `from_range_helpers.h` should explicitly include both `<array>` and `<vector>`, even though they were apparently being dragged in already. In `libcxx/test/std/containers/views/views.span/span.cons/iterator_sentinel.pass.cpp`, fix `throw_operator_minus`. The error was pretty complicated, caused by the concepts machinery noticing that `value_type` and `element_type` were inconsistent. In the template instantiation context, you can see the critical detail that `throw_operator_minus<std::_Array_iterator>` is being formed. Fortunately, the fix is extremely simple. To produce `element_type` (which retains any cv-qualification, unlike `value_type`), we shouldn't attempt to `remove_pointer` with the iterator type `It`. Instead, we've already obtained the `reference` type, so we can `remove_reference_t`. (This is modern code, where we have access to the alias templates, so I saw no reason to use the older verbose form.)
1 parent 34cdc91 commit f1db578

File tree

13 files changed

+152
-99
lines changed

13 files changed

+152
-99
lines changed

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace.pass.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,35 +29,35 @@ struct Test {
2929
void operator()(ExecutionPolicy&& policy) {
3030
{ // simple test
3131
std::array a = {1, 2, 3, 4, 5, 6, 7, 8};
32-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 6);
32+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 6);
3333
assert((a == std::array{1, 2, 6, 4, 5, 6, 7, 8}));
3434
}
3535

3636
{ // empty range works
3737
std::array<int, 0> a = {};
38-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 6);
38+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 6);
3939
}
4040

4141
{ // non-empty range without a match works
4242
std::array a = {1, 2};
43-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 6);
43+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 6);
4444
}
4545

4646
{ // single element range works
4747
std::array a = {3};
48-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 6);
48+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 6);
4949
assert((a == std::array{6}));
5050
}
5151

5252
{ // two element range works
5353
std::array a = {3, 4};
54-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 6);
54+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 6);
5555
assert((a == std::array{6, 4}));
5656
}
5757

5858
{ // multiple matching elements work
5959
std::array a = {1, 2, 3, 4, 3, 3, 5, 6, 3};
60-
std::replace(policy, Iter(std::begin(a)), Iter(std::end(a)), 3, 9);
60+
std::replace(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), 3, 9);
6161
assert((a == std::array{1, 2, 9, 4, 9, 9, 5, 6, 9}));
6262
}
6363

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace_copy.pass.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,40 +32,40 @@ struct Test {
3232
{ // simple test
3333
std::array a = {1, 2, 3, 4, 5, 6, 7, 8};
3434
std::array<int, a.size()> out;
35-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), 3, 6);
35+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(std::data(out)), 3, 6);
3636
assert((out == std::array{1, 2, 6, 4, 5, 6, 7, 8}));
3737
}
3838

3939
{ // empty range works
4040
std::array<int, 0> a = {};
41-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(a)), 3, 6);
41+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(std::data(a)), 3, 6);
4242
}
4343

4444
{ // non-empty range without a match works
4545
std::array a = {1, 2};
4646
std::array<int, a.size()> out;
47-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(out.data()), 3, 6);
47+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(out.data()), 3, 6);
4848
assert((out == std::array{1, 2}));
4949
}
5050

5151
{ // single element range works
5252
std::array a = {3};
5353
std::array<int, a.size()> out;
54-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), 3, 6);
54+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(std::data(out)), 3, 6);
5555
assert((out == std::array{6}));
5656
}
5757

5858
{ // two element range works
5959
std::array a = {3, 4};
6060
std::array<int, a.size()> out;
61-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), 3, 6);
61+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(std::data(out)), 3, 6);
6262
assert((out == std::array{6, 4}));
6363
}
6464

6565
{ // multiple matching elements work
6666
std::array a = {1, 2, 3, 4, 3, 3, 5, 6, 3};
6767
std::array<int, a.size()> out;
68-
std::replace_copy(policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), 3, 9);
68+
std::replace_copy(policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), Iter(std::data(out)), 3, 9);
6969
assert((out == std::array{1, 2, 9, 4, 9, 9, 5, 6, 9}));
7070
}
7171

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace_copy_if.pass.cpp

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,45 +34,75 @@ struct Test {
3434
std::array a = {1, 2, 3, 4, 5, 6, 7, 8};
3535
std::array<int, a.size()> out;
3636
std::replace_copy_if(
37-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), [](int i) { return i == 3; }, 6);
37+
policy,
38+
Iter(std::data(a)),
39+
Iter(std::data(a) + std::size(a)),
40+
Iter(std::data(out)),
41+
[](int i) { return i == 3; },
42+
6);
3843
assert((out == std::array{1, 2, 6, 4, 5, 6, 7, 8}));
3944
}
4045

4146
{ // empty range works
4247
std::array<int, 0> a = {};
4348
std::replace_copy_if(
44-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(a)), [](int i) { return i == 3; }, 6);
49+
policy,
50+
Iter(std::data(a)),
51+
Iter(std::data(a) + std::size(a)),
52+
Iter(std::data(a)),
53+
[](int i) { return i == 3; },
54+
6);
4555
}
4656

4757
{ // non-empty range without a match works
4858
std::array a = {1, 2};
4959
std::array<int, a.size()> out;
5060
std::replace_copy_if(
51-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(out.data()), [](int i) { return i == 3; }, 6);
61+
policy,
62+
Iter(std::data(a)),
63+
Iter(std::data(a) + std::size(a)),
64+
Iter(out.data()),
65+
[](int i) { return i == 3; },
66+
6);
5267
assert((out == std::array{1, 2}));
5368
}
5469

5570
{ // single element range works
5671
std::array a = {3};
5772
std::array<int, a.size()> out;
5873
std::replace_copy_if(
59-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), [](int i) { return i == 3; }, 6);
74+
policy,
75+
Iter(std::data(a)),
76+
Iter(std::data(a) + std::size(a)),
77+
Iter(std::data(out)),
78+
[](int i) { return i == 3; },
79+
6);
6080
assert((out == std::array{6}));
6181
}
6282

6383
{ // two element range works
6484
std::array a = {3, 4};
6585
std::array<int, a.size()> out;
6686
std::replace_copy_if(
67-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), [](int i) { return i == 3; }, 6);
87+
policy,
88+
Iter(std::data(a)),
89+
Iter(std::data(a) + std::size(a)),
90+
Iter(std::data(out)),
91+
[](int i) { return i == 3; },
92+
6);
6893
assert((out == std::array{6, 4}));
6994
}
7095

7196
{ // multiple matching elements work
7297
std::array a = {1, 2, 3, 4, 3, 3, 5, 6, 3};
7398
std::array<int, a.size()> out;
7499
std::replace_copy_if(
75-
policy, Iter(std::begin(a)), Iter(std::end(a)), Iter(std::begin(out)), [](int i) { return i == 3; }, 9);
100+
policy,
101+
Iter(std::data(a)),
102+
Iter(std::data(a) + std::size(a)),
103+
Iter(std::data(out)),
104+
[](int i) { return i == 3; },
105+
9);
76106
assert((out == std::array{1, 2, 9, 4, 9, 9, 5, 6, 9}));
77107
}
78108

libcxx/test/std/algorithms/alg.modifying.operations/alg.replace/pstl.replace_if.pass.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,38 +30,40 @@ struct Test {
3030
{ // simple test
3131
std::array a = {1, 2, 3, 4, 5, 6, 7, 8};
3232
std::replace_if(
33-
policy, Iter(std::begin(a)), Iter(std::end(a)), [](int i) { return i == 3 || i == 7; }, 6);
33+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int i) { return i == 3 || i == 7; }, 6);
3434
assert((a == std::array{1, 2, 6, 4, 5, 6, 6, 8}));
3535
}
3636

3737
{ // empty range works
3838
std::array<int, 0> a = {};
3939
std::replace_if(
40-
policy, Iter(std::begin(a)), Iter(std::end(a)), [](int) { return false; }, 6);
40+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int) { return false; }, 6);
4141
}
4242

4343
{ // non-empty range without a match works
4444
std::array a = {1, 2};
45-
std::replace_if(policy, Iter(std::begin(a)), Iter(std::end(a)), [](int) { return false; }, 6);
45+
std::replace_if(
46+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int) { return false; }, 6);
4647
}
4748

4849
{ // single element range works
4950
std::array a = {3};
5051
std::replace_if(
51-
policy, Iter(std::begin(a)), Iter(std::end(a)), [](int i) { return i == 3; }, 6);
52+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int i) { return i == 3; }, 6);
5253
assert((a == std::array{6}));
5354
}
5455

5556
{ // two element range works
5657
std::array a = {3, 4};
5758
std::replace_if(
58-
policy, Iter(std::begin(a)), Iter(std::end(a)), [](int i) { return i == 3; }, 6);
59+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int i) { return i == 3; }, 6);
5960
assert((a == std::array{6, 4}));
6061
}
6162

6263
{ // multiple matching elements work
6364
std::array a = {1, 2, 3, 4, 3, 3, 5, 6, 3};
64-
std::replace_if(policy, Iter(std::begin(a)), Iter(std::end(a)), [](int i) { return i == 3; }, 9);
65+
std::replace_if(
66+
policy, Iter(std::data(a)), Iter(std::data(a) + std::size(a)), [](int i) { return i == 3; }, 9);
6567
assert((a == std::array{1, 2, 9, 4, 9, 9, 5, 6, 9}));
6668
}
6769

libcxx/test/std/algorithms/alg.sorting/alg.merge/pstl.merge.pass.cpp

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,36 @@ struct Test {
5353
std::array<int, 0> a;
5454
std::array<int, 0> b;
5555
std::array<int, std::size(a) + std::size(b)> out;
56-
std::merge(
57-
policy, Iter1(std::begin(a)), Iter1(std::end(a)), Iter2(std::begin(b)), Iter2(std::end(b)), std::begin(out));
56+
std::merge(policy,
57+
Iter1(std::data(a)),
58+
Iter1(std::data(a) + std::size(a)),
59+
Iter2(std::data(b)),
60+
Iter2(std::data(b) + std::size(b)),
61+
std::begin(out));
5862
}
5963
{ // check that it works with the first range being empty
6064
std::array<int, 0> a;
6165
int b[] = {2, 4, 6, 8, 10};
6266
std::array<int, std::size(a) + std::size(b)> out;
63-
std::merge(
64-
policy, Iter1(std::begin(a)), Iter1(std::end(a)), Iter2(std::begin(b)), Iter2(std::end(b)), std::begin(out));
67+
std::merge(policy,
68+
Iter1(std::data(a)),
69+
Iter1(std::data(a) + std::size(a)),
70+
Iter2(std::begin(b)),
71+
Iter2(std::end(b)),
72+
std::begin(out));
6573
assert((out == std::array{2, 4, 6, 8, 10}));
6674
}
6775

6876
{ // check that it works with the second range being empty
6977
int a[] = {2, 4, 6, 8, 10};
7078
std::array<int, 0> b;
7179
std::array<int, std::size(a) + std::size(b)> out;
72-
std::merge(
73-
policy, Iter1(std::begin(a)), Iter1(std::end(a)), Iter2(std::begin(b)), Iter2(std::end(b)), std::begin(out));
80+
std::merge(policy,
81+
Iter1(std::begin(a)),
82+
Iter1(std::end(a)),
83+
Iter2(std::data(b)),
84+
Iter2(std::data(b) + std::size(b)),
85+
std::begin(out));
7486
assert((out == std::array{2, 4, 6, 8, 10}));
7587
}
7688

libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way.pass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@
2727

2828
template <typename Iter1, typename Iter2, typename C1, typename C2, typename Order>
2929
constexpr void test_lexicographical_compare(C1 a, C2 b, Order expected) {
30-
std::same_as<Order> decltype(auto) result =
31-
std::lexicographical_compare_three_way(Iter1{a.begin()}, Iter1{a.end()}, Iter2{b.begin()}, Iter2{b.end()});
30+
std::same_as<Order> decltype(auto) result = std::lexicographical_compare_three_way(
31+
Iter1{a.data()}, Iter1{a.data() + a.size()}, Iter2{b.data()}, Iter2{b.data() + b.size()});
3232
assert(expected == result);
3333
}
3434

libcxx/test/std/algorithms/alg.sorting/alg.three.way/lexicographical_compare_three_way_comp.pass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ static_assert(has_lexicographical_compare<decltype(compare_int_result)>);
5656

5757
template <typename Iter1, typename Iter2, typename C1, typename C2, typename Order, typename Comparator>
5858
constexpr void test_lexicographical_compare(C1 a, C2 b, Comparator comp, Order expected) {
59-
std::same_as<Order> decltype(auto) result =
60-
std::lexicographical_compare_three_way(Iter1{a.begin()}, Iter1{a.end()}, Iter2{b.begin()}, Iter2{b.end()}, comp);
59+
std::same_as<Order> decltype(auto) result = std::lexicographical_compare_three_way(
60+
Iter1{a.data()}, Iter1{a.data() + a.size()}, Iter2{b.data()}, Iter2{b.data() + b.size()}, comp);
6161
assert(expected == result);
6262
}
6363

libcxx/test/std/containers/from_range_helpers.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
#ifndef SUPPORT_FROM_RANGE_HELPERS_H
1010
#define SUPPORT_FROM_RANGE_HELPERS_H
1111

12+
#include <array>
1213
#include <cstddef>
1314
#include <iterator>
1415
#include <type_traits>
16+
#include <vector>
1517

1618
#include "min_allocator.h"
1719
#include "test_allocator.h"
@@ -34,6 +36,13 @@ constexpr auto wrap_input(Range&& input) {
3436
return std::ranges::subrange(std::move(b), std::move(e));
3537
}
3638

39+
template <class Iter, class Sent, class T, std::size_t N>
40+
constexpr auto wrap_input(std::array<T, N>& input) {
41+
auto b = Iter(input.data());
42+
auto e = Sent(Iter(input.data() + input.size()));
43+
return std::ranges::subrange(std::move(b), std::move(e));
44+
}
45+
3746
template <class Iter, class Sent, class T>
3847
constexpr auto wrap_input(std::vector<T>& input) {
3948
auto b = Iter(input.data());

libcxx/test/std/containers/views/views.span/span.cons/iterator_sentinel.pass.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class throw_operator_minus {
7171
typedef typename std::iterator_traits<It>::difference_type difference_type;
7272
typedef It pointer;
7373
typedef typename std::iterator_traits<It>::reference reference;
74-
typedef typename std::remove_pointer<It>::type element_type;
74+
typedef std::remove_reference_t<reference> element_type;
7575

7676
throw_operator_minus() : it_() {}
7777
explicit throw_operator_minus(It it) : it_(it) {}

libcxx/test/std/ranges/range.adaptors/range.chunk.by/range.chunk.by.iter/compare.pass.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,15 @@ constexpr void test() {
3131
using ChunkByView = std::ranges::chunk_by_view<Underlying, std::ranges::less_equal>;
3232
using ChunkByIterator = std::ranges::iterator_t<ChunkByView>;
3333

34-
auto make_chunk_by_view = [](auto begin, auto end) {
35-
View view{Iterator(begin), Sentinel(Iterator(end))};
34+
auto make_chunk_by_view = [](auto& arr) {
35+
View view{Iterator(arr.data()), Sentinel(Iterator(arr.data() + arr.size()))};
3636
return ChunkByView(std::move(view), std::ranges::less_equal{});
3737
};
3838

3939
// Test operator==
4040
{
4141
std::array array{0, 1, 2};
42-
ChunkByView view = make_chunk_by_view(array.begin(), array.end());
42+
ChunkByView view = make_chunk_by_view(array);
4343
ChunkByIterator i = view.begin();
4444
ChunkByIterator j = view.begin();
4545

@@ -52,7 +52,7 @@ constexpr void test() {
5252
// Test synthesized operator!=
5353
{
5454
std::array array{0, 1, 2};
55-
ChunkByView view = make_chunk_by_view(array.begin(), array.end());
55+
ChunkByView view = make_chunk_by_view(array);
5656
ChunkByIterator i = view.begin();
5757
ChunkByIterator j = view.begin();
5858

@@ -65,7 +65,7 @@ constexpr void test() {
6565
// Test operator== with std::default_sentinel_t
6666
{
6767
std::array array{0, 1, 2};
68-
ChunkByView view = make_chunk_by_view(array.begin(), array.end());
68+
ChunkByView view = make_chunk_by_view(array);
6969
ChunkByIterator i = view.begin();
7070

7171
std::same_as<bool> decltype(auto) result = (i == std::default_sentinel);
@@ -77,7 +77,7 @@ constexpr void test() {
7777
// Test synthesized operator!= with std::default_sentinel_t
7878
{
7979
std::array array{0, 1, 2};
80-
ChunkByView view = make_chunk_by_view(array.begin(), array.end());
80+
ChunkByView view = make_chunk_by_view(array);
8181
ChunkByIterator i = view.begin();
8282

8383
std::same_as<bool> decltype(auto) result = (i != std::default_sentinel);

0 commit comments

Comments
 (0)