On Wed, 2 Jul 2025, Jonathan Wakely wrote: > 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
Do you mean homogenous, not heterogeneous, here? > > > 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) D'oh, sorry about that. LGTM as-is > > > > > > > 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 > > > > > > > > > >