<div class="gmail_quote">On Wed, Jun 22, 2011 at 1:23 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com">dgregor@apple.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="im"><br>
On Jun 22, 2011, at 10:48 AM, Kaelyn Uhrain wrote:<br>
<br>
> So in the interest of keeping the overall changes a bit more digestible, I've split my dev branch in two. The first part, for which I've attached the latest diff, contains just the changes for the improved version of Sema::CorrectTypo and does not include the new unit test or any changes to any of the callers of CorrectTypo (all calls are going through the compatibility wrapper, and all unit tests pass). The second part, which I'm still working on (and which is already a third of the combined patch size), already has many of the callers modified to use the new version of CorrectTypo so I'd be fine with getting the first part submitted. Splitting it apart should also make the changes a bit easier to bisect if something breaks down the road.<br>

<br>
</div>Thanks for breaking this up. However, I'd like to have at least one test in the initial patch, so that we know that things are starting to work. Then follow-on commits/patches can switch over call sites to get the improved typo correction everywhere.<br>
</blockquote><div><br>Well the catch is that until at least one of the call sites are switched to the new version of CorrectTypo, the code would at best give an incorrect/incomplete suggestion--it would suggest using an identifier from another namespace without the proper qualifiers to make the identifier visible, so the suggestion would (in the fixit case) end up tripping another warning.  If you'd like I could fix up one of the CorrectTypo callers and add stripped-down version of missing-namespace-qualifier-typo-corrections.cpp that only tests that one instance.<br>
<br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<br>
A few small comments/questions:<br>
<br>
+class SpecifierInfo {<br>
+ public:<br>
+  DeclContext* declContext;<br>
+  NestedNameSpecifier* nameSpecifier;<br>
+  unsigned editDistance;<br>
+<br>
+  SpecifierInfo(DeclContext *Ctx, NestedNameSpecifier *NNS, unsigned ED)<br>
+      : declContext(Ctx), nameSpecifier(NNS), editDistance(ED) {}<br>
+};<br>
<br>
Public fields should start with an uppercase letter, per the LLVM coding standards.<br></blockquote><div><br>Whoops.  Fixed.<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<br>
+    // If there are results in the closest bucket, stop<br>
+    if (!DI->second.empty()<br>
+        /* TODO: Fix ObjC tests (in particular SemaObjC/ivar-ref-misuse.m) */<br>
+        || (getLangOptions().ObjC1 && ED > 0))<br>
+      break;<br>
<br>
Is this still an issue to be resolved?<br></blockquote><div><br>I'll test it and let you know. ;)<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<div class="im"><br>
+    // Only perform the qualified lookups for C++<br>
</div>+    // 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><br>Nope.  I've removed the FIXME.<br> <br></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">

<br>
... and one larger issue, which is that I'm seeing an infinite loop on this test case:<br>
<br>
#include <vector><br>
#include <algorithm><br>
<br>
int main() {<br>
  std::vector<int> v;<br>
  (sort)(v.begin(), v.end());<br>
}<br>
<br>
in the typo-correction logic.<br></blockquote><div><br>Interesting... looking into it.<br><br>Thanks!<br>Kaelyn<br></div></div>