On Mon, 7 Nov 2022 at 22:49, Jason Merrill <ja...@redhat.com> wrote:
>
> On 11/7/22 12:04, Jonathan Wakely wrote:
> > On Mon, 7 Nov 2022 at 21:56, Jonathan Wakely <jwak...@redhat.com> wrote:
> >>
> >> On Mon, 7 Nov 2022 at 20:58, Jason Merrill <ja...@redhat.com> wrote:
> >>>
> >>> Tested x86_64-pc-linux-gnu. Jonathan, what do you want to do about the
> >>> library
> >>> test failure?
> >>>
> >>> -- >8 --
> >>>
> >>> This paper is resolving the problem of well-formed C++17 code becoming
> >>> ambiguous in C++20 due to asymmetrical operator== being compared with
> >>> itself
> >>> in reverse. I had previously implemented a tiebreaker such that if the
> >>> two
> >>> candidates were functions with the same parameter types, we would prefer
> >>> the
> >>> non-reversed candidate. But the committee went with a different approach:
> >>> if there's an operator!= with the same parameter types as the operator==,
> >>> don't consider the reversed form of the ==.
> >>>
> >>> So this patch implements that, and changes my old tiebreaker to give a
> >>> pedwarn if it is used. I also noticed that we were giving duplicate
> >>> errors
> >>> for some testcases, and fixed the tourney logic to avoid that.
> >>>
> >>> As a result, a lot of tests of the form
> >>>
> >>> struct A { bool operator==(const A&); };
> >>>
> >>> need to be fixed to add a const function-cv-qualifier, e.g.
> >>>
> >>> struct A { bool operator==(const A&) const; };
> >>>
> >>> The committee thought such code ought to be fixed, so breaking it was
> >>> fine.
> >>>
> >>> 18_support/comparisons/algorithms/fallback.cc also breaks with this patch,
> >>> because of the similarly asymmetrical
> >>>
> >>> bool operator==(const S&, S&) { return true; }
> >>>
> >>> I assume this was written this way deliberately, so I'm not sure what to
> >>> do
> >>> about it.
> >>
> >> Yes, that was deliberate. The compare_strong_order_fallback function
> >> has these constraints:
> >>
> >> template<typename _Tp, __decayed_same_as<_Tp> _Up>
> >> requires __strongly_ordered<_Tp, _Up> || __op_eq_lt<_Tp, _Up>
> >> constexpr strong_ordering
> >> operator() [[nodiscard]] (_Tp&& __e, _Up&& __f) const
> >>
> >> And similarly for the other fallbacks. So I wanted to check that two
> >> types that decay to the same type (like S and const S) can be compared
> >> with == and <, and therefore can be used with this function.
> >>
> >> But if such asymmetry is no longer allowed, maybe the library function
> >> is no longer usable with pathological cases like the test, and so the
> >> test should be changed. We can't just replace the decayed_same_as
> >> constraint with same_as because the std::strong_order customization
> >> point still supports similar types, but we could do:
> >>
> >> --- a/libstdc++-v3/libsupc++/compare
> >> +++ b/libstdc++-v3/libsupc++/compare
> >> @@ -1057,11 +1057,11 @@ namespace std _GLIBCXX_VISIBILITY(default)
> >> };
> >>
> >> template<typename _Tp, typename _Up>
> >> - concept __op_eq_lt = requires(_Tp&& __t, _Up&& __u)
> >> + concept __op_eq_lt = same_as<_Tp, _Up> && requires(_Tp&& __t)
> >> {
> >> - { static_cast<_Tp&&>(__t) == static_cast<_Up&&>(__u) }
> >> + { static_cast<_Tp&&>(__t) == static_cast<_Tp&&>(__t) }
> >> -> convertible_to<bool>;
> >> - { static_cast<_Tp&&>(__t) < static_cast<_Up&&>(__u) }
> >> + { static_cast<_Tp&&>(__t) < static_cast<_Tp&&>(__t) }
> >> -> convertible_to<bool>;
> >> };
> >
> > No wait, that's nonsense. We can still try to compare similar types,
> > it's just that they won't be comparable unless their comparison ops
> > have two parameters of the same type.
>
> Basically, though in this case the problem is that the arguments are the
> same type and the parameters are different.
Ah, so the operator== isn't actually rejected (despite being
pathologically dumb) it's just that some uses of it result in
ambiguities.
> >> And then adjust the test accordingly. If those fallbacks can no longer
> >> support mixed types, does the resolution of
> >> https://cplusplus.github.io/LWG/issue3465 even make sense now? If E
> >> and F must be the same type now, then E < F already implies F < E. I
> >> think we need some library changes to sync with P2468R2.
> >
> > I think this bit was right though. E and F might be different types,
> > but E < F implies F < E. Is that right?
>
> The P2468 changes only affect ==/!=, not <, so LWG3465 should be unaffected.
Gotcha.
> I think the only problem is the test itself: the <S,S> asserts need to
> be inverted because S and S cannot be compared for equality with the
> asymmetrical op== due to the normal candidate being better for arg 2 and
> the reversed candidate being better for arg 1. The <const S, S> asserts
> are fine because the arguments match the asymmetry, so the normal
> candidate is better for both args.
Gotcha.
> So, the below fixes the test, does
> it make sense to you?
Yep, looks good. Thanks.
