On Jun 22, 2011, at 10:48 AM, Kaelyn Uhrain wrote:

> 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.

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.

A few small comments/questions:

+class SpecifierInfo {
+ public:
+  DeclContext* declContext;
+  NestedNameSpecifier* nameSpecifier;
+  unsigned editDistance;
+
+  SpecifierInfo(DeclContext *Ctx, NestedNameSpecifier *NNS, unsigned ED)
+      : declContext(Ctx), nameSpecifier(NNS), editDistance(ED) {}
+};

Public fields should start with an uppercase letter, per the LLVM coding 
standards.

+    // If there are results in the closest bucket, stop
+    if (!DI->second.empty()
+        /* TODO: Fix ObjC tests (in particular SemaObjC/ivar-ref-misuse.m) */
+        || (getLangOptions().ObjC1 && ED > 0))
+      break;

Is this still an issue to be resolved?

+    // Only perform the qualified lookups for C++
+    // 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?

... and one larger issue, which is that I'm seeing an infinite loop on this 
test case:

#include <vector>
#include <algorithm>

int main() {
  std::vector<int> v;
  (sort)(v.begin(), v.end());
}

in the typo-correction logic.

        - Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to