diff --git a/libcxx/include/__iterator/advance.h b/libcxx/include/__iterator/advance.h --- a/libcxx/include/__iterator/advance.h +++ b/libcxx/include/__iterator/advance.h @@ -73,12 +73,6 @@ struct __fn { private: - template - _LIBCPP_HIDE_FROM_ABI - static constexpr _Tp __magnitude_geq(_Tp __a, _Tp __b) noexcept { - return __a < 0 ? (__a <= __b) : (__a >= __b); - } - template _LIBCPP_HIDE_FROM_ABI static constexpr void __advance_forward(_Ip& __i, iter_difference_t<_Ip> __n) { @@ -155,6 +149,12 @@ // If `S` and `I` model `sized_sentinel_for`: if constexpr (sized_sentinel_for<_Sp, _Ip>) { // If |n| >= |bound - i|, equivalent to `ranges::advance(i, bound)`. + // __magnitude_geq(a, b) returns |a| >= |b|, assuming they have the same sign. + auto __magnitude_geq = [](auto __a, auto __b) { + return __a == 0 ? __b == 0 : + __a > 0 ? __a >= __b : + __a <= __b; + }; if (const auto __M = __bound - __i; __magnitude_geq(__n, __M)) { (*this)(__i, __bound); return __n - __M; diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp --- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp +++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.advance/iterator_count_sentinel.pass.cpp @@ -119,6 +119,54 @@ constexpr bool test() { int range[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + // Basic functionality test: advance forward, bound has the same type + { + int *p; + p = range+5; assert(std::ranges::advance(p, 0, range+7) == 0); assert(p == range+5); + p = range+5; assert(std::ranges::advance(p, 1, range+7) == 0); assert(p == range+6); + p = range+5; assert(std::ranges::advance(p, 2, range+7) == 0); assert(p == range+7); + p = range+5; assert(std::ranges::advance(p, 3, range+7) == 1); assert(p == range+7); + } + + // Basic functionality test: advance forward, bound is not the same type and not assignable + { + int *p; + using ConstPtr = const int*; + p = range+5; assert(std::ranges::advance(p, 0, ConstPtr(range+7)) == 0); assert(p == range+5); + p = range+5; assert(std::ranges::advance(p, 1, ConstPtr(range+7)) == 0); assert(p == range+6); + p = range+5; assert(std::ranges::advance(p, 2, ConstPtr(range+7)) == 0); assert(p == range+7); + p = range+5; assert(std::ranges::advance(p, 3, ConstPtr(range+7)) == 1); assert(p == range+7); + } + + // Basic functionality test: advance forward, bound has different type but assignable + { + const int *pc; + pc = range+5; assert(std::ranges::advance(pc, 0, range+7) == 0); assert(pc == range+5); + pc = range+5; assert(std::ranges::advance(pc, 1, range+7) == 0); assert(pc == range+6); + pc = range+5; assert(std::ranges::advance(pc, 2, range+7) == 0); assert(pc == range+7); + pc = range+5; assert(std::ranges::advance(pc, 3, range+7) == 1); assert(pc == range+7); + } + + // Basic functionality test: advance backward, bound has the same type + // Note that we don't test advancing backward with a bound of a different type because that's UB + { + int *p; + p = range+5; assert(std::ranges::advance(p, 0, range+3) == 0); assert(p == range+5); + p = range+5; assert(std::ranges::advance(p, -1, range+3) == 0); assert(p == range+4); + p = range+5; assert(std::ranges::advance(p, -2, range+3) == 0); assert(p == range+3); + p = range+5; assert(std::ranges::advance(p, -3, range+3) == -1); assert(p == range+3); + } + + // Basic functionality test: advance backward with an array as a sentinel + { + int* p; + p = range+5; assert(std::ranges::advance(p, 0, range) == 0); assert(p == range+5); + p = range+5; assert(std::ranges::advance(p, -1, range) == 0); assert(p == range+4); + p = range+5; assert(std::ranges::advance(p, -5, range) == 0); assert(p == range); + p = range+5; assert(std::ranges::advance(p, -6, range) == -1); assert(p == range); + } + + // Exhaustive checks for n and range sizes for (int size = 0; size != 10; ++size) { for (int n = 0; n != 20; ++n) { @@ -145,16 +193,15 @@ // Note that we can only test ranges::advance with a negative n for iterators that // are sized sentinels for themselves, because ranges::advance is UB otherwise. // In particular, that excludes bidirectional_iterators since those are not sized sentinels. - // TODO: Enable these tests once we fix the bug in ranges::advance - // int* expected = n > size ? range : range + size - n; - // check_backward>(range, range+size, -n, expected); - // check_backward>( range, range+size, -n, expected); - // check_backward( range, range+size, -n, expected); + int* expected = n > size ? range : range + size - n; + check_backward>(range, range+size, -n, expected); + check_backward>( range, range+size, -n, expected); + check_backward( range, range+size, -n, expected); } } } - // regression-test that INT_MIN doesn't cause any undefined behavior + // Regression-test that INT_MIN doesn't cause any undefined behavior { auto i = iota_iterator{+1}; assert(std::ranges::advance(i, INT_MIN, iota_iterator{-2}) == INT_MIN+3); diff --git a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.prev/iterator_count_sentinel.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.prev/iterator_count_sentinel.pass.cpp --- a/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.prev/iterator_count_sentinel.pass.cpp +++ b/libcxx/test/std/iterators/iterator.primitives/range.iter.ops/range.iter.ops.prev/iterator_count_sentinel.pass.cpp @@ -28,9 +28,7 @@ assert(base(result) == expected); } -// TODO: Re-enable once we fix the bug in ranges::advance constexpr bool test() { -#if 0 int range[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; for (int size = 0; size != 10; ++size) { @@ -42,7 +40,6 @@ check( range, range+size, n, expected); } } -#endif return true; }