On 11/7/22 12:04, Jonathan Wakely wrote:
> On Mon, 7 Nov 2022 at 21:56, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> On Mon, 7 Nov 2022 at 20:58, Jason Merrill <jason@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.

>> 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.

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.  So, the below fixes the test, does 
it make sense to you?

Jason