patch: fix typo-correction vs overload

[Kaelyn Uhrain]

On Mon, May 6, 2013 at 4:19 PM, Nick Lewycky <nlewycky at google.com> wrote:

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

Ah yes, that does indeed make sense. I had a feeling it was related to when
typo correction happened for members being used. :)


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

You're welcome! The new patch looks spot on to me. :)

Cheers,
Kaelyn


> 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/20130507/62ab0c28/attachment.html>