From: Richard Smith Date: Tue, 20 Aug 2013 20:35:18 +0000 (+0000) Subject: During typo correction, check for an exact match in an unimported module. If we X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d67679d7439bd17b06574781b908630f4640c662;p=clang During typo correction, check for an exact match in an unimported module. If we find one, then report the error as a missing import instead of as a typo. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@188821 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index f5e1c89973..2cf364933c 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6637,6 +6637,8 @@ def err_module_private_local : Error< def err_module_private_local_class : Error< "local %select{struct|interface|union|class|enum}0 cannot be declared " "__module_private__">; +def err_module_private_declaration : Error< + "declaration of %0 must be imported from module '%1' before it is required">; def err_module_private_definition : Error< "definition of %0 must be imported from module '%1' before it is required">; } diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h index 3ab177b72e..d22b219d9b 100644 --- a/include/clang/Sema/Lookup.h +++ b/include/clang/Sema/Lookup.h @@ -690,6 +690,11 @@ public: /// \brief Destroys the visible declaration consumer. virtual ~VisibleDeclConsumer(); + /// \brief Determine whether hidden declarations (from unimported + /// modules) should be given to this consumer. By default, they + /// are not included. + virtual bool includeHiddenDecls() const; + /// \brief Invoked each time \p Sema::LookupVisibleDecls() finds a /// declaration visible from the current scope or context. /// diff --git a/include/clang/Sema/TypoCorrection.h b/include/clang/Sema/TypoCorrection.h index b4065be743..0deb74201c 100644 --- a/include/clang/Sema/TypoCorrection.h +++ b/include/clang/Sema/TypoCorrection.h @@ -43,7 +43,8 @@ public: unsigned QualifierDistance = 0) : CorrectionName(Name), CorrectionNameSpec(NNS), CharDistance(CharDistance), QualifierDistance(QualifierDistance), - CallbackDistance(0), ForceSpecifierReplacement(false) { + CallbackDistance(0), ForceSpecifierReplacement(false), + RequiresImport(false) { if (NameDecl) CorrectionDecls.push_back(NameDecl); } @@ -52,7 +53,7 @@ public: unsigned CharDistance = 0) : CorrectionName(Name->getDeclName()), CorrectionNameSpec(NNS), CharDistance(CharDistance), QualifierDistance(0), CallbackDistance(0), - ForceSpecifierReplacement(false) { + ForceSpecifierReplacement(false), RequiresImport(false) { if (Name) CorrectionDecls.push_back(Name); } @@ -61,11 +62,12 @@ public: unsigned CharDistance = 0) : CorrectionName(Name), CorrectionNameSpec(NNS), CharDistance(CharDistance), QualifierDistance(0), CallbackDistance(0), - ForceSpecifierReplacement(false) {} + ForceSpecifierReplacement(false), RequiresImport(false) {} TypoCorrection() : CorrectionNameSpec(0), CharDistance(0), QualifierDistance(0), - CallbackDistance(0), ForceSpecifierReplacement(false) {} + CallbackDistance(0), ForceSpecifierReplacement(false), + RequiresImport(false) {} /// \brief Gets the DeclarationName of the typo correction DeclarationName getCorrection() const { return CorrectionName; } @@ -134,13 +136,19 @@ public: DeclClass *getCorrectionDeclAs() const { return dyn_cast_or_null(getCorrectionDecl()); } - + /// \brief Clears the list of NamedDecls before adding the new one. void setCorrectionDecl(NamedDecl *CDecl) { CorrectionDecls.clear(); addCorrectionDecl(CDecl); } + /// \brief Clears the list of NamedDecls and adds the given set. + void setCorrectionDecls(ArrayRef Decls) { + CorrectionDecls.clear(); + CorrectionDecls.insert(CorrectionDecls.begin(), Decls.begin(), Decls.end()); + } + /// \brief Add the given NamedDecl to the list of NamedDecls that are the /// declarations associated with the DeclarationName of this TypoCorrection void addCorrectionDecl(NamedDecl *CDecl); @@ -206,6 +214,11 @@ public: } const_decl_iterator end() const { return CorrectionDecls.end(); } + /// \brief Returns whether this typo correction is correcting to a + /// declaration that was declared in a module that has not been imported. + bool requiresImport() const { return RequiresImport; } + void setRequiresImport(bool Req) { RequiresImport = Req; } + private: bool hasCorrectionDecl() const { return (!isKeyword() && !CorrectionDecls.empty()); @@ -220,6 +233,7 @@ private: unsigned CallbackDistance; SourceRange CorrectionRange; bool ForceSpecifierReplacement; + bool RequiresImport; }; /// @brief Base class for callback objects used by Sema::CorrectTypo to check diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 07379d5100..727f49b5fa 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -1229,13 +1229,13 @@ bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) { /// /// \returns D, or a visible previous declaration of D, whichever is more recent /// and visible. If no declaration of D is visible, returns null. -NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { - assert(!isVisible(SemaRef, D) && "not in slow case"); +static NamedDecl *findAcceptableDecl(Sema &SemaRef, NamedDecl *D) { + assert(!LookupResult::isVisible(SemaRef, D) && "not in slow case"); for (Decl::redecl_iterator RD = D->redecls_begin(), RDEnd = D->redecls_end(); RD != RDEnd; ++RD) { if (NamedDecl *ND = dyn_cast(*RD)) { - if (isVisible(SemaRef, ND)) + if (LookupResult::isVisible(SemaRef, ND)) return ND; } } @@ -1243,6 +1243,10 @@ NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { return 0; } +NamedDecl *LookupResult::getAcceptableDeclSlow(NamedDecl *D) const { + return findAcceptableDecl(SemaRef, D); +} + /// @brief Perform unqualified name lookup starting from a given /// scope. /// @@ -2900,6 +2904,8 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, bool Operator, //---------------------------------------------------------------------------- VisibleDeclConsumer::~VisibleDeclConsumer() { } +bool VisibleDeclConsumer::includeHiddenDecls() const { return false; } + namespace { class ShadowContextRAII; @@ -3194,7 +3200,7 @@ static void LookupVisibleDecls(Scope *S, LookupResult &Result, Result.getNameLoc(), Sema::LookupMemberName); if (ObjCInterfaceDecl *IFace = Method->getClassInterface()) { LookupVisibleDecls(IFace, IvarResult, /*QualifiedNameLookup=*/false, - /*InBaseClass=*/false, Consumer, Visited); + /*InBaseClass=*/false, Consumer, Visited); } } @@ -3259,6 +3265,7 @@ void Sema::LookupVisibleDecls(Scope *S, LookupNameKind Kind, // Look for visible declarations. LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind); + Result.setAllowHidden(Consumer.includeHiddenDecls()); VisibleDeclsRecord Visited; if (!IncludeGlobalScope) Visited.visitedContext(Context.getTranslationUnitDecl()); @@ -3270,6 +3277,7 @@ void Sema::LookupVisibleDecls(DeclContext *Ctx, LookupNameKind Kind, VisibleDeclConsumer &Consumer, bool IncludeGlobalScope) { LookupResult Result(*this, DeclarationName(), SourceLocation(), Kind); + Result.setAllowHidden(Consumer.includeHiddenDecls()); VisibleDeclsRecord Visited; if (!IncludeGlobalScope) Visited.visitedContext(Context.getTranslationUnitDecl()); @@ -3339,7 +3347,9 @@ class TypoCorrectionConsumer : public VisibleDeclConsumer { public: explicit TypoCorrectionConsumer(Sema &SemaRef, IdentifierInfo *Typo) : Typo(Typo->getName()), - SemaRef(SemaRef) { } + SemaRef(SemaRef) {} + + bool includeHiddenDecls() const { return true; } virtual void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx, bool InBaseClass); @@ -3390,6 +3400,12 @@ void TypoCorrectionConsumer::FoundDecl(NamedDecl *ND, NamedDecl *Hiding, if (!Name) return; + // Only consider visible declarations and declarations from modules with + // names that exactly match. + if (!LookupResult::isVisible(SemaRef, ND) && Name->getName() != Typo && + !findAcceptableDecl(SemaRef, ND)) + return; + FoundName(Name->getName()); } @@ -3664,10 +3680,12 @@ static void LookupPotentialTypoResult(Sema &SemaRef, Scope *S, CXXScopeSpec *SS, DeclContext *MemberContext, bool EnteringContext, - bool isObjCIvarLookup) { + bool isObjCIvarLookup, + bool FindHidden) { Res.suppressDiagnostics(); Res.clear(); Res.setLookupName(Name); + Res.setAllowHidden(FindHidden); if (MemberContext) { if (ObjCInterfaceDecl *Class = dyn_cast(MemberContext)) { if (isObjCIvarLookup) { @@ -3858,6 +3876,50 @@ static bool isCandidateViable(CorrectionCandidateCallback &CCC, return Candidate.getEditDistance(false) != TypoCorrection::InvalidDistance; } +/// \brief Check whether the declarations found for a typo correction are +/// visible, and if none of them are, convert the correction to an 'import +/// a module' correction. +static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC, + DeclarationName TypoName) { + if (TC.begin() == TC.end()) + return; + + TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end(); + + for (/**/; DI != DE; ++DI) + if (!LookupResult::isVisible(SemaRef, *DI)) + break; + // Nothing to do if all decls are visible. + if (DI == DE) + return; + + llvm::SmallVector NewDecls(TC.begin(), DI); + bool AnyVisibleDecls = !NewDecls.empty(); + + for (/**/; DI != DE; ++DI) { + NamedDecl *VisibleDecl = *DI; + if (!LookupResult::isVisible(SemaRef, *DI)) + VisibleDecl = findAcceptableDecl(SemaRef, *DI); + + if (VisibleDecl) { + if (!AnyVisibleDecls) { + // Found a visible decl, discard all hidden ones. + AnyVisibleDecls = true; + NewDecls.clear(); + } + NewDecls.push_back(VisibleDecl); + } else if (!AnyVisibleDecls && !(*DI)->isModulePrivate()) + NewDecls.push_back(*DI); + } + + if (NewDecls.empty()) + TC = TypoCorrection(); + else { + TC.setCorrectionDecls(NewDecls); + TC.setRequiresImport(!AnyVisibleDecls); + } +} + /// \brief Try to "correct" a typo in the source code by finding /// visible declarations whose names are similar to the name that was /// present in the source code. @@ -4000,7 +4062,7 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName, bool SearchNamespaces = getLangOpts().CPlusPlus && (IsUnqualifiedLookup || (QualifiedDC && QualifiedDC->isNamespace())); - // In a few cases we *only* want to search for corrections bases on just + // In a few cases we *only* want to search for corrections based on just // adding or changing the nested name specifier. bool AllowOnlyNNSChanges = Typo->getName().size() < 3; @@ -4122,7 +4184,9 @@ TypoCorrection Sema::CorrectTypo(const DeclarationNameInfo &TypoName, retry_lookup: LookupPotentialTypoResult(*this, TmpRes, Name, S, TempSS, TempMemberContext, EnteringContext, - CCC.IsObjCIvarLookup); + CCC.IsObjCIvarLookup, + Name == TypoName.getName() && + !Candidate.WillReplaceSpecifier()); switch (TmpRes.getResultKind()) { case LookupResult::NotFound: @@ -4271,6 +4335,7 @@ retry_lookup: TypoCorrection TC = Result; TC.setCorrectionRange(SS, TypoName); + checkCorrectionVisibility(*this, TC, TypoName.getName()); return TC; } else if (BestResults.size() > 1 @@ -4391,6 +4456,24 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction, ErrorRecovery); } +/// Find which declaration we should import to provide the definition of +/// the given declaration. +static const NamedDecl *getDefinitionToImport(const NamedDecl *D) { + if (const VarDecl *VD = dyn_cast(D)) + return VD->getDefinition(); + if (const FunctionDecl *FD = dyn_cast(D)) + return FD->isDefined(FD) ? FD : 0; + if (const TagDecl *TD = dyn_cast(D)) + return TD->getDefinition(); + if (const ObjCInterfaceDecl *ID = dyn_cast(D)) + return ID->getDefinition(); + if (const ObjCProtocolDecl *PD = dyn_cast(D)) + return PD->getDefinition(); + if (const TemplateDecl *TD = dyn_cast(D)) + return getDefinitionToImport(TD->getTemplatedDecl()); + return 0; +} + /// \brief Diagnose a successfully-corrected typo. Separated from the correction /// itself to allow external validation of the result, etc. /// @@ -4412,6 +4495,31 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction, FixItHint FixTypo = FixItHint::CreateReplacement( Correction.getCorrectionRange(), CorrectedStr); + // Maybe we're just missing a module import. + if (Correction.requiresImport()) { + NamedDecl *Decl = Correction.getCorrectionDecl(); + assert(Decl && "import required but no declaration to import"); + + // Suggest importing a module providing the definition of this entity, if + // possible. + const NamedDecl *Def = getDefinitionToImport(Decl); + if (!Def) + Def = Decl; + Module *Owner = Def->getOwningModule(); + assert(Owner && "definition of hidden declaration is not in a module"); + + Diag(Correction.getCorrectionRange().getBegin(), + diag::err_module_private_declaration) + << Def << Owner->getFullModuleName(); + Diag(Def->getLocation(), diag::note_previous_declaration); + + // Recover by implicitly importing this module. + if (!isSFINAEContext() && ErrorRecovery) + createImplicitModuleImport(Correction.getCorrectionRange().getBegin(), + Owner); + return; + } + Diag(Correction.getCorrectionRange().getBegin(), TypoDiag) << CorrectedQuotedStr << (ErrorRecovery ? FixTypo : FixItHint()); diff --git a/test/Modules/auto-module-import.m b/test/Modules/auto-module-import.m index 7351828182..23bb63b924 100644 --- a/test/Modules/auto-module-import.m +++ b/test/Modules/auto-module-import.m @@ -1,10 +1,12 @@ // RUN: rm -rf %t +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -F %S/Inputs %s -verify -DERRORS // RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -F %S/Inputs %s -verify +// +// Test both with and without the declarations that refer to unimported +// entities. For error recovery, those cases implicitly trigger an import. #include // expected-warning{{treating #include as an import of module 'DependsOnModule'}} -// expected-note@Inputs/NoUmbrella.framework/PrivateHeaders/A_Private.h:1{{'no_umbrella_A_private' declared here}} - #ifdef MODULE_H_MACRO # error MODULE_H_MACRO should have been hidden #endif @@ -13,15 +15,21 @@ # error DEPENDS_ON_MODULE should have been hidden #endif -Module *mod; // expected-error{{unknown type name 'Module'}} - +#ifdef ERRORS +Module *mod; // expected-error{{declaration of 'Module' must be imported from module 'Module' before it is required}} +// expected-note@Inputs/Module.framework/Headers/Module.h:15 {{previous}} +#else #import // expected-warning{{treating #import as an import of module 'AlsoDependsOnModule'}} +#endif Module *mod2; int getDependsOther() { return depends_on_module_other; } void testSubframeworkOther() { - double *sfo1 = sub_framework_other; // expected-error{{use of undeclared identifier 'sub_framework_other'}} +#ifdef ERRORS + double *sfo1 = sub_framework_other; // expected-error{{declaration of 'sub_framework_other' must be imported from module 'DependsOnModule.SubFramework.Other'}} + // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/Other.h:15 {{previous}} +#endif } // Test umbrella-less submodule includes @@ -32,8 +40,10 @@ int getNoUmbrellaA() { return no_umbrella_A; } #include // expected-warning{{treating #include as an import of module 'NoUmbrella.SubDir.C'}} int getNoUmbrellaC() { return no_umbrella_C; } +#ifndef ERRORS // Test header cross-subframework include pattern. #include // expected-warning{{treating #include as an import of module 'DependsOnModule.SubFramework.Other'}} +#endif void testSubframeworkOtherAgain() { double *sfo1 = sub_framework_other; @@ -61,7 +71,8 @@ int getModulePrivate() { return module_private; } #include // expected-warning{{treating #include as an import of module 'NoUmbrella.Private.A_Private'}} int getNoUmbrellaAPrivate() { return no_umbrella_A_private; } -int getNoUmbrellaBPrivateFail() { return no_umbrella_B_private; } // expected-error{{use of undeclared identifier 'no_umbrella_B_private'; did you mean 'no_umbrella_A_private'?}} +int getNoUmbrellaBPrivateFail() { return no_umbrella_B_private; } // expected-error{{declaration of 'no_umbrella_B_private' must be imported from module 'NoUmbrella.Private.B_Private'}} +// expected-note@Inputs/NoUmbrella.framework/PrivateHeaders/B_Private.h:1 {{previous}} // Test inclusion of headers that are under an umbrella directory but // not actually part of the module. diff --git a/test/Modules/decldef.m b/test/Modules/decldef.m index 7ed82b57e9..efd2d7c42c 100644 --- a/test/Modules/decldef.m +++ b/test/Modules/decldef.m @@ -1,13 +1,16 @@ // RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_EARLY // RUN: %clang_cc1 -fmodules -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -// expected-note@Inputs/def.h:5 {{previous definition is here}} +// expected-note@Inputs/def.h:5 {{previous}} @class Def; Def *def; @import decldef; -A *a1; // expected-error{{unknown type name 'A'}} +#ifdef USE_EARLY +A *a1; // expected-error{{declaration of 'A' must be imported from module 'decldef.Def' before it is required}} +#endif B *b1; // expected-error{{must use 'struct' tag to refer to type 'B'}} @import decldef.Decl; @@ -15,7 +18,10 @@ A *a2; struct B *b; void testA(A *a) { - a->ivar = 17; // expected-error{{definition of 'A' must be imported from module 'decldef.Def' before it is required}} + a->ivar = 17; +#ifndef USE_EARLY + // expected-error@-2{{definition of 'A' must be imported from module 'decldef.Def' before it is required}} +#endif } void testB() { diff --git a/test/Modules/decldef.mm b/test/Modules/decldef.mm index 593f53b2c6..c693c7f2ba 100644 --- a/test/Modules/decldef.mm +++ b/test/Modules/decldef.mm @@ -1,7 +1,8 @@ // RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -DUSE_EARLY // RUN: %clang_cc1 -fmodules -fobjc-arc -I %S/Inputs -fmodules-cache-path=%t %s -verify -// expected-note@Inputs/def.h:5 {{previous definition is here}} +// expected-note@Inputs/def.h:5 {{previous}} @class Def; Def *def; @@ -13,15 +14,20 @@ Def2 *def2; @end @import decldef; -A *a1; // expected-error{{unknown type name 'A'}} -B *b1; // expected-error{{unknown type name 'B'}} +#ifdef USE_EARLY +A *a1; // expected-error{{declaration of 'A' must be imported from module 'decldef.Def'}} +B *b1; +#endif @import decldef.Decl; A *a2; B *b; void testA(A *a) { - a->ivar = 17; // expected-error{{definition of 'A' must be imported from module 'decldef.Def' before it is required}} + a->ivar = 17; +#ifndef USE_EARLY + // expected-error@-2{{definition of 'A' must be imported from module 'decldef.Def' before it is required}} +#endif } void testB() { diff --git a/test/Modules/normal-module-map.cpp b/test/Modules/normal-module-map.cpp index 8155318fb3..70fed60b12 100644 --- a/test/Modules/normal-module-map.cpp +++ b/test/Modules/normal-module-map.cpp @@ -24,8 +24,8 @@ int testNestedUmbrellaA() { int testNestedUmbrellaBFail() { return nested_umbrella_b; - // expected-error@-1{{use of undeclared identifier 'nested_umbrella_b'; did you mean 'nested_umbrella_a'?}} - // expected-note@Inputs/normal-module-map/nested_umbrella/a.h:1{{'nested_umbrella_a' declared here}} + // expected-error@-1{{declaration of 'nested_umbrella_b' must be imported from module 'nested_umbrella.b' before it is required}} + // expected-note@Inputs/normal-module-map/nested_umbrella/b.h:1{{previous}} } @import nested_umbrella.b; diff --git a/test/Modules/subframeworks.m b/test/Modules/subframeworks.m index ad70cc2b22..5d70bc0840 100644 --- a/test/Modules/subframeworks.m +++ b/test/Modules/subframeworks.m @@ -5,7 +5,8 @@ @import DependsOnModule; void testSubFramework() { - float *sf1 = sub_framework; // expected-error{{use of undeclared identifier 'sub_framework'}} + float *sf1 = sub_framework; // expected-error{{declaration of 'sub_framework' must be imported from module 'DependsOnModule.SubFramework' before it is required}} + // expected-note@Inputs/DependsOnModule.framework/Frameworks/SubFramework.framework/Headers/SubFramework.h:2 {{previous}} } @import DependsOnModule.SubFramework; diff --git a/test/Modules/submodules.cpp b/test/Modules/submodules.cpp index 1b4f5d886e..c653dddbbb 100644 --- a/test/Modules/submodules.cpp +++ b/test/Modules/submodules.cpp @@ -7,8 +7,9 @@ vector vi; // Note: remove_reference is not visible yet. -remove_reference::type *int_ptr = 0; // expected-error{{unknown type name 'remove_reference'}} \ -// expected-error{{expected unqualified-id}} +remove_reference::type *int_ptr = 0; // expected-error{{declaration of 'remove_reference' must be imported from module 'std.type_traits' before it is required}} +// expected-note@Inputs/submodules/type_traits.h:2{{previous}} +// expected-note@Inputs/submodules/hash_map.h:1{{previous}} @import std.typetraits; // expected-error{{no submodule named 'typetraits' in module 'std'; did you mean 'type_traits'?}} @@ -20,8 +21,7 @@ remove_reference::type *int_ptr2 = 0; @import std; // import everything in 'std' // hash_map still isn't available. -hash_map ints_to_floats; // expected-error{{unknown type name 'hash_map'}} \ -// expected-error{{expected unqualified-id}} +hash_map ints_to_floats; // expected-error{{declaration of 'hash_map' must be imported from module 'std.hash_map' before it is required}} @import std.hash_map;