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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits