On Thu, 27 Feb 2025 at 20:52, Patrick Palka <ppa...@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
> 14?

OK for both.

Do we want a library issue for this? The "buggy" code you're changing
is exactly what the standard says.


>
> -- >8 --
>
> Here for
>
>   using RCI = reverse_iterator<basic_const_iterator<vector<int>::iterator>>
>   static_assert(std::totally_ordered<RCI>);
>
> we effectively need to check the requirement
>
>   requires (RCI x) { x RELOP x; }  for each RELOP in {<, >, <=, >=}
>
> which we expect to be straightforwardly satisfied by reverse_iterator's
> namespace-scope relops.  But due to ADL we find ourselves also
> considering the basic_const_iterator relop friends, which before CWG
> 2369 would be quickly discarded since RCI clearly isn't convertible to
> basic_const_iterator.  After CWG 2369 though we must first check these
> relops' constraints (with _It = vector<int>::iterator and _It2 = RCI),
> which entails checking totally_ordered<RCI> recursively.
>
> This patch fixes this by turning the problematic non-dependent parameter
> of type basic_const_iterator<_It> into a dependent parameter of type
> basic_const_iterator<_It3> where _It3 is constrained to match _It.
> Thus the basic_const_iterator relop friends now get quickly discarded
> during deduction since RCI isn't a specialization of basic_const_iterator
> (or derived from one) and so _It3 is not deduced.
>
>         PR libstdc++/112490
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/stl_iterator.h (basic_const_iterator::operator<):
>         Replace non-dependent basic_const_iterator parameter with
>         a dependent one of type basic_const_iterator<_It3>.  Constrain
>         _It3 to match _It.
>         (basic_const_iterator::operator>): Likewise.
>         (basic_const_iterator::operator<=): Likewise.
>         (basic_const_iterator::operator>=): Likewise.
>         * testsuite/24_iterators/const_iterator/112490.cc: New test.
> ---
>  libstdc++-v3/include/bits/stl_iterator.h         | 16 ++++++++--------
>  .../24_iterators/const_iterator/112490.cc        | 12 ++++++++++++
>  2 files changed, 20 insertions(+), 8 deletions(-)
>  create mode 100644 
> libstdc++-v3/testsuite/24_iterators/const_iterator/112490.cc
>
> diff --git a/libstdc++-v3/include/bits/stl_iterator.h 
> b/libstdc++-v3/include/bits/stl_iterator.h
> index 3e025342fb5..33732b1a428 100644
> --- a/libstdc++-v3/include/bits/stl_iterator.h
> +++ b/libstdc++-v3/include/bits/stl_iterator.h
> @@ -2881,30 +2881,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>         && three_way_comparable_with<_It, _It2>
>        { return _M_current <=> __y; }
>
> -    template<__detail::__not_a_const_iterator _It2>
> +    template<__detail::__not_a_const_iterator _It2, same_as<_It> _It3>
>        friend constexpr bool
> -      operator<(const _It2& __x, const basic_const_iterator& __y)
> +      operator<(const _It2& __x, const basic_const_iterator<_It3>& __y)
>        noexcept(noexcept(__x < __y._M_current))
>        requires random_access_iterator<_It> && totally_ordered_with<_It, _It2>
>        { return __x < __y._M_current; }
>
> -    template<__detail::__not_a_const_iterator _It2>
> +    template<__detail::__not_a_const_iterator _It2, same_as<_It> _It3>
>        friend constexpr bool
> -      operator>(const _It2& __x, const basic_const_iterator& __y)
> +      operator>(const _It2& __x, const basic_const_iterator<_It3>& __y)
>        noexcept(noexcept(__x > __y._M_current))
>        requires random_access_iterator<_It> && totally_ordered_with<_It, _It2>
>        { return __x > __y._M_current; }
>
> -    template<__detail::__not_a_const_iterator _It2>
> +    template<__detail::__not_a_const_iterator _It2, same_as<_It> _It3>
>        friend constexpr bool
> -      operator<=(const _It2& __x, const basic_const_iterator& __y)
> +      operator<=(const _It2& __x, const basic_const_iterator<_It3>& __y)
>        noexcept(noexcept(__x <= __y._M_current))
>        requires random_access_iterator<_It> && totally_ordered_with<_It, _It2>
>        { return __x <= __y._M_current; }
>
> -    template<__detail::__not_a_const_iterator _It2>
> +    template<__detail::__not_a_const_iterator _It2, same_as<_It> _It3>
>        friend constexpr bool
> -      operator>=(const _It2& __x, const basic_const_iterator& __y)
> +      operator>=(const _It2& __x, const basic_const_iterator<_It3>& __y)
>        noexcept(noexcept(__x >= __y._M_current))
>        requires random_access_iterator<_It> && totally_ordered_with<_It, _It2>
>        { return __x >= __y._M_current; }
> diff --git a/libstdc++-v3/testsuite/24_iterators/const_iterator/112490.cc 
> b/libstdc++-v3/testsuite/24_iterators/const_iterator/112490.cc
> new file mode 100644
> index 00000000000..9bb154847cf
> --- /dev/null
> +++ b/libstdc++-v3/testsuite/24_iterators/const_iterator/112490.cc
> @@ -0,0 +1,12 @@
> +// { dg-do compile { target c++23 } }
> +
> +// PR libstdc++/112490 - infinite meta error in
> +// reverse_iterator<basic_const_iterator<vector<int>::iterator>>
> +
> +#include <iterator>
> +#include <vector>
> +
> +using I = std::vector<int>::iterator;
> +using CI = std::basic_const_iterator<I>;
> +using RCI = std::reverse_iterator<CI>;
> +static_assert(std::totally_ordered<RCI>);
> --
> 2.49.0.rc0
>

Reply via email to