[PATCH] libstdc++: Fix constraint recursion in std::expected's operator== [PR119714]

[Tomasz Kaminski]

On Mon, May 12, 2025 at 3:01 PM Patrick Palka <ppalka@redhat.com> wrote:

> On Tue, 6 May 2025, Tomasz Kaminski wrote:
>
> >
> >
> > On Mon, May 5, 2025 at 8:50 PM Patrick Palka <ppalka@redhat.com> wrote:
> >       Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15?
> >
> > This LGTM.
> > Out of curiosity, would declaring them as members also fix the issue?
>
> Ah yes it does fix it, and in fact all of expected's operator==
> overloads can be converted to member functions.  But IIUC it's generally
> preferred in the stdlib to define operator== as non-members, so I guess
> we should still go with the below more narrow workaround.  Thanks for
> the reviews.
>
Agree on that point, but just wonder if the hidden friends policy is now
causing
us more trouble than good.

>
> >
> >       -- >8 --
> >
> >       This std::expected friend operator== is prone to constraint
> recursion
> >       after CWG 2369 for the same reason as basic_const_iterator's
> comparison
> >       operators were before the r15-7757-g4342c50ca84ae5 workaround.
> This
> >       patch works around the constraint recursion here in a similar
> manner,
> >       by making the function parameter of type std::expected dependent
> in a
> >       trivial way.
> >
> >               PR libstdc++/119714
> >               PR libstdc++/112490
> >
> >       libstdc++-v3/ChangeLog:
> >
> >               * include/std/expected (expected::operator==): Replace
> >               non-dependent std::expected function parameter with a
> dependent
> >               one of type expected<_Vp, _Er> where _Vp matches _Tp.
> >               * testsuite/20_util/expected/119714.cc: New test.
> >       ---
> >        libstdc++-v3/include/std/expected                 | 4 ++--
> >        libstdc++-v3/testsuite/20_util/expected/119714.cc | 9 +++++++++
> >        2 files changed, 11 insertions(+), 2 deletions(-)
> >        create mode 100644
> libstdc++-v3/testsuite/20_util/expected/119714.cc
> >
> >       diff --git a/libstdc++-v3/include/std/expected
> b/libstdc++-v3/include/std/expected
> >       index 5dc1dfbe5b8a..60f1565f15b9 100644
> >       --- a/libstdc++-v3/include/std/expected
> >       +++ b/libstdc++-v3/include/std/expected
> >       @@ -1169,13 +1169,13 @@ namespace __expected
> >                   return !__y.has_value() && bool(__x.error() ==
> __y.error());
> >               }
> >
> >       -      template<typename _Up>
> >       +      template<typename _Up, same_as<_Tp> _Vp>
> >               requires (!__expected::__is_expected<_Up>)
> >                 && requires (const _Tp& __t, const _Up& __u) {
> >                   { __t == __u } -> convertible_to<bool>;
> >                 }
> >               friend constexpr bool
> >       -       operator==(const expected& __x, const _Up& __v)
> >       +       operator==(const expected<_Vp, _Er>& __x, const _Up& __v)
> >               noexcept(noexcept(bool(*__x == __v)))
> >               { return __x.has_value() && bool(*__x == __v); }
> >
> >       diff --git a/libstdc++-v3/testsuite/20_util/expected/119714.cc
> b/libstdc++-v3/testsuite/20_util/expected/119714.cc
> >       new file mode 100644
> >       index 000000000000..a8dc6e891e1d
> >       --- /dev/null
> >       +++ b/libstdc++-v3/testsuite/20_util/expected/119714.cc
> >       @@ -0,0 +1,9 @@
> >       +// { dg-do compile { target c++23 } }
> >       +
> >       +// PR libstdc++/119714 - constraint recursion with
> std::expected::operator==
> >       +
> >       +#include <expected>
> >       +#include <vector>
> >       +
> >       +using I = std::vector<std::expected<int,int>>::iterator;
> >       +static_assert(std::totally_ordered<I>);
> >       --
> >       2.49.0.503.g6c0bd1fc70
> >
> >
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250512/cce96675/attachment.htm>