shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

[libc++] Remove redundant and somewhat confusing assertions around advance() (#133276)

Browse Source 
The std::advance function has a clear precondition that it can only be
called with a negative distance when a bidirectional iterator is used.
However, prev() and next() don't have such preconditions explicitly,
they inherit it from calling advance().

This patch removes assertions in prev() and next() that were duplicates
of similar ones in advance(), and removes a copy-pasted comment that was
trying to justify the use of _LIBCPP_ASSERT_PEDANTIC but IMO is creating
confusion with little benefit.
This commit is contained in:
Louis Dionne 2025-05-06 15:44:26 -04:00 committed by GitHub
parent 60b43ef70b
commit d05ab119e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 8 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
libcxx/include/__iterator/advance.h
View File

@ -65,8 +65,7 @@ template < class _InputIter,
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 void advance(_InputIter& __i, _Distance __orig_n) {
typedef typename iterator_traits<_InputIter>::difference_type _Difference;
_Difference __n = static_cast<_Difference>(std::__convert_to_integral(__orig_n));
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
_LIBCPP_ASSERT_PEDANTIC(__n >= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
_LIBCPP_ASSERT_PEDANTIC(__has_bidirectional_iterator_category<_InputIter>::value || __n >= 0,
"Attempt to advance(it, n) with negative n on a non-bidirectional iterator");
std::__advance(__i, __n, typename iterator_traits<_InputIter>::iterator_category());
}
@ -98,9 +97,8 @@ public:
// Preconditions: If `I` does not model `bidirectional_iterator`, `n` is not negative.
template <input_or_output_iterator _Ip>
_LIBCPP_HIDE_FROM_ABI constexpr void operator()(_Ip& __i, iter_difference_t<_Ip> __n) const {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
_LIBCPP_ASSERT_PEDANTIC(
__n >= 0 || bidirectional_iterator<_Ip>, "If `n < 0`, then `bidirectional_iterator<I>` must be true.");
_LIBCPP_ASSERT_PEDANTIC(bidirectional_iterator<_Ip> || __n >= 0,
"ranges::advance: Can only pass a negative `n` with a bidirectional_iterator.");
// If `I` models `random_access_iterator`, equivalent to `i += n`.
if constexpr (random_access_iterator<_Ip>) {
@ -149,9 +147,9 @@ public:
template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
_LIBCPP_HIDE_FROM_ABI constexpr iter_difference_t<_Ip>
operator()(_Ip& __i, iter_difference_t<_Ip> __n, _Sp __bound_sentinel) const {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
_LIBCPP_ASSERT_PEDANTIC((__n >= 0) || (bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>),
"If `n < 0`, then `bidirectional_iterator<I> && same_as<I, S>` must be true.");
_LIBCPP_ASSERT_PEDANTIC(
(bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>) || (__n >= 0),
"ranges::advance: Can only pass a negative `n` with a bidirectional_iterator coming from a common_range.");
// If `S` and `I` model `sized_sentinel_for<S, I>`:
if constexpr (sized_sentinel_for<_Sp, _Ip>) {
// If |n| >= |bound_sentinel - i|, equivalent to `ranges::advance(i, bound_sentinel)`.

6
libcxx/include/__iterator/next.h
View File

@ -10,7 +10,6 @@
#ifndef _LIBCPP___ITERATOR_NEXT_H
#define _LIBCPP___ITERATOR_NEXT_H
#include <__assert>
#include <__config>
#include <__iterator/advance.h>
#include <__iterator/concepts.h>
@ -27,11 +26,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
next(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
// Note that this check duplicates the similar check in `std::advance`.
_LIBCPP_ASSERT_PEDANTIC(__n >= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
"Attempt to next(it, n) with negative n on a non-bidirectional iterator");
std::advance(__x, __n);
return __x;
}

5
libcxx/include/__iterator/prev.h
View File

@ -10,7 +10,6 @@
#ifndef _LIBCPP___ITERATOR_PREV_H
#define _LIBCPP___ITERATOR_PREV_H
#include <__assert>
#include <__config>
#include <__iterator/advance.h>
#include <__iterator/concepts.h>
@ -31,10 +30,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
[[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n) {
// Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
// Note that this check duplicates the similar check in `std::advance`.
_LIBCPP_ASSERT_PEDANTIC(__n <= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
"Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
std::advance(__x, -__n);
return __x;
}

2
libcxx/test/libcxx/iterators/assert.next.pass.cpp
View File

@ -25,7 +25,7 @@ int main(int, char**) {
forward_iterator<int *> it(a+1);
(void)std::next(it, 1); // should work fine
(void)std::next(it, 0); // should work fine
TEST_LIBCPP_ASSERT_FAILURE(std::next(it, -1), "Attempt to next(it, n) with negative n on a non-bidirectional iterator");
TEST_LIBCPP_ASSERT_FAILURE(std::next(it, -1), "Attempt to advance(it, n) with negative n on a non-bidirectional iterator");
return 0;
}

2
libcxx/test/libcxx/iterators/assert.prev.pass.cpp
View File

@ -31,7 +31,7 @@ int main(int, char**) {
forward_iterator<int *> it(a+1);
(void)std::prev(it, -1); // should work fine
(void)std::prev(it, 0); // should work fine
TEST_LIBCPP_ASSERT_FAILURE(std::prev(it, 1), "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
TEST_LIBCPP_ASSERT_FAILURE(std::prev(it, 1), "Attempt to advance(it, n) with negative n on a non-bidirectional iterator");
return 0;
}