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

[Kaelyn Uhrain]

And here is the second half, which changes the rest of the callers of
CorrectTypo, expands the unit test, adds a new test case to
test/FixIt/typo.cpp to verify the namespace qualifier correction is applied
properly, and removes the CorrectTypo compatibility wrapper. It should be
applied on top of the previous patch (ns-pt1-20110627.diff).

Both patches are based on the tip of http://llvm.org/git/clang as of ~3pm
PDT today. All unit tests pass both with only the first patch applied and
with this patch applied.

Cheers,
Kaelyn

On Mon, Jun 27, 2011 at 5:01 PM, Kaelyn Uhrain <rikka at google.com> wrote:

> 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/7fc62bfc/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ns-pt2-20110627.diff
Type: text/x-diff
Size: 43227 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110627/7fc62bfc/attachment.diff>