Re: [PATCH RFC(libstdc++)] c++: implement P2468R2, the equality operator you are looking for

[Jonathan Wakely <jwakely@redhat.com>]

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>;
       };


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.