[gcc(refs/users/mikael/heads/pr121185_v01)] libstdc++: Make ranges::advance(it, n, bound) follow standard more strictly

Mikael Morin mikael@gcc.gnu.org
Thu Jul 24 07:59:33 GMT 2025


https://gcc.gnu.org/g:c28084bf02205eafceecbfc9cde160d2d36b2ad8

commit c28084bf02205eafceecbfc9cde160d2d36b2ad8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jul 14 11:21:24 2025 +0100

    libstdc++: Make ranges::advance(it, n, bound) follow standard more strictly
    
    The standard specifies some of the effects of ranges::advance in terms
    of "Equivalent to:" and it's observable that our current implementation
    deviates from the precise specification in the standard.  This was
    causing some failures in the libc++ testsuite.
    
    For the sized_sentinel_for<I, S> case I optimized our implementation to
    avoid redundant calls when we have already checked that there's nothing
    to do.  We were eliding `advance(i, bound)` when the iterator already
    equals the sentinel, and eliding `advance(i, n)` when `n` is zero. In
    both cases, removing the seemingly redundant calls is not equivalent to
    the spec because `i = std::move(bound)` or `i += 0` operations can be
    observed by program-defined iterators. This patch inlines the observable
    side effects of advance(i, bound) or advance(i, 0) without actually
    calling those functions.
    
    For the non-sized sentinel case, `if (i == bound || n == 0)` is
    different from `if (n == 0 || i == bound)` for the case where n is zero
    and a program-defined iterator observes the number of comparisons.
    This patch changes it to do `n == 0` first. I don't think this is
    required by the standard, as this condition is not "Equivalent to:" any
    observable sequence of operations, but testing `n == 0` first is
    probably cheaper anyway.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/ranges_base.h (ranges::advance(i, n, bound)):
            Ensure that observable side effects on iterators match what is
            specified in the standard.
    
    Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>

Diff:
---
 libstdc++-v3/include/bits/ranges_base.h | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
index 0251e5d0928a..7df638db2d6f 100644
--- a/libstdc++-v3/include/bits/ranges_base.h
+++ b/libstdc++-v3/include/bits/ranges_base.h
@@ -879,10 +879,17 @@ namespace ranges
       {
 	if constexpr (sized_sentinel_for<_Sent, _It>)
 	  {
-	    const auto __diff = __bound - __it;
+	    const iter_difference_t<_It> __diff = __bound - __it;
 
 	    if (__diff == 0)
-	      return __n;
+	      {
+		// inline any possible side effects of advance(it, bound)
+		if constexpr (assignable_from<_It&, _Sent>)
+		  __it = std::move(__bound);
+		else if constexpr (random_access_iterator<_It>)
+		  __it += 0;
+		return __n;
+	      }
 	    else if (__diff > 0 ? __n >= __diff : __n <= __diff)
 	      {
 		(*this)(__it, __bound);
@@ -897,9 +904,14 @@ namespace ranges
 		return 0;
 	      }
 	    else
-	      return 0;
+	      {
+		// inline any possible side effects of advance(it, n)
+		if constexpr (random_access_iterator<_It>)
+		  __it += 0;
+		return 0;
+	      }
 	  }
-	else if (__it == __bound || __n == 0)
+	else if (__n == 0 || __it == __bound)
 	  return __n;
 	else if (__n > 0)
 	  {


More information about the Libstdc++-cvs mailing list