Skip to content

Commit 1bbf3a3

Browse files
huixie90ldionne
andauthored
[libc++] Fix reverse_iterator when underlying is c++20 bidirectional_iterator but not Cpp17BidirectionalIterator (#112100)
`reverse_iterator` supports either c++20 `bidirectional_iterator` or `Cpp17BidirectionalIterator ` http://eel.is/c++draft/reverse.iter.requirements The current `reverse_iterator` uses `std::prev` in its `operator->`, which only supports the `Cpp17BidirectionalIterator` properly. If the underlying iterator is c++20 `bidirectional_iterator` but does not satisfy the named requirement `Cpp17BidirectionalIterator`, (examples are `zip_view::iterator`, `flat_map::iterator`), the current `std::prev` silently compiles but does a no-op and returns the same iterator back. So `reverse_iterator::operator->` will silently give a wrong answer. Even if we fix the behaviour of `std::prev`, at best, we could fail to compile the code. But this is not ok, because we need to support this kind of iterators in `reverse_iterator`. The solution is simply to not use `std::prev`. --------- Co-authored-by: Louis Dionne <[email protected]>
1 parent aa32060 commit 1bbf3a3

26 files changed

+229
-56
lines changed

libcxx/include/__iterator/reverse_iterator.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,12 @@ class _LIBCPP_TEMPLATE_VIS reverse_iterator
136136
_LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const
137137
requires is_pointer_v<_Iter> || requires(const _Iter __i) { __i.operator->(); }
138138
{
139+
_Iter __tmp = current;
140+
--__tmp;
139141
if constexpr (is_pointer_v<_Iter>) {
140-
return std::prev(current);
142+
return __tmp;
141143
} else {
142-
return std::prev(current).operator->();
144+
return __tmp.operator->();
143145
}
144146
}
145147
#else

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/equal.pass.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3333
test(bidirectional_iterator<const char*>(s), bidirectional_iterator<const char*>(s+1), false);
3434
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), true);
3535
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), false);
36+
#if TEST_STD_VER >= 20
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), true);
38+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), false);
39+
#endif
3640
test(s, s, true);
3741
test(s, s+1, false);
3842
return true;

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/greater-equal.pass.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3232
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), true);
3333
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), true);
3434
test(random_access_iterator<const char*>(s+1), random_access_iterator<const char*>(s), false);
35+
#if TEST_STD_VER >= 20
36+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), true);
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), true);
38+
test(cpp20_random_access_iterator<const char*>(s + 1), cpp20_random_access_iterator<const char*>(s), false);
39+
#endif
3540
test(s, s, true);
3641
test(s, s+1, true);
3742
test(s+1, s, false);

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/greater.pass.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3232
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), false);
3333
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), true);
3434
test(random_access_iterator<const char*>(s+1), random_access_iterator<const char*>(s), false);
35+
#if TEST_STD_VER >= 20
36+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), false);
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), true);
38+
test(cpp20_random_access_iterator<const char*>(s + 1), cpp20_random_access_iterator<const char*>(s), false);
39+
#endif
3540
test(s, s, false);
3641
test(s, s+1, true);
3742
test(s+1, s, false);

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/less-equal.pass.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3232
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), true);
3333
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), false);
3434
test(random_access_iterator<const char*>(s+1), random_access_iterator<const char*>(s), true);
35+
#if TEST_STD_VER >= 20
36+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), true);
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), false);
38+
test(cpp20_random_access_iterator<const char*>(s + 1), cpp20_random_access_iterator<const char*>(s), true);
39+
#endif
3540
test(s, s, true);
3641
test(s, s+1, false);
3742
test(s+1, s, true);

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/less.pass.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3232
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), false);
3333
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), false);
3434
test(random_access_iterator<const char*>(s+1), random_access_iterator<const char*>(s), true);
35+
#if TEST_STD_VER >= 20
36+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), false);
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), false);
38+
test(cpp20_random_access_iterator<const char*>(s + 1), cpp20_random_access_iterator<const char*>(s), true);
39+
#endif
3540
test(s, s, false);
3641
test(s, s+1, false);
3742
test(s+1, s, true);

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cmp/not-equal.pass.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ TEST_CONSTEXPR_CXX17 bool tests() {
3333
test(bidirectional_iterator<const char*>(s), bidirectional_iterator<const char*>(s+1), true);
3434
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s), false);
3535
test(random_access_iterator<const char*>(s), random_access_iterator<const char*>(s+1), true);
36+
#if TEST_STD_VER >= 20
37+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s), false);
38+
test(cpp20_random_access_iterator<const char*>(s), cpp20_random_access_iterator<const char*>(s + 1), true);
39+
#endif
3640
test(s, s, false);
3741
test(s, s+1, true);
3842
return true;

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/assign.pass.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ TEST_CONSTEXPR_CXX17 bool tests() {
5959
Derived d;
6060
test<bidirectional_iterator<Base*> >(bidirectional_iterator<Derived*>(&d));
6161
test<random_access_iterator<const Base*> >(random_access_iterator<Derived*>(&d));
62+
#if TEST_STD_VER >= 20
63+
test<cpp20_random_access_iterator<const Base*> >(cpp20_random_access_iterator<Derived*>(&d));
64+
#endif
6265
test<Base*>(&d);
6366

6467
char c = '\0';

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/ctor.default.pass.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ TEST_CONSTEXPR_CXX17 void test() {
2626
TEST_CONSTEXPR_CXX17 bool tests() {
2727
test<bidirectional_iterator<const char*> >();
2828
test<random_access_iterator<char*> >();
29+
#if TEST_STD_VER >= 20
30+
test<cpp20_random_access_iterator<char*> >();
31+
#endif
2932
test<char*>();
3033
test<const char*>();
3134
return true;

libcxx/test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.cons/ctor.iter.pass.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ TEST_CONSTEXPR_CXX17 bool tests() {
2828
const char s[] = "123";
2929
test(bidirectional_iterator<const char*>(s));
3030
test(random_access_iterator<const char*>(s));
31+
#if TEST_STD_VER >= 20
32+
test(cpp20_random_access_iterator<const char*>(s));
33+
#endif
3134
test(s);
3235
return true;
3336
}

0 commit comments

Comments
 (0)