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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
