Thanks for the heads-up, Doug. :) On Tue, Jul 12, 2011 at 6:30 PM, Douglas Gregor <[email protected]> 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 <[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
