From 438ee1fc5e5baaa204faede83cb999e45bb6b57e Mon Sep 17 00:00:00 2001 From: Kaelyn Uhrain Date: Mon, 23 Jan 2012 20:18:59 +0000 Subject: [PATCH] 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 --- lib/Sema/SemaLookup.cpp | 74 +++++++++++++++++++------------- test/SemaCXX/typo-correction.cpp | 19 +++++++- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index dd50cd2a5a..23ecb25090 100644 --- 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 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 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 edf4c6460a..56417cd87d 100644 --- a/test/SemaCXX/typo-correction.cpp +++ b/test/SemaCXX/typo-correction.cpp @@ -85,11 +85,26 @@ template 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); -- 2.40.0