[PATCH] D87350: [AST][RecoveryExpr] Fix a crash on undeduced type.
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 5 02:16:09 PDT 2020
hokein added inline comments.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:12880
return Result.getValueOr(QualType());
}
----------------
sammccall wrote:
> 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.
thanks, I think the behavior you suggest is more reasonable -- in the given example, go-to-def will yield two candidates, it is wired to catch one of the return type (not the other).
Addressed it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87350/new/
https://reviews.llvm.org/D87350
More information about the cfe-commits
mailing list