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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
