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]]
| ^~~~~~~~~~~~~
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.
