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

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson marked 3 inline comments as done.
royjacobson added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:1324
+    if ((NewRC != nullptr) != (OldRC != nullptr))
+      // RC are most certainly different - these are overloads.
+      return true;
----------------
erichkeane wrote:
> 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.
removed it.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:1327
+
+    if (NewRC) {
+      llvm::FoldingSetNodeID NewID, OldID;
----------------
erichkeane wrote:
> 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.
Went for `Sema::AreConstraintExpressionsEqual` to reduce code duplication. Only drawback is calling `CalculateTemplateDepthForConstraints` unnecessarily which seems reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138749