From 6c35b2dea457cdcf5360d3ff8caf99868f2666c1 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 14 Jul 2016 21:50:09 +0000 Subject: [PATCH] [modules] Don't pass interesting decls to the consumer for a module file that's passed on the command line but never actually used. We consider a (top-level) module to be used if any part of it is imported, either by the current translation unit, or by any part of a top-level module that is itself used. (Put another way, a module is used if an implicit modules build would have loaded its .pcm file.) llvm-svn: 275481 --- clang/include/clang/Serialization/ASTReader.h | 28 ++++++++- clang/lib/Serialization/ASTReader.cpp | 90 +++++++++++++++++++++++++++ clang/lib/Serialization/ASTReaderDecl.cpp | 17 ++--- clang/lib/Serialization/ASTWriterDecl.cpp | 2 +- clang/test/Modules/unused-global-init.cpp | 36 +++++++++++ 5 files changed, 163 insertions(+), 10 deletions(-) create mode 100644 clang/test/Modules/unused-global-init.cpp diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 1b40494..883defc 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -613,7 +613,7 @@ private: /// \brief A mapping from each of the hidden submodules to the deserialized /// declarations in that submodule that could be made visible. HiddenNamesMapType HiddenNamesMap; - + /// \brief A module import, export, or conflict that hasn't yet been resolved. struct UnresolvedModuleRef { @@ -947,6 +947,24 @@ private: /// Objective-C protocols. std::deque InterestingDecls; + /// \brief Contains imported interesting declarations from modules that have + /// not yet themselves been imported, even indirectly. + typedef llvm::SmallVector ModuleInterestingDecls; + + /// Map from top-level modules to their list of interesting declarations. + /// Each map entry may be in one of three states: no entry means we've never + /// seen this module and never transitively imported it. A null pointer means + /// we're not tracking interesting decls: they should be handed straight to + /// the consumer because we've transitively imported this module already. + /// Otherwise, a list of pending interesting decls. + /// + /// As an added twist, declarations that are re-exported are listed against + /// the owning module and the re-exporting one, but the owning module's list + /// is definitive: the decl is there if and only if it has not been handed to + /// the consumer already. + llvm::DenseMap + UnimportedModuleInterestingDecls; + /// \brief The list of redeclaration chains that still need to be /// reconstructed, and the local offset to the corresponding list /// of redeclarations. @@ -1249,6 +1267,9 @@ private: llvm::iterator_range getModuleFileLevelDecls(ModuleFile &Mod); + void addInterestingDecl(Decl *D, + llvm::Optional OwnerOverride = llvm::None); + void PassInterestingDeclsToConsumer(); void PassInterestingDeclToConsumer(Decl *D); @@ -1386,6 +1407,11 @@ public: /// \brief Make the names within this set of hidden names visible. void makeNamesVisible(const HiddenNames &Names, Module *Owner); + /// \brief Mark a module as "used". This implies that any global initializers + /// of declarations contained transitively within it should be run as part of + /// the current compilation. + void markModuleUsed(Module *M); + /// \brief Take the AST callbacks listener. std::unique_ptr takeListener() { return std::move(Listener); diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index b35bd7b..d4bcdac 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2691,6 +2691,8 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { case EAGERLY_DESERIALIZED_DECLS: // FIXME: Skip reading this record if our ASTConsumer doesn't care // about "interesting" decls (for instance, if we're building a module). + // FIXME: Store this somewhere per-module and defer until + // markModuleReferenced is called. for (unsigned I = 0, N = Record.size(); I != N; ++I) EagerlyDeserializedDecls.push_back(getGlobalDeclID(F, Record[I])); break; @@ -3361,6 +3363,9 @@ void ASTReader::makeNamesVisible(const HiddenNames &Names, Module *Owner) { void ASTReader::makeModuleVisible(Module *Mod, Module::NameVisibilityKind NameVisibility, SourceLocation ImportLoc) { + // If we import anything from the module in any way, then it is used. + markModuleUsed(Mod); + llvm::SmallPtrSet Visited; SmallVector Stack; Stack.push_back(Mod); @@ -6727,10 +6732,95 @@ void ASTReader::PassInterestingDeclsToConsumer() { Decl *D = InterestingDecls.front(); InterestingDecls.pop_front(); + // If we have found an interesting ImportDecl, then its imported module + // is considered used. + if (auto *ID = dyn_cast(D)) + markModuleUsed(ID->getImportedModule()); + PassInterestingDeclToConsumer(D); } } +void ASTReader::markModuleUsed(Module *M) { + M = M->getTopLevelModule(); + // Mark that interesting decls in this module should now be passed to the + // consumer, and pass any pending decls. + auto MInterestingDecls = + UnimportedModuleInterestingDecls.insert(std::make_pair(M, nullptr)).first; + if (auto *Decls = MInterestingDecls->second) { + MInterestingDecls->second = nullptr; + for (auto *D : *Decls) { + Module *Owner = D->getImportedOwningModule(); + if (Owner) + Owner = Owner->getTopLevelModule(); + if (Owner != M) { + // Mark that this decl has been handed to the consumer in its original + // module, and stop if it's already been removed from there. + auto OwnerIt = UnimportedModuleInterestingDecls.find(Owner); + if (OwnerIt == UnimportedModuleInterestingDecls.end() || + !OwnerIt->second) + continue; + auto NewEnd = + std::remove(OwnerIt->second->begin(), OwnerIt->second->end(), D); + if (NewEnd == OwnerIt->second->end()) + continue; + OwnerIt->second->erase(NewEnd, OwnerIt->second->end()); + } + InterestingDecls.push_back(D); + } + } +} + +void ASTReader::addInterestingDecl(Decl *D, + llvm::Optional OwnerOverride) { + Module *Owner = D->getImportedOwningModule(); + if (Owner) + Owner = Owner->getTopLevelModule(); + Module *ExportedBy = OwnerOverride ? *OwnerOverride : Owner; + if (ExportedBy) + ExportedBy = ExportedBy->getTopLevelModule(); + + auto It = ExportedBy ? UnimportedModuleInterestingDecls.find(ExportedBy) + : UnimportedModuleInterestingDecls.end(); + if (It == UnimportedModuleInterestingDecls.end()) + It = UnimportedModuleInterestingDecls.insert( + std::make_pair(ExportedBy, new (Context) ModuleInterestingDecls)) + .first; + ModuleInterestingDecls *Interesting = It->second; + + // If this declaration's module has been imported, hand it to the consumer. + if (!ExportedBy || !Interesting) { + if (Owner != ExportedBy) { + // Mark that this decl has been handed to the consumer in its original + // module, and stop if it's already been removed from there. + auto OwnerIt = UnimportedModuleInterestingDecls.find(Owner); + if (OwnerIt == UnimportedModuleInterestingDecls.end() || !OwnerIt->second) + return; + auto NewEnd = + std::remove(OwnerIt->second->begin(), OwnerIt->second->end(), D); + if (NewEnd == OwnerIt->second->end()) + return; + OwnerIt->second->erase(NewEnd, OwnerIt->second->end()); + } + InterestingDecls.push_back(D); + return; + } + assert(Owner && "re-export of unowned decl"); + + // If this is a re-export of another module's decl, check whether the decl + // has already been handed to the consumer. + if (Owner != ExportedBy) { + auto OwnerIt = UnimportedModuleInterestingDecls.find(Owner); + if (OwnerIt != UnimportedModuleInterestingDecls.end() && + (!OwnerIt->second || + std::find(OwnerIt->second->begin(), OwnerIt->second->end(), D) == + OwnerIt->second->end())) + return; + } + + Interesting->push_back(D); +} + void ASTReader::PassInterestingDeclToConsumer(Decl *D) { if (ObjCImplDecl *ImplD = dyn_cast(D)) PassObjCImplDeclToConsumer(ImplD, Consumer); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 5442399..21db402 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3462,6 +3462,13 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) { } assert(Idx == Record.size()); + // If we have deserialized a declaration that has a definition the + // AST consumer might need to know about, queue it. + // We don't pass it to the consumer immediately because we may be in recursive + // loading, and some declarations may still be initializing. + if (isConsumerInterestedIn(D, Reader.hasPendingBody())) + addInterestingDecl(D); + // Load any relevant update records. PendingUpdateRecords.push_back(std::make_pair(ID, D)); @@ -3470,13 +3477,6 @@ Decl *ASTReader::ReadDeclRecord(DeclID ID) { if (Class->isThisDeclarationADefinition()) loadObjCCategories(ID, Class); - // If we have deserialized a declaration that has a definition the - // AST consumer might need to know about, queue it. - // We don't pass it to the consumer immediately because we may be in recursive - // loading, and some declarations may still be initializing. - if (isConsumerInterestedIn(D, Reader.hasPendingBody())) - InterestingDecls.push_back(D); - return D; } @@ -3511,7 +3511,7 @@ void ASTReader::loadDeclUpdateRecords(serialization::DeclID ID, Decl *D) { // we need to hand it off to the consumer. if (!WasInteresting && isConsumerInterestedIn(D, Reader.hasPendingBody())) { - InterestingDecls.push_back(D); + addInterestingDecl(D); WasInteresting = true; } } @@ -3945,6 +3945,7 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, // The declaration is now visible. Exported->Hidden = false; } + Reader.addInterestingDecl(Exported, Owner); break; } diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 0940dd2..6bc7d72 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -2124,7 +2124,7 @@ static bool isRequiredDecl(const Decl *D, ASTContext &Context, // ImportDecl is used by codegen to determine the set of imported modules to // search for inputs for automatic linking; include it if it has a semantic // effect. - if (isa(D) && !WritingModule) + if (isa(D)) return true; return Context.DeclMustBeEmitted(D); diff --git a/clang/test/Modules/unused-global-init.cpp b/clang/test/Modules/unused-global-init.cpp new file mode 100644 index 0000000..bf26b59 --- /dev/null +++ b/clang/test/Modules/unused-global-init.cpp @@ -0,0 +1,36 @@ +// RUN: rm -rf %t +// +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-module %S/Inputs/unused-global-init/module.modulemap -fmodule-name=init -o %t/init.pcm +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-module %S/Inputs/unused-global-init/module.modulemap -fmodule-name=unused -o %t/unused.pcm -fmodule-file=%t/init.pcm +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-module %S/Inputs/unused-global-init/module.modulemap -fmodule-name=used -o %t/used.pcm -fmodule-file=%t/init.pcm +// +// No module file: init.h performs init. +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -DINIT | FileCheck --check-prefix=CHECK-INIT %s +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -DOTHER -DUSED -DUNUSED | FileCheck --check-prefix=CHECK-NO-INIT %s +// +// With module files: if there is a transitive import of any part of the +// module, we run its global initializers (even if the imported piece is not +// visible here). +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -fmodule-file=%t/used.pcm -fmodule-file=%t/unused.pcm -DINIT | FileCheck --check-prefix=CHECK-INIT %s +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -fmodule-file=%t/used.pcm -fmodule-file=%t/unused.pcm -DOTHER | FileCheck --check-prefix=CHECK-INIT %s +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -fmodule-file=%t/used.pcm -fmodule-file=%t/unused.pcm -DUSED | FileCheck --check-prefix=CHECK-INIT %s +// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I %S/Inputs/unused-global-init -triple %itanium_abi_triple -emit-llvm -o - %s -fmodule-file=%t/used.pcm -fmodule-file=%t/unused.pcm -DUNUSED | FileCheck --check-prefix=CHECK-NO-INIT %s + +#ifdef INIT +#include "init.h" +#endif + +#ifdef OTHER +#include "other.h" +#endif + +#ifdef USED +#include "used.h" +#endif + +#ifdef UNUSED +#include "unused.h" +#endif + +// CHECK-INIT: call {{.*}}@_ZN4InitC +// CHECK-NO-INIT-NOT: call {{.*}}@_ZN4InitC -- 2.7.4