[cfe-commits] PATCH: Add support for C++ namespace-aware typo correcting

[Kaelyn Uhrain]

So in hopes of having my changes finally committed soon, I've modified
basic.lookup.argdep/p4.cpp to expect the messages triggered by trying to
correct func(B::B()) to use C::func and failing on the conversions from
class B::B to C::C, along with a lengthy FIXME comment. Playing around with
the code a bit, there doesn't seem to be a good way to resolve this in the
clang code base as it is, since it would effectively require--in some
manner--checking the "corrected" function can work with the given arguments
before emitting any diagnostic messages about correcting the function name
or about attempted argument type conversions (including the lack of viable
copy-type constructors). I've also removed the requiresADL boolean from
CorrectTypo since it is no longer needed and was incorrect to begin with
since it only noted that an expression was a candidate for ADL, not that it
was determined ADL was definitely necessary.

Attached is the first patch with the changes to CorrectTypo and to one
caller of CorrectTypo (Sema::DiagnoseEmptyLookup in SemaExpr.cpp), along
with a basic unit test that exercises the new CorrectTypo behavior via
DiagnoseEmptyLookup.

On Fri, Jun 24, 2011 at 1:44 PM, Kaelyn Uhrain <rikka at google.com> wrote:

> FYI, I have all of the CorrectTypo call sites updated and everything
> working (including fixit mode, which didn't require mucking with the
> CXXScopeSpec [and said mucking screws up diagnostic messages]); I'm just
> waiting on guidance for resolving the issues with
> basic.lookup.argdep/p4.cpp. I'm tempted to just fix up the expectations in
> basic.lookup.argdep/p4.cpp, except the old error message is far more useful
> than the new and misleading/bogus messages and I'd like to figure out a way
> to keep the old behavior if possible without severely crippling the
> missing-namespace corrections.
>
> Cheers,
> Kaelyn
>
>
> On Thu, Jun 23, 2011 at 5:11 PM, Kaelyn Uhrain <rikka at google.com> wrote:
>
>> +    // FIXME: Don't do the lookups if argument dependent lookup is
>>> required as
>>> +    // this will break
>>> test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
>>>
>>> Is this FIXME still relevant, given that we're doing the appropriate
>>> requiresADL check?
>>>
>>
>> I've found a problem with the simple ADL check in CorrectTypo... with it,
>> common cases we want to suggest corrections for such as:
>>
>> namespace fizbin {
>>   namespace nested { bool moreFoobar() { return true; } }
>> }
>> void foo() {
>>   moreFoobar();
>> }
>>
>> give:
>>
>> tmp.cpp:5:3: error: use of undeclared identifier 'moreFoobar'
>>   moreFoobar();
>>   ^
>> 1 error generated.
>>
>> instead of:
>>
>> tmp.cpp:5:3: error: use of undeclared identifier 'moreFoobar'; did you
>> mean
>>       'fizbin::nested::moreFoobar'?
>>   moreFoobar();
>>   ^~~~~~~~~~
>>   fizbin::nested::moreFoobar
>> tmp.cpp:2:27: note: 'fizbin::nested::moreFoobar' declared here
>>   namespace nested { bool moreFoobar() { return true; } }
>>
>> but without the check, basic.lookup.argdep/p4.cpp reports:
>>
>> error: 'error' diagnostics seen but not expected:
>>   Line 35: no viable conversion from 'B::B' to 'C::C'
>>
>> error: 'note' diagnostics seen but not expected:
>>   Line 23: 'C::func' declared here
>>   Line 22: candidate constructor (the implicit copy constructor) not
>> viable: no known conversion from 'B::B' to 'const C::C &' for 1st argument
>>   Line 23: passing argument to parameter here
>> 4 errors generated.
>>
>> Ideas?
>>
>> - Kaelyn
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110627/21e31235/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ns-pt1-20110627.diff
Type: text/x-diff
Size: 51906 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110627/21e31235/attachment.diff>