<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jun 27, 2011, at 5:07 PM, Kaelyn Uhrain wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">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).<br>
<br>Both patches are based on the tip of <a href="http://llvm.org/git/clang">http://llvm.org/git/clang</a> as of ~3pm PDT today. All unit tests pass both with only the first patch applied and with this patch applied.<br></blockquote><div><br></div><div>FYI, there's a request for improving the diagnostics here:</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span><a href="http://llvm.org/bugs/show_bug.cgi?id=10318">http://llvm.org/bugs/show_bug.cgi?id=10318</a></div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><br><blockquote type="cite">
Cheers,<br>Kaelyn<br><br><div class="gmail_quote">On Mon, Jun 27, 2011 at 5:01 PM, Kaelyn Uhrain <span dir="ltr"><<a href="mailto:rikka@google.com">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
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.<br>
<br>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.<div>
<div></div><div class="h5"><br>
<br><div class="gmail_quote">On Fri, Jun 24, 2011 at 1:44 PM, Kaelyn Uhrain <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
<br>Cheers,<br><font color="#888888">Kaelyn</font><div><div></div><div><br><br><div class="gmail_quote">On Thu, Jun 23, 2011 at 5:11 PM, Kaelyn Uhrain <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+ // FIXME: Don't do the lookups if argument dependent lookup is required as<br>
+ // this will break test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp<br>
<br>
Is this FIXME still relevant, given that we're doing the appropriate requiresADL check?<br></blockquote></div><div><br>I've found a problem with the simple ADL check in CorrectTypo... with it, common cases we want to suggest corrections for such as:<br>
<br><div style="margin-left:40px">namespace fizbin {<br> namespace nested { bool moreFoobar() { return true; } }<br>}<br>void foo() {<br> moreFoobar();<br>}<br></div></div></div><br>give:<br><br><div style="margin-left:40px">
tmp.cpp:5:3: error: use of undeclared identifier 'moreFoobar'<br> moreFoobar();<br> ^<br>1 error generated.<br></div><br>instead of:<br><br><div style="margin-left:40px">tmp.cpp:5:3: error: use of undeclared identifier 'moreFoobar'; did you mean<br>
'fizbin::nested::moreFoobar'?<br> moreFoobar();<br> ^~~~~~~~~~<br> fizbin::nested::moreFoobar<br>tmp.cpp:2:27: note: 'fizbin::nested::moreFoobar' declared here<br> namespace nested { bool moreFoobar() { return true; } }<br>
</div><br>but without the check, basic.lookup.argdep/p4.cpp reports:<br><br><div style="margin-left:40px"><div>error: 'error' diagnostics seen but not expected: <br></div> Line 35: no viable conversion from 'B::B' to 'C::C'<div>
<br>
error: 'note' diagnostics seen but not expected: <br></div> Line 23: 'C::func' declared here<br> Line 22: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'B::B' to 'const C::C &' for 1st argument<br>
Line 23: passing argument to parameter here<br>4 errors generated.<br></div><br>Ideas?<br><font color="#888888"><br>- Kaelyn<br>
</font></blockquote></div><br>
</div></div></blockquote></div><br>
</div></div></blockquote></div><br>
<span><ns-pt2-20110627.diff></span></blockquote></div><br></body></html>