On Wed, 2 Jul 2025 at 15:29, Patrick Palka <ppa...@redhat.com> wrote:
>
>
> On Wed, 11 Jun 2025, Jonathan Wakely wrote:
>
> > As suggested by Jason, this makes all __normal_iterator operators into
> > friends so they can be found by ADL and don't need to be separately
> > exported in module std.
> >
> > The operator<=> comparing two iterators of the same type is removed
> > entirely, instead of being made a hidden friend. That overload was added
> > by r12-5882-g2c7fb16b5283cf to deal with unconstrained operator
> > overloads found by ADL, as defined in the testsuite_greedy_ops.h header.
> > We don't actually test that case as there's no unconstrained <=> in that
> > header, and it doesn't seem reasonable for anybody to define such an
> > operator<=> in C++20 when they should constrain their overloads properly
> > (e.g. using a requires-clause). The heterogeneous operator<=> overloads
> > added for reverse_iterator and move_iterator could also be removed, but
> > that's not part of this commit.
> >
> > I also had to reorder the __attribute__((always_inline)) and
> > [[nodiscard]] attributes, which have to be in a particular order when
> > used on friend functions.
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/bits/stl_iterator.h (__normal_iterator): Make all
> > non-member operators hidden friends, except ...
> > (operator<=>(__normal_iterator<I,C>, __normal_iterator<I,C>)):
>
> __normal_iterator<I,C>, __normal_iterator<J,C> rather (i.e. the
> heterogeneous overload)? LGTM besides that
Unless I've really confused myself, the heterogeneous overload is
retained, and the homogeneous one is removed, so the ChangeLog is
right (and matches the description above it in the commit msg).
>
> > Remove.
> > * src/c++11/string-inst.cc: Remove explicit instantiations of
> > operators that are no longer templates.
> > * src/c++23/std.cc.in (__gnu_cxx): Do not export operators for
> > __normal_iterator.
> > ---
> >
> > v2: removed the unnecessary operator<=>, removed std.cc exports, fixed
> > other minor issues noticed by Patrick.
> >
> > Tested x86_64-linux.
> >
> > libstdc++-v3/include/bits/stl_iterator.h | 327 ++++++++++++-----------
> > libstdc++-v3/src/c++11/string-inst.cc | 11 -
> > libstdc++-v3/src/c++23/std.cc.in | 9 -
> > 3 files changed, 169 insertions(+), 178 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/bits/stl_iterator.h
> > b/libstdc++-v3/include/bits/stl_iterator.h
> > index 478a98fe8a4f..a7188f46f6db 100644
> > --- a/libstdc++-v3/include/bits/stl_iterator.h
> > +++ b/libstdc++-v3/include/bits/stl_iterator.h
> > @@ -1164,188 +1164,199 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> > const _Iterator&
> > base() const _GLIBCXX_NOEXCEPT
> > { return _M_current; }
> > - };
> >
> > - // Note: In what follows, the left- and right-hand-side iterators are
> > - // allowed to vary in types (conceptually in cv-qualification) so that
> > - // comparison between cv-qualified and non-cv-qualified iterators be
> > - // valid. However, the greedy and unfriendly operators in std::rel_ops
> > - // will make overload resolution ambiguous (when in scope) if we don't
> > - // provide overloads whose operands are of the same type. Can someone
> > - // remind me what generic programming is about? -- Gaby
> > + private:
> > + // Note: In what follows, the left- and right-hand-side iterators are
> > + // allowed to vary in types (conceptually in cv-qualification) so
> > that
> > + // comparison between cv-qualified and non-cv-qualified iterators be
> > + // valid. However, the greedy and unfriendly operators in
> > std::rel_ops
> > + // will make overload resolution ambiguous (when in scope) if we
> > don't
> > + // provide overloads whose operands are of the same type. Can
> > someone
> > + // remind me what generic programming is about? -- Gaby
> >
> > #ifdef __cpp_lib_three_way_comparison
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - [[nodiscard, __gnu__::__always_inline__]]
> > - constexpr bool
> > - operator==(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - noexcept(noexcept(__lhs.base() == __rhs.base()))
> > - requires requires {
> > - { __lhs.base() == __rhs.base() } -> std::convertible_to<bool>;
> > - }
> > - { return __lhs.base() == __rhs.base(); }
> > + template<typename _Iter>
> > + [[nodiscard, __gnu__::__always_inline__]]
> > + friend
> > + constexpr bool
> > + operator==(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + noexcept(noexcept(__lhs.base() == __rhs.base()))
> > + requires requires {
> > + { __lhs.base() == __rhs.base() } -> std::convertible_to<bool>;
> > + }
> > + { return __lhs.base() == __rhs.base(); }
> >
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - [[nodiscard, __gnu__::__always_inline__]]
> > - constexpr std::__detail::__synth3way_t<_IteratorR, _IteratorL>
> > - operator<=>(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - noexcept(noexcept(std::__detail::__synth3way(__lhs.base(),
> > __rhs.base())))
> > - { return std::__detail::__synth3way(__lhs.base(), __rhs.base()); }
> > + [[nodiscard, __gnu__::__always_inline__]]
> > + friend
> > + constexpr bool
> > + operator==(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + noexcept(noexcept(__lhs.base() == __rhs.base()))
> > + requires requires {
> > + { __lhs.base() == __rhs.base() } -> std::convertible_to<bool>;
> > + }
> > + { return __lhs.base() == __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - [[nodiscard, __gnu__::__always_inline__]]
> > - constexpr bool
> > - operator==(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - noexcept(noexcept(__lhs.base() == __rhs.base()))
> > - requires requires {
> > - { __lhs.base() == __rhs.base() } -> std::convertible_to<bool>;
> > - }
> > - { return __lhs.base() == __rhs.base(); }
> > -
> > - template<typename _Iterator, typename _Container>
> > - [[nodiscard, __gnu__::__always_inline__]]
> > - constexpr std::__detail::__synth3way_t<_Iterator>
> > - operator<=>(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - noexcept(noexcept(std::__detail::__synth3way(__lhs.base(),
> > __rhs.base())))
> > - { return std::__detail::__synth3way(__lhs.base(), __rhs.base()); }
> > + template<typename _Iter>
> > + [[nodiscard, __gnu__::__always_inline__]]
> > + friend
> > + constexpr std::__detail::__synth3way_t<_Iterator, _Iter>
> > + operator<=>(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + noexcept(noexcept(std::__detail::__synth3way(__lhs.base(),
> > __rhs.base())))
> > + requires requires {
> > + std::__detail::__synth3way(__lhs.base(), __rhs.base());
> > + }
> > + { return std::__detail::__synth3way(__lhs.base(), __rhs.base()); }
> > #else
> > - // Forward iterator requirements
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator==(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() == __rhs.base(); }
> > + // Forward iterator requirements
> > + template<typename _Iter>
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator==(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() == __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator==(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() == __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator==(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() == __rhs.base(); }
> >
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator!=(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() != __rhs.base(); }
> > + template<typename _Iter>
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator!=(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() != __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator!=(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() != __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator!=(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() != __rhs.base(); }
> >
> > - // Random access iterator requirements
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator<(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() < __rhs.base(); }
> > + // Random access iterator requirements
> > + template<typename _Iter>
> > + friend
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > _GLIBCXX_CONSTEXPR
> > + inline bool
> > + operator<(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() < __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX20_CONSTEXPR
> > - inline bool
> > - operator<(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() < __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX20_CONSTEXPR
> > + bool
> > + operator<(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() < __rhs.base(); }
> >
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator>(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() > __rhs.base(); }
> > + template<typename _Iter>
> > + friend
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > _GLIBCXX_CONSTEXPR
> > + bool
> > + operator>(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() > __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator>(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() > __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator>(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() > __rhs.base(); }
> >
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator<=(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() <= __rhs.base(); }
> > + template<typename _Iter>
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator<=(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() <= __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator<=(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() <= __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator<=(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() <= __rhs.base(); }
> >
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator>=(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() >= __rhs.base(); }
> > + template<typename _Iter>
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator>=(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() >= __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline bool
> > - operator>=(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() >= __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + bool
> > + operator>=(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() >= __rhs.base(); }
> > #endif // three-way comparison
> >
> > - // _GLIBCXX_RESOLVE_LIB_DEFECTS
> > - // According to the resolution of DR179 not only the various comparison
> > - // operators but also operator- must accept mixed iterator/const_iterator
> > - // parameters.
> > - template<typename _IteratorL, typename _IteratorR, typename _Container>
> > + // _GLIBCXX_RESOLVE_LIB_DEFECTS
> > + // 179. Comparison of const_iterators to iterators doesn't work
> > + // According to the resolution of DR179 not only the various
> > comparison
> > + // operators but also operator- must accept mixed
> > iterator/const_iterator
> > + // parameters.
> > + template<typename _Iter>
> > #if __cplusplus >= 201103L
> > - // DR 685.
> > - [[__nodiscard__, __gnu__::__always_inline__]]
> > - constexpr auto
> > - operator-(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs) noexcept
> > - -> decltype(__lhs.base() - __rhs.base())
> > + [[__nodiscard__, __gnu__::__always_inline__]]
> > + friend
> > + // _GLIBCXX_RESOLVE_LIB_DEFECTS
> > + // 685. reverse_iterator/move_iterator difference has invalid
> > signatures
> > + constexpr auto
> > + operator-(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs) noexcept
> > + -> decltype(__lhs.base() - __rhs.base())
> > #else
> > - inline typename __normal_iterator<_IteratorL,
> > _Container>::difference_type
> > - operator-(const __normal_iterator<_IteratorL, _Container>& __lhs,
> > - const __normal_iterator<_IteratorR, _Container>& __rhs)
> > + friend
> > + difference_type
> > + operator-(const __normal_iterator& __lhs,
> > + const __normal_iterator<_Iter, _Container>& __rhs)
> > #endif
> > - { return __lhs.base() - __rhs.base(); }
> > + { return __lhs.base() - __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline typename __normal_iterator<_Iterator,
> > _Container>::difference_type
> > - operator-(const __normal_iterator<_Iterator, _Container>& __lhs,
> > - const __normal_iterator<_Iterator, _Container>& __rhs)
> > - _GLIBCXX_NOEXCEPT
> > - { return __lhs.base() - __rhs.base(); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + difference_type
> > + operator-(const __normal_iterator& __lhs, const __normal_iterator&
> > __rhs)
> > + _GLIBCXX_NOEXCEPT
> > + { return __lhs.base() - __rhs.base(); }
> >
> > - template<typename _Iterator, typename _Container>
> > - _GLIBCXX_NODISCARD __attribute__((__always_inline__))
> > _GLIBCXX_CONSTEXPR
> > - inline __normal_iterator<_Iterator, _Container>
> > - operator+(typename __normal_iterator<_Iterator,
> > _Container>::difference_type
> > - __n, const __normal_iterator<_Iterator, _Container>& __i)
> > - _GLIBCXX_NOEXCEPT
> > - { return __normal_iterator<_Iterator, _Container>(__i.base() + __n); }
> > + __attribute__((__always_inline__)) _GLIBCXX_NODISCARD
> > + friend
> > + _GLIBCXX_CONSTEXPR
> > + __normal_iterator
> > + operator+(difference_type __n, const __normal_iterator& __i)
> > + _GLIBCXX_NOEXCEPT
> > + { return __normal_iterator(__i.base() + __n); }
> > + };
> >
> > _GLIBCXX_END_NAMESPACE_VERSION
> > } // namespace __gnu_cxx
> > diff --git a/libstdc++-v3/src/c++11/string-inst.cc
> > b/libstdc++-v3/src/c++11/string-inst.cc
> > index 34df909b31a2..1056e646b12c 100644
> > --- a/libstdc++-v3/src/c++11/string-inst.cc
> > +++ b/libstdc++-v3/src/c++11/string-inst.cc
> > @@ -119,14 +119,3 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >
> > _GLIBCXX_END_NAMESPACE_VERSION
> > } // namespace
> > -
> > -namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
> > -{
> > -_GLIBCXX_BEGIN_NAMESPACE_VERSION
> > -
> > - using std::S;
> > - template bool operator==(const S::iterator&, const S::iterator&);
> > - template bool operator==(const S::const_iterator&, const
> > S::const_iterator&);
> > -
> > -_GLIBCXX_END_NAMESPACE_VERSION
> > -} // namespace
> > diff --git a/libstdc++-v3/src/c++23/std.cc.in
> > b/libstdc++-v3/src/c++23/std.cc.in
> > index 4cd3e52c8bbf..f1c3e2e4b284 100644
> > --- a/libstdc++-v3/src/c++23/std.cc.in
> > +++ b/libstdc++-v3/src/c++23/std.cc.in
> > @@ -1729,15 +1729,6 @@ export namespace std
> > using std::make_const_sentinel;
> > #endif
> > }
> > -// FIXME these should be friends of __normal_iterator to avoid exporting
> > -// __gnu_cxx.
> > -export namespace __gnu_cxx
> > -{
> > - using __gnu_cxx::operator==;
> > - using __gnu_cxx::operator<=>;
> > - using __gnu_cxx::operator+;
> > - using __gnu_cxx::operator-;
> > -}
> >
> > // <latch>
> > export namespace std
> > --
> > 2.49.0
> >
> >
>
