On Thu, 27 Feb 2025, Jonathan Wakely wrote:
> On Thu, 27 Feb 2025 at 20:52, Patrick Palka <[email protected]> wrote:
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk and perhaps
> > 14?
>
> OK for both.
Thanks.
>
> Do we want a library issue for this? The "buggy" code you're changing
> is exactly what the standard says.
I think so, I'll submit one. It seems like a GCC specific issue
only because we're still the only ones to implement CWG 2369 AFAICT.
>
>
> >
> > -- >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
> >
>
>
