[cfe-commits] PATCH: Add support for C++ namespace-aware typo correcting

[Douglas Gregor]

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