hokein added inline comments.
================ Comment at: clang/lib/Sema/SemaOverload.cpp:12814 // end up here. return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc, MultiExprArg(Args.data(), Args.size()), ---------------- sammccall wrote: > hokein wrote: > > sammccall wrote: > > > This results in (IIUC): > > > > > > ``` > > > CallExpr > > > >RecoveryExpr (indirection we inserted) > > > >>DeclRefExpr (callee) > > > >Arg1 > > > ``` > > > > > > Whereas when overload resolution fails, we create: > > > ``` > > > RecoveryExpr (call) > > > > OverloadExpr (callee) > > > > Arg1 > > > ``` > > > > > > I can see advantages for either, but is there a reason not to be > > > consistent? > > > (Which I guess means emitting the first one here) > > > (Which I guess means emitting the first one here) > > > > yes, exactly. > > > > reasons: > > - BuildCallExpr could emit diagnostics if we pass a non-dependent callee to > > it, this is something we want to avoid; > > - we'd save some cost -- `BuildCallExpr` has dedicated code path to handle > > dependent expr; > Sorry, I meant emitting the second one here. (Or changing the callexpr > recovery to use the first form, but that's a larger patch). > > I understand we save some code, but not very much (the second form is easy to > produce), and we produce quite different ASTs for essentially the same > pattern (the only difference is whether we attempted recovery or not). This > doesn't seem like a good trade... Changed to the second form for consistency. And it doesn't break any existing tests (neither improvements nor regressions). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89946/new/ https://reviews.llvm.org/D89946 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits