[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

Mon Apr 11 14:02:29 PDT 2022

royjacobson added inline comments.

================
Comment at: clang/lib/Sema/SemaOverload.cpp:2958
 /// ArgPos will have the parameter index of the first different parameter.
+/// If `Reversed` is true, exactly one of FT1 and FT2 is an overload
+/// candidate with a reversed parameter order.
----------------
erichkeane wrote:
> I don't really get what you mean for 'Reversed', can you better clarify?  Both in comments, and here?
I tried to modify the documentation a bit, I hope it's more understandable :)

C++20 added 'synthesized reverse operator overloads'. We support them in the overload resolution code by just remembering if we use a candidate with reverse order.

I need this here so the example in p6 with `operator==` works. It's a pretty annoying and weird corner case, but if it's explicitly in one of the standard's examples I might as well support it...

(BTW, GCC just don't implement this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105174)

================
Comment at: clang/lib/Sema/SemaOverload.cpp:9829
+  bool CanCompareConstraints = false;
+  if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
+      Cand2.Function->hasPrototype()) {
----------------
erichkeane wrote:
> Since the point of this is to just calculate the CanCompareConstraints, I think it should be a separate function called below where it is used.  
Do you mean as in a separate `Sema` function? Or a local lambda?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123182/new/

https://reviews.llvm.org/D123182