patch: fix typo-correction vs overload

[Nick Lewycky]

On 9 April 2013 15:59, Kaelyn Uhrain <rikka at google.com> wrote:

> 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.
>

Yes, the TypoCorrection does contain them all. I didn't do that because I
thought I would need to do a lookup on the correct name in order to trigger
instantiation of dependent return types, but that wasn't correct, and I've
included a testcase for that. What really happens is that we build an
UnresolvedMemberExpr now and then the necessary template instantiations are
performed in BuildCallToMemberFunction, after we have all the arguments.
(As with all things, this makes perfect sense in retrospect.)

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.
>

Yep, thanks for pointing that out. I took a look and while I'm not certain,
I also think it doesn't affect LookupMemberExprInRecord.

New patch attached!

Nick

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/20130506/d9266898/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: typo-member-expr-2.patch
Type: application/octet-stream
Size: 3705 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130506/d9266898/attachment.obj>