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

Reply via email to