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
