[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:12880
 
   return Result.getValueOr(QualType());
 }
----------------
hokein wrote:
> sammccall wrote:
> > Wouldn't we be better to handle undeduced type here rather than in the loop?
> > 
> > Not sure if it practically matters, but given: `auto f(); double f(int); f(0,0);`
> > the code in this patch will discard auto and yield `double`, while treating this as multiple candidates --> unknown return type seems more correct.
> > 
> I feel like yield `double` is better in your given example -- in this case, double is the only candidate, so it is reasonable to preserve the `double` type.
There are two overloads, one with `double` and one with an unknown type. They're both candidates for the function being called, and I'm not sure we have any reason to believe the first is more likely.

Two counterarguments I can think of:
 - the unknown type is probably also double, as overloads tend to have the same return type
 - the cost of being wrong isn't so high, so it's OK to not be totally sure

And maybe there are others... these are probably OK justifications but subtle so I'd like to see a comment tothat effect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87350