[PATCH] D89946: [clang] Suppress "follow-up" diagnostics on recovery call expressions.

[Sam McCall via Phabricator via cfe-commits]

sammccall 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()),
----------------
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...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89946