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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@148720 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index dd50cd2a5a25f34ca9226f0178a92303c3bb5f45..23ecb2509030db95d4f80203649a837fb6d96cc1 100644 (file)
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -3569,6 +3569,11 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName,
 
   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 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName,
     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;
+    // 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());
 
-          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();
+    // 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 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName,
   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 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName,
     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();

diff --git a/test/SemaCXX/typo-correction.cpp b/test/SemaCXX/typo-correction.cpp
index edf4c6460a7912c30397ba2060a67449c58c77c8..56417cd87dc5949cf3d0643c0d720b8dbb0de592 100644 (file)
--- a/test/SemaCXX/typo-correction.cpp
+++ b/test/SemaCXX/typo-correction.cpp
@@ -85,11 +85,26 @@ template<typename ...TypeNames> struct count { // expected-note{{parameter pack
 
 // 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);