[PATCH] D123182: [Concepts] Fix issue #53640

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Can you improve the commit-message?  Title in particular, but more details would be greatly appreciated.



================
Comment at: clang/lib/Sema/SemaOverload.cpp:2955
 /// FunctionParamTypesAreEqual - This routine checks two function proto types
 /// for equality of their argument types. Caller has already checked that
+/// they have same number of parameters.  If the parameters are different,
----------------



================
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.
----------------
I don't really get what you mean for 'Reversed', can you better clarify?  Both in comments, and here?


================
Comment at: clang/lib/Sema/SemaOverload.cpp:2970
+           E = OldType->param_type_end();
+       O && (O != E); ++O, (Reversed ? --N : ++N)) {
     // Ignore address spaces in pointee type. This is to disallow overloading
----------------
Now I REALLY don't get it :) 


================
Comment at: clang/lib/Sema/SemaOverload.cpp:9829
+  bool CanCompareConstraints = false;
+  if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
+      Cand2.Function->hasPrototype()) {
----------------
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.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182