[libcxx-commits] [libcxx] [libc++] `std::ranges::advance`: avoid unneeded bounds checks when advancing iterator (PR #84126)

[via libcxx-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Jan Kokemüller (jiixyj)

<details>
<summary>Changes</summary>

Currently, the bounds check in `std::ranges::advance(it, n, s)` is done _before_ `n` is checked. This results in one extra, unneeded bounds check.

Thus, `std::ranges::advance(it, 1, s)` currently is _not_ simply equivalent to:

```c++
if (it != s) {
    ++it;
}
```

This difference in behavior matters when the check involves some "expensive" logic. For example, the `==` operator of `std::istreambuf_iterator` may actually have to read the underlying `streambuf`.

Swapping around the checks in the `while` results in the expected behavior.

---
Full diff: https://github.com/llvm/llvm-project/pull/84126.diff


1 Files Affected:

- (modified) libcxx/include/__iterator/advance.h (+2-2) 


``````````diff
diff --git a/libcxx/include/__iterator/advance.h b/libcxx/include/__iterator/advance.h
index 7959bdeae32643..296db1aaab6526 100644
--- a/libcxx/include/__iterator/advance.h
+++ b/libcxx/include/__iterator/advance.h
@@ -170,14 +170,14 @@ struct __fn {
     } else {
       // Otherwise, if `n` is non-negative, while `bool(i != bound_sentinel)` is true, increments `i` but at
       // most `n` times.
-      while (__i != __bound_sentinel && __n > 0) {
+      while (__n > 0 && __i != __bound_sentinel) {
         ++__i;
         --__n;
       }
 
       // Otherwise, while `bool(i != bound_sentinel)` is true, decrements `i` but at most `-n` times.
       if constexpr (bidirectional_iterator<_Ip> && same_as<_Ip, _Sp>) {
-        while (__i != __bound_sentinel && __n < 0) {
+        while (__n < 0 && __i != __bound_sentinel) {
           --__i;
           ++__n;
         }

``````````

</details>


https://github.com/llvm/llvm-project/pull/84126