On Thu, 12 Jun 2025 at 09:34, Daniel Krügler <daniel.krueg...@gmail.com> wrote:
>
> Am Do., 12. Juni 2025 um 10:22 Uhr schrieb Jonathan Wakely
> <jwak...@redhat.com>:
>>
>> On Thu, 12 Jun 2025 at 09:21, Jonathan Wakely <jwak...@redhat.com> wrote:
>> >
>> > On Thu, 12 Jun 2025 at 08:12, Daniel Krügler <daniel.krueg...@gmail.com>
>> > wrote:
>> > >
>> > > Am Do., 12. Juni 2025 um 00:10 Uhr schrieb Jonathan Wakely
>> > > <jwak...@redhat.com>:
>> > >>
>> > >> 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>)):
>> > >> 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
>> > >
>> > >
>> > > Just curious: So in this case the attribute order intentionally
>> > > potentially deviates from the order of the other friend functions using
>> > > [[nodiscard, __gnu__::__always_inline__]]?
>> >
>> > Yes because operator== needs to compile as C++98 so it uses
>> > __attribute__((always_inline)) not [[gnu::always_inline]], and Clang
>> > will only allow __attribute__((foo)) [[bar]] and not the other way
>> > around:
>> > https://godbolt.org/z/nGWss9YG3
>> >
>> > <source>:5:36: error: an attribute list cannot appear here
>> > 5 | __attribute__((always_inline)) [[nodiscard]]
>> > | ^~~~~~~~~~~~~
>>
>> I'll update the commit msg to note that the order is needed for Clang,
>> as GCC seems happy with either order.
>>
>> >
>> > We could change [[nodiscard,gnu::always_inline]] to be in the opposite
>> > order, but when either order works, I prefer to have the standard
>> > attribute first and the non-standard one second.
>
>
> That would be helpful - thanks,
It now says:
I also had to reorder the __attribute__((always_inline)) and
[[nodiscard]] attributes on the pre-c++20 operators, because Clang won't
allow [[foo]] after __attribute__((bar)) on a friend function:
<source>:4:36: error: an attribute list cannot appear here
4 | __attribute__((always_inline)) [[nodiscard]] friend bool
| ^~~~~~~~~~~~~
