Applied in r172295. Please confirm. ...Takumi
2013/1/12 NAKAMURA Takumi <[email protected]>: > Doug, didn't you update unittests? > http://bb.pgr.jp/builders/cmake-clang-x86_64-linux/builds/2487 > > (I could add no-op VoidModuleLoader::makeModuleVisible()) > > ...Takumi > > 2013/1/12 Douglas Gregor <[email protected]>: >> Author: dgregor >> Date: Fri Jan 11 19:29:50 2013 >> New Revision: 172290 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=172290&view=rev >> Log: >> Provide Decl::getOwningModule(), which determines the (sub)module in >> which a particular declaration resides. Use this information to >> customize the "definition of 'blah' must be imported from another >> module" diagnostic with the module the user actually has to >> import. Additionally, recover by importing that module, so we don't >> complain about other names in that module. >> >> Still TODO: coming up with decent Fix-Its for these cases, and expand >> this recovery approach for other name lookup failures. >> >> >> Modified: >> cfe/trunk/include/clang/AST/DeclBase.h >> cfe/trunk/include/clang/AST/ExternalASTSource.h >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/include/clang/Frontend/ASTUnit.h >> cfe/trunk/include/clang/Frontend/CompilerInstance.h >> cfe/trunk/include/clang/Lex/ModuleLoader.h >> cfe/trunk/include/clang/Sema/Lookup.h >> cfe/trunk/include/clang/Sema/Sema.h >> cfe/trunk/include/clang/Serialization/ASTReader.h >> cfe/trunk/lib/AST/DeclBase.cpp >> cfe/trunk/lib/Frontend/CompilerInstance.cpp >> cfe/trunk/lib/Sema/SemaDecl.cpp >> cfe/trunk/lib/Sema/SemaType.cpp >> cfe/trunk/lib/Serialization/ASTReader.cpp >> cfe/trunk/test/Modules/decldef.mm >> >> Modified: cfe/trunk/include/clang/AST/DeclBase.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/DeclBase.h (original) >> +++ cfe/trunk/include/clang/AST/DeclBase.h Fri Jan 11 19:29:50 2013 >> @@ -32,6 +32,7 @@ >> class EnumDecl; >> class FunctionDecl; >> class LinkageSpecDecl; >> +class Module; >> class NamedDecl; >> class NamespaceDecl; >> class ObjCCategoryDecl; >> @@ -593,7 +594,18 @@ >> >> return 0; >> } >> - >> + >> +private: >> + Module *getOwningModuleSlow() const; >> + >> +public: >> + Module *getOwningModule() const { >> + if (!isFromASTFile()) >> + return 0; >> + >> + return getOwningModuleSlow(); >> + } >> + >> unsigned getIdentifierNamespace() const { >> return IdentifierNamespace; >> } >> >> Modified: cfe/trunk/include/clang/AST/ExternalASTSource.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExternalASTSource.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/ExternalASTSource.h (original) >> +++ cfe/trunk/include/clang/AST/ExternalASTSource.h Fri Jan 11 19:29:50 2013 >> @@ -25,6 +25,7 @@ >> class DeclarationName; >> class ExternalSemaSource; // layering violation required for downcasting >> class FieldDecl; >> +class Module; >> class NamedDecl; >> class RecordDecl; >> class Selector; >> @@ -131,9 +132,12 @@ >> /// \brief Ensures that the table of all visible declarations inside this >> /// context is up to date. >> /// >> - /// The default implementation of this functino is a no-op. >> + /// The default implementation of this function is a no-op. >> virtual void completeVisibleDeclsMap(const DeclContext *DC); >> >> + /// \brief Retrieve the module that corresponds to the given module ID. >> + virtual Module *getModule(unsigned ID) { return 0; } >> + >> /// \brief Finds all declarations lexically contained within the given >> /// DeclContext, after applying an optional filter predicate. >> /// >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 11 19:29:50 >> 2013 >> @@ -6082,7 +6082,7 @@ >> "local %select{struct|interface|union|class|enum}0 cannot be declared " >> "__module_private__">; >> def err_module_private_definition : Error< >> - "definition of %0 must be imported before it is required">; >> + "definition of %0 must be imported from module '%1' before it is >> required">; >> } >> >> let CategoryName = "Documentation Issue" in { >> >> Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original) >> +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Fri Jan 11 19:29:50 2013 >> @@ -837,6 +837,10 @@ >> // ASTUnit doesn't know how to load modules (not that this matters). >> return ModuleLoadResult(); >> } >> + >> + virtual void makeModuleVisible(Module *Mod, >> + Module::NameVisibilityKind Visibility) { } >> + >> }; >> >> } // namespace clang >> >> Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original) >> +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Fri Jan 11 19:29:50 >> 2013 >> @@ -649,6 +649,10 @@ >> ModuleIdPath Path, >> Module::NameVisibilityKind Visibility, >> bool IsInclusionDirective); >> + >> + virtual void makeModuleVisible(Module *Mod, >> + Module::NameVisibilityKind Visibility); >> + >> }; >> >> } // end namespace clang >> >> Modified: cfe/trunk/include/clang/Lex/ModuleLoader.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleLoader.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Lex/ModuleLoader.h (original) >> +++ cfe/trunk/include/clang/Lex/ModuleLoader.h Fri Jan 11 19:29:50 2013 >> @@ -80,6 +80,10 @@ >> ModuleIdPath Path, >> Module::NameVisibilityKind Visibility, >> bool IsInclusionDirective) = 0; >> + >> + /// \brief Make the given module visible. >> + virtual void makeModuleVisible(Module *Mod, >> + Module::NameVisibilityKind Visibility) = 0; >> }; >> >> } >> >> Modified: cfe/trunk/include/clang/Sema/Lookup.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/Lookup.h (original) >> +++ cfe/trunk/include/clang/Sema/Lookup.h Fri Jan 11 19:29:50 2013 >> @@ -138,7 +138,8 @@ >> IDNS(0), >> Redecl(Redecl != Sema::NotForRedeclaration), >> HideTags(true), >> - Diagnose(Redecl == Sema::NotForRedeclaration) >> + Diagnose(Redecl == Sema::NotForRedeclaration), >> + AllowHidden(Redecl == Sema::ForRedeclaration) >> { >> configure(); >> } >> @@ -158,7 +159,8 @@ >> IDNS(0), >> Redecl(Redecl != Sema::NotForRedeclaration), >> HideTags(true), >> - Diagnose(Redecl == Sema::NotForRedeclaration) >> + Diagnose(Redecl == Sema::NotForRedeclaration), >> + AllowHidden(Redecl == Sema::ForRedeclaration) >> { >> configure(); >> } >> @@ -176,7 +178,8 @@ >> IDNS(Other.IDNS), >> Redecl(Other.Redecl), >> HideTags(Other.HideTags), >> - Diagnose(false) >> + Diagnose(false), >> + AllowHidden(Other.AllowHidden) >> {} >> >> ~LookupResult() { >> @@ -214,10 +217,16 @@ >> return Redecl; >> } >> >> + /// \brief Specify whether hidden declarations are visible, e.g., >> + /// for recovery reasons. >> + void setAllowHidden(bool AH) { >> + AllowHidden = AH; >> + } >> + >> /// \brief Determine whether this lookup is permitted to see hidden >> /// declarations, such as those in modules that have not yet been >> imported. >> bool isHiddenDeclarationVisible() const { >> - return Redecl || LookupKind == Sema::LookupTagName; >> + return AllowHidden || LookupKind == Sema::LookupTagName; >> } >> >> /// Sets whether tag declarations should be hidden by non-tag >> @@ -483,6 +492,7 @@ >> /// \brief Change this lookup's redeclaration kind. >> void setRedeclarationKind(Sema::RedeclarationKind RK) { >> Redecl = RK; >> + AllowHidden = (RK == Sema::ForRedeclaration); >> configure(); >> } >> >> @@ -644,6 +654,9 @@ >> bool HideTags; >> >> bool Diagnose; >> + >> + /// \brief True if we should allow hidden declarations to be 'visible'. >> + bool AllowHidden; >> }; >> >> /// \brief Consumes visible declarations found when searching for >> >> Modified: cfe/trunk/include/clang/Sema/Sema.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/Sema.h (original) >> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Jan 11 19:29:50 2013 >> @@ -275,12 +275,6 @@ >> /// This is only necessary for issuing pretty diagnostics. >> ExtVectorDeclsType ExtVectorDecls; >> >> - /// \brief The set of types for which we have already complained about the >> - /// definitions being hidden. >> - /// >> - /// This set is used to suppress redundant diagnostics. >> - llvm::SmallPtrSet<NamedDecl *, 4> HiddenDefinitions; >> - >> /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes. >> OwningPtr<CXXFieldCollector> FieldCollector; >> >> @@ -1456,6 +1450,14 @@ >> DeclResult ActOnModuleImport(SourceLocation AtLoc, SourceLocation >> ImportLoc, >> ModuleIdPath Path); >> >> + /// \brief Create an implicit import of the given module at the given >> + /// source location. >> + /// >> + /// This routine is typically used for error recovery, when the entity >> found >> + /// by name lookup is actually hidden within a module that we know about >> but >> + /// the user has forgotten to import. >> + void createImplicitModuleImport(SourceLocation Loc, Module *Mod); >> + >> /// \brief Retrieve a suitable printing policy. >> PrintingPolicy getPrintingPolicy() const { >> return getPrintingPolicy(Context, PP); >> >> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) >> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jan 11 19:29:50 >> 2013 >> @@ -1558,7 +1558,12 @@ >> /// \brief Retrieve the submodule that corresponds to a global submodule >> ID. >> /// >> Module *getSubmodule(serialization::SubmoduleID GlobalID); >> - >> + >> + /// \brief Retrieve the module that corresponds to the given module ID. >> + /// >> + /// Note: overrides method in ExternalASTSource >> + virtual Module *getModule(unsigned ID); >> + >> /// \brief Retrieve a selector from the given module with its local ID >> /// number. >> Selector getLocalSelector(ModuleFile &M, unsigned LocalID); >> >> Modified: cfe/trunk/lib/AST/DeclBase.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/lib/AST/DeclBase.cpp (original) >> +++ cfe/trunk/lib/AST/DeclBase.cpp Fri Jan 11 19:29:50 2013 >> @@ -59,6 +59,11 @@ >> return Result; >> } >> >> +Module *Decl::getOwningModuleSlow() const { >> + assert(isFromASTFile() && "Not from AST file?"); >> + return >> getASTContext().getExternalSource()->getModule(getOwningModuleID()); >> +} >> + >> const char *Decl::getDeclKindName() const { >> switch (DeclKind) { >> default: llvm_unreachable("Declaration not in DeclNodes.inc!"); >> >> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) >> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Jan 11 19:29:50 2013 >> @@ -1201,3 +1201,9 @@ >> LastModuleImportResult = ModuleLoadResult(Module, false); >> return LastModuleImportResult; >> } >> + >> +void CompilerInstance::makeModuleVisible(Module *Mod, >> + Module::NameVisibilityKind >> Visibility){ >> + ModuleManager->makeModuleVisible(Mod, Visibility); >> +} >> + >> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jan 11 19:29:50 2013 >> @@ -11173,6 +11173,18 @@ >> return Import; >> } >> >> +void Sema::createImplicitModuleImport(SourceLocation Loc, Module *Mod) { >> + // Create the implicit import declaration. >> + TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl(); >> + ImportDecl *ImportD = ImportDecl::CreateImplicit(getASTContext(), TU, >> + Loc, Mod, Loc); >> + TU->addDecl(ImportD); >> + Consumer.HandleImplicitImportDecl(ImportD); >> + >> + // Make the module visible. >> + PP.getModuleLoader().makeModuleVisible(Mod, Module::AllVisible); >> +} >> + >> void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name, >> IdentifierInfo* AliasName, >> SourceLocation PragmaLoc, >> >> Modified: cfe/trunk/lib/Sema/SemaType.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaType.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaType.cpp Fri Jan 11 19:29:50 2013 >> @@ -4392,9 +4392,14 @@ >> // repeating the diagnostic. >> // FIXME: Add a Fix-It that imports the corresponding module or >> includes >> // the header. >> - if (isSFINAEContext() || HiddenDefinitions.insert(Def)) { >> - Diag(Loc, diag::err_module_private_definition) << T; >> - Diag(Def->getLocation(), diag::note_previous_definition); >> + Module *Owner = Def->getOwningModule(); >> + Diag(Loc, diag::err_module_private_definition) >> + << T << Owner->getFullModuleName(); >> + Diag(Def->getLocation(), diag::note_previous_definition); >> + >> + if (!isSFINAEContext()) { >> + // Recover by implicitly importing this module. >> + createImplicitModuleImport(Loc, Owner); >> } >> } >> >> >> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jan 11 19:29:50 2013 >> @@ -6200,7 +6200,11 @@ >> >> return SubmodulesLoaded[GlobalID - NUM_PREDEF_SUBMODULE_IDS]; >> } >> - >> + >> +Module *ASTReader::getModule(unsigned ID) { >> + return getSubmodule(ID); >> +} >> + >> Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) { >> return DecodeSelector(getGlobalSelectorID(M, LocalID)); >> } >> >> Modified: cfe/trunk/test/Modules/decldef.mm >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/decldef.mm?rev=172290&r1=172289&r2=172290&view=diff >> ============================================================================== >> --- cfe/trunk/test/Modules/decldef.mm (original) >> +++ cfe/trunk/test/Modules/decldef.mm Fri Jan 11 19:29:50 2013 >> @@ -2,13 +2,7 @@ >> // RUN: %clang_cc1 -fmodules -I %S/Inputs -fmodule-cache-path %t %s -verify >> >> >> -// in other file: expected-note{{previous definition is here}} >> - >> - >> - >> - >> - >> -// in other file: expected-note{{previous definition is here}} >> +// In other file: expected-note {{previous definition is here}} >> >> @import decldef; >> A *a1; // expected-error{{unknown type name 'A'}} >> @@ -19,10 +13,9 @@ >> B *b; >> >> void testA(A *a) { >> - a->ivar = 17; // expected-error{{definition of 'A' must be imported >> before it is required}} >> + a->ivar = 17; // expected-error{{definition of 'A' must be imported from >> module 'decldef.Def' before it is required}} >> } >> >> void testB() { >> - B b; // expected-error{{definition of 'B' must be imported before it is >> required}} >> - B b2; // Note: the reundant error was silenced. >> + B b; // Note: redundant error silenced >> } >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
