[PATCH] D138749: [clang] Compare constraints before diagnosing mismatched ref qualifiers (GH58962)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I can't find any example that breaks and needs the `AreConstraintExpressionsEqual`, so feel free to do it if you'd like.



================
Comment at: clang/lib/Sema/SemaOverload.cpp:1324
+    if ((NewRC != nullptr) != (OldRC != nullptr))
+      // RC are most certainly different - these are overloads.
+      return true;
----------------
cor3ntin wrote:
> I know it's preexisting but, I'm not sure that comment adds anything.
> If we want to keep it, "requires clauses are different, - these are overloads."
The comment is definitely low quality here, I would probably prefer a little more elaboration if we're going to keep it.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:1327
+
+    if (NewRC) {
+      llvm::FoldingSetNodeID NewID, OldID;
----------------
I did some work that extracted these at one point, and it matters because we have to 'fix' the depths sometimes.  I suspect it wont' really come up here, since we have already checked that these are both the same 'type' of template parameter by now?  But at some point (perhaps not in this patch?) we might find calling ` Sema::AreConstraintExpressionsEqual` necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138749