[libcxx-commits] [libcxx] [libc++] `std::ranges::advance`: avoid unneeded bounds checks when advancing iterator (PR #84126)
Jan Kokemüller via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 26 22:52:48 PDT 2024
================
@@ -213,6 +221,20 @@ constexpr bool test() {
assert(i == iota_iterator{INT_MIN+1});
}
+ // Check that we don't do an unneeded bounds check when decrementing a
----------------
jiixyj wrote:
Yes, that comment does not seem to be correct.
My intention with the `!sized_sentinel_for` check was to _not_ fall into this case: <https://eel.is/c++draft/iterators#range.iter.op.advance-6.1>, but instead test the other one: <https://eel.is/c++draft/iterators#range.iter.op.advance-6.2>.
But it should actually be possible to remove the new, "special case" test and instead improve the generic `check_backward` tests, right? Because as you mentioned, being `sized_sentinel_for<It, It>` is not actually a precondition for "`advance` negative `n`". In an improved `check_backward` test, one would only have to ensure the actual preconditions:
- `[bound, i)` is a range
- that `It` is a `std::bidirectional_iterator`
- that `S` (the sentinel type) is the same as `It`
Then, one could instantiate `check_backward` with different iterator types that either model `sized_sentinel_for<It, It>` or don't. This way it would be possible to get coverage for both ["range.iter.op.advance-6.1"](<https://eel.is/c++draft/iterators#range.iter.op.advance-6.1>) and ["range.iter.op.advance-6.2"](<https://eel.is/c++draft/iterators#range.iter.op.advance-6.2>). Does this sound sensible to you?
https://github.com/llvm/llvm-project/pull/84126
More information about the libcxx-commits
mailing list