Re: Make deque iterator operators hidden friends

[Jonathan Wakely <jwakely at redhat dot com>]

On 08/05/19 18:50 +0200, François Dumont wrote:
> Here is a patch to reduce number of operators exposed at std namespace 
> scope.
>
>     * include/bits/stl_deque.h
>     (_Deque_iterator<>::operator+(difference_type)): Make hidden friend.
>     (_Deque_iterator<>::operator-(difference_type)): Likewise.
>     (operator==(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>     (operator!=(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>     (operator<(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>     (operator<=(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>     (operator>(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>     (operator>=(const _Deque_iterator<>&, const _Deque_iterator<>&)):
>     Likewise.
>
> Tested under Linux x86_64 normal/debug modes.
>
> Ok to commit ?
>
> François

> diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
> index 7a7a42aa903..72420c27646 100644
> --- a/libstdc++-v3/include/bits/stl_deque.h
> +++ b/libstdc++-v3/include/bits/stl_deque.h
> @@ -238,24 +238,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> 	return *this;
>       }
>
> -      _Self
> -      operator+(difference_type __n) const _GLIBCXX_NOEXCEPT
> -      {
> -	_Self __tmp = *this;
> -	return __tmp += __n;
> -      }
> -
>       _Self&
>       operator-=(difference_type __n) _GLIBCXX_NOEXCEPT
>       { return *this += -__n; }
>
> -      _Self
> -      operator-(difference_type __n) const _GLIBCXX_NOEXCEPT
> -      {
> -	_Self __tmp = *this;
> -	return __tmp -= __n;
> -      }
> -

These two are already not visible at namespace-scope, but it doesn't
hurt to make them hidden friends for consistency.

> +      friend _Self
> +      operator+(const _Self& __x, difference_type __n) _GLIBCXX_NOEXCEPT
> +      {
> +	_Self __tmp = __x;
> +	return __tmp += __n;

I know you've just reused the original implementation, but it has a
problem that we might as well fix while touching t his code.

This function prevents NRVO copy elision, because __tmp += __n returns
a reference, which gets copied into the return value.

If you write it like this then the copy is elided, and __tmp is
constructed directly in the return value slot:

 _Self __tmp = __x;
 __tmp += __n;
 return __tmp;


> +      friend _Self
> +      operator-(const _Self& __x, difference_type __n) _GLIBCXX_NOEXCEPT
> +      {
> +	_Self __tmp = __x;
> +	return __tmp -= __n;

Same problem here. It would be better to write:

 _Self __tmp = __x;
 __tmp -= __n;
 return __tmp;

OK for trunk with those two changes, thanks.