[cfe-commits] r148720 - in /cfe/trunk: lib/Sema/SemaLookup.cpp test/SemaCXX/typo-correction.cpp

[Kaelyn Uhrain]

Author: rikka
Date: Mon Jan 23 14:18:59 2012
New Revision: 148720

URL: http://llvm.org/viewvc/llvm-project?rev=148720&view=rev
Log:
In CorrectTypo, use the cached correction as a starting point instead.

Previously, for unqualified lookups, a positive cache hit is used as the
only non-keyword correction and a negative cache hit immediately returns
an empty TypoCorrection. With the new callback objects, this behavior
causes false negatives by not accounting for the fact that callback
objects alter the set of potential/allowed corrections. The new behavior
is to seed the set of corrections with the cached correction (for
positive hits) to estabilishing a baseline edit distance. Negative cache
hits are only stored or used when either no callback object is provided
or when it returns true for a call to ValidateCandidate with an empty
TypoCorrection (i.e. when ValidateCandidate does not seem to be doing
any checking of the TypoCorrection, such as when an instance of the base
callback class is used solely to specify the set of keywords to be accepted).

Modified:
    cfe/trunk/lib/Sema/SemaLookup.cpp
    cfe/trunk/test/SemaCXX/typo-correction.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=148720&r1=148719&r2=148720&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Jan 23 14:18:59 2012
@@ -3569,6 +3569,11 @@
 
   TypoCorrectionConsumer Consumer(*this, Typo);
 
+  // If a callback object returns true for an empty typo correction candidate,
+  // assume it does not do any actual validation of the candidates.
+  TypoCorrection EmptyCorrection;
+  bool ValidatingCallback = CCC && !CCC->ValidateCandidate(EmptyCorrection);
+
   // Perform name lookup to find visible, similarly-named entities.
   bool IsUnqualifiedLookup = false;
   if (MemberContext) {
@@ -3598,41 +3603,46 @@
     IsUnqualifiedLookup = true;
     UnqualifiedTyposCorrectedMap::iterator Cached
       = UnqualifiedTyposCorrected.find(Typo);
-    if (Cached == UnqualifiedTyposCorrected.end() ||
-        (Cached->second && CCC && !CCC->ValidateCandidate(Cached->second))) {
+    if (Cached != UnqualifiedTyposCorrected.end()) {
+      // Add the cached value, unless it's a keyword or fails validation. In the
+      // keyword case, we'll end up adding the keyword below.
+      if (Cached->second) {
+        if (!Cached->second.isKeyword() &&
+            (!CCC || CCC->ValidateCandidate(Cached->second)))
+          Consumer.addCorrection(Cached->second);
+      } else {
+        // Only honor no-correction cache hits when a callback that will validate
+        // correction candidates is not being used.
+        if (!ValidatingCallback)
+          return TypoCorrection();
+      }
+    }
+    if (Cached == UnqualifiedTyposCorrected.end()) {
       // Provide a stop gap for files that are just seriously broken.  Trying
       // to correct all typos can turn into a HUGE performance penalty, causing
       // some files to take minutes to get rejected by the parser.
       if (TyposCorrected + UnqualifiedTyposCorrected.size() >= 20)
         return TypoCorrection();
+    }
 
-      // For unqualified lookup, look through all of the names that we have
-      // seen in this translation unit.
-      for (IdentifierTable::iterator I = Context.Idents.begin(),
-                                  IEnd = Context.Idents.end();
-           I != IEnd; ++I)
-        Consumer.FoundName(I->getKey());
-
-      // Walk through identifiers in external identifier sources.
-      if (IdentifierInfoLookup *External
-                              = Context.Idents.getExternalIdentifierLookup()) {
-        llvm::OwningPtr<IdentifierIterator> Iter(External->getIdentifiers());
-        do {
-          StringRef Name = Iter->Next();
-          if (Name.empty())
-            break;
-
-          Consumer.FoundName(Name);
-        } while (true);
-      }
-    } else {
-      // Use the cached value, unless it's a keyword. In the keyword case, we'll
-      // end up adding the keyword below.
-      if (!Cached->second)
-        return TypoCorrection();
+    // For unqualified lookup, look through all of the names that we have
+    // seen in this translation unit.
+    for (IdentifierTable::iterator I = Context.Idents.begin(),
+                                IEnd = Context.Idents.end();
+         I != IEnd; ++I)
+      Consumer.FoundName(I->getKey());
+
+    // Walk through identifiers in external identifier sources.
+    if (IdentifierInfoLookup *External
+                            = Context.Idents.getExternalIdentifierLookup()) {
+      llvm::OwningPtr<IdentifierIterator> Iter(External->getIdentifiers());
+      do {
+        StringRef Name = Iter->Next();
+        if (Name.empty())
+          break;
 
-      if (!Cached->second.isKeyword())
-        Consumer.addCorrection(Cached->second);
+        Consumer.FoundName(Name);
+      } while (true);
     }
   }
 
@@ -3813,8 +3823,10 @@
   ED = Consumer.begin()->first;
 
   if (ED > 0 && Typo->getName().size() / ED < 3) {
-    // If this was an unqualified lookup, note that no correction was found.
-    if (IsUnqualifiedLookup)
+    // If this was an unqualified lookup and we believe the callback
+    // object wouldn't have filtered out possible corrections, note
+    // that no correction was found.
+    if (IsUnqualifiedLookup && !ValidatingCallback)
       (void)UnqualifiedTyposCorrected[Typo];
 
     return TypoCorrection();
@@ -3872,7 +3884,9 @@
     return BestResults["super"];
   }
 
-  if (IsUnqualifiedLookup)
+  // If this was an unqualified lookup and we believe the callback object did
+  // not filter out possible corrections, note that no correction was found.
+  if (IsUnqualifiedLookup && !ValidatingCallback)
     (void)UnqualifiedTyposCorrected[Typo];
 
   return TypoCorrection();

Modified: cfe/trunk/test/SemaCXX/typo-correction.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction.cpp?rev=148720&r1=148719&r2=148720&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/typo-correction.cpp (original)
+++ cfe/trunk/test/SemaCXX/typo-correction.cpp Mon Jan 23 14:18:59 2012
@@ -85,11 +85,26 @@
 
 // Test the typo-correction callback in Sema::DiagnoseUnknownTypeName.
 namespace unknown_type_test {
-  class StreamOut {}; // expected-note{{'StreamOut' declared here}}
-  long stream_count;
+  class StreamOut {}; // expected-note 2 {{'StreamOut' declared here}}
+  long stream_count; // expected-note 2 {{'stream_count' declared here}}
 };
 unknown_type_test::stream_out out; // expected-error{{no type named 'stream_out' in namespace 'unknown_type_test'; did you mean 'StreamOut'?}}
 
+// Demonstrate a case where using only the cached value returns the wrong thing
+// when the cached value was the result of a previous callback object that only
+// accepts a subset of the current callback object.
+namespace {
+using namespace unknown_type_test;
+void bar(long i);
+void before_caching_classname() {
+  bar((stream_out)); // expected-error{{use of undeclared identifier 'stream_out'; did you mean 'stream_count'?}}
+}
+stream_out out; // expected-error{{unknown type name 'stream_out'; did you mean 'StreamOut'?}}
+void after_caching_classname() {
+  bar((stream_out)); // expected-error{{use of undeclared identifier 'stream_out'; did you mean 'stream_count'?}}
+}
+}
+
 // Test the typo-correction callback in Sema::DiagnoseInvalidRedeclaration.
 struct BaseDecl {
   void add_in(int i);