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

[Kaelyn Uhrain]

Thanks for the heads-up, Doug. :)

On Tue, Jul 12, 2011 at 6:30 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Jun 27, 2011, at 5:07 PM, Kaelyn Uhrain wrote:
>
> 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.
>
>
> FYI, there's a request for improving the diagnostics here:
>
> http://llvm.org/bugs/show_bug.cgi?id=10318
>
> - Doug
>
> 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
>>>>
>>>
>>>
>>
> <ns-pt2-20110627.diff>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110713/b5630729/attachment.html>