patch: fix typo-correction vs overload

[Kaelyn Uhrain]

The patch looks good to me for the most part. One thing you may want to try
though: the TypoCorrection *should* already contain all of the NamedDecls
needed for overload resolution (TypeCorrection::getCorrectionDecl just
returns the first NamedDecl if there are any associated with the
TypoCorrection, i.e. the correction is for an existing declaration and is
not a keyword), so you ought to be able to avoid the additional name lookup
by doing:

for (TypoCorrection::decl_iterator DI = Corrected.begin(), DIEnd =
Corrected.end(); DI != DIEnd; ++DI) {
  R.addDecl(*DI);
}

instead of calling "SemaRef.LookupQualifiedName(R, DC);"... that the
function was only adding the first NamedDecl to the LookupResult instead of
all of them was because I missed it when I improved the handling of typo
corrections in the presence of overloaded names.

And one important difference I thought of while writing the above: although
it probably doesn't affect LookupMemberExprInRecord right now since member
lookups occur before the function call information is available, the
NamedDecls in the TypoCorrection object would not necessarily be the same
set returned by LookupQualifiedName... they would have been filtered by
the CorrectionCandidateCallback object passed to CorrectTypo, reducing the
work that the overload resolution would have to do later and possibly
avoiding it altogether if the callback has enough information to exclude
all but one candidate. Calling LookupQualifiedName could lead to overload
resolution being done because it found multiple NamedDecls even when
CorrectTypo returned a correction with a single NamedDecl. Also, the
TypoCorrection may include a NestedNameSpecifier that is needed for the the
name to be visible from the given DeclContext (the "DC" variable), and in
those situations the call to LookupQualifiedName on the correction's
DeclarationName would fail.

On Mon, Apr 8, 2013 at 4:35 PM, Nick Lewycky <nlewycky at google.com> wrote:

> The problem is best explained with a testcase:
>
> struct A {};
> struct B {};
>
> struct S {
>   void method(A*);
>   void method(B*);
> };
>
> void test() {
>   B b;
>   S s;
>   s.methodd(&b);
> }
>
> we currently typo-correct the "s.methodd" to "s.method" but refer to the
> NamedDecl* for "void method(A*);" both in the note and when continuing to
> parse. That in turn causes an extra diagnostics (can't convert B* to A*),
> but worse in my mind is the fact that the parser isn't recovering as though
> the suggested replacement text were applied.
>
> Patch attached, now we recover by returning a LookupResult with all the
> overload candidates found by looking up the typo-corrected name, and we
> don't emit the note pointing to the previous decl(s) in case of overloaded
> results. Please review closely, I'm not very familiar with this part of
> clang.
>
> Nick
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130409/641822fa/attachment.html>