diff --git a/clang/include/clang/Lex/ExternalPreprocessorSource.h b/clang/include/clang/Lex/ExternalPreprocessorSource.h --- a/clang/include/clang/Lex/ExternalPreprocessorSource.h +++ b/clang/include/clang/Lex/ExternalPreprocessorSource.h @@ -13,8 +13,11 @@ #ifndef LLVM_CLANG_LEX_EXTERNALPREPROCESSORSOURCE_H #define LLVM_CLANG_LEX_EXTERNALPREPROCESSORSOURCE_H +#include "llvm/ADT/DenseSet.h" + namespace clang { +class FileEntry; class IdentifierInfo; class Module; @@ -40,6 +43,10 @@ /// Map a module ID to a module. virtual Module *getModule(unsigned ModuleID) = 0; + + /// Return the files directly included in the given (sub)module. + virtual const llvm::DenseSet * + getIncludedFiles(Module *M) = 0; }; } diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -718,6 +718,7 @@ using MacroMap = llvm::DenseMap; struct SubmoduleState; + struct SubmoduleIncludeState; /// Information about a submodule that we're currently building. struct BuildingSubmoduleInfo { @@ -736,12 +737,17 @@ /// The number of pending module macro names when we started building this. unsigned OuterPendingModuleMacroNames; + /// The previous SubmoduleIncludeState. + SubmoduleIncludeState *OuterSubmoduleIncludeState; + BuildingSubmoduleInfo(Module *M, SourceLocation ImportLoc, bool IsPragma, SubmoduleState *OuterSubmoduleState, - unsigned OuterPendingModuleMacroNames) + unsigned OuterPendingModuleMacroNames, + SubmoduleIncludeState *OuterSubmoduleIncludeState) : M(M), ImportLoc(ImportLoc), IsPragma(IsPragma), OuterSubmoduleState(OuterSubmoduleState), - OuterPendingModuleMacroNames(OuterPendingModuleMacroNames) {} + OuterPendingModuleMacroNames(OuterPendingModuleMacroNames), + OuterSubmoduleIncludeState(OuterSubmoduleIncludeState) {} }; SmallVector BuildingSubmoduleStack; @@ -765,6 +771,23 @@ /// in a submodule. SubmoduleState *CurSubmoduleState; + /// Information about a (sub)module's preprocessor include state. + struct SubmoduleIncludeState { + /// The files that have been included. + llvm::DenseSet IncludedFiles; + + /// The set of modules that are visible within the submodule. + VisibleModuleSet VisibleModules; + }; + /// The include state for each (sub)module. + std::map SubmoduleIncludeStates; + + /// The include state outside of any (sub)module. + SubmoduleIncludeState NullSubmoduleIncludeState; + + /// The current include state. + SubmoduleIncludeState *CurSubmoduleIncludeState; + /// The files that have been included. llvm::DenseSet IncludedFiles; @@ -923,6 +946,10 @@ void updateOutOfDateIdentifier(IdentifierInfo &II) const; + /// Get the external include information for the given (sub)module. + const llvm::DenseSet * + getExternalSubmoduleIncludes(Module *M) const; + public: Preprocessor(std::shared_ptr PPOpts, DiagnosticsEngine &diags, LangOptions &opts, SourceManager &SM, @@ -1230,22 +1257,27 @@ /// Increment the count for the number of times the specified FileEntry has /// been entered. Returns true if this is the first time it file was included. bool incrementIncludeCount(const FileEntry *File) { + CurSubmoduleIncludeState->IncludedFiles.insert(File); return IncludedFiles.insert(File).second; } + /// Increment the include counter for a file transitively included file. + void incrementTransitiveIncludeCount(const FileEntry *File); + /// Return true if this header has already been included. bool alreadyIncluded(const FileEntry *File) const { return IncludedFiles.count(File); } - /// Get the included files. - llvm::DenseSet &getIncludedFiles() { - return IncludedFiles; - } - const llvm::DenseSet &getIncludedFiles() const { - return IncludedFiles; + /// Get the include information outside of any (sub)module. + const llvm::DenseSet &getNullSubmoduleIncludes() const { + return NullSubmoduleIncludeState.IncludedFiles; } + /// Get the include information for the given local (sub)module. + const llvm::DenseSet * + getLocalSubmoduleIncludes(Module *M) const; + /// Return the name of the macro defined before \p Loc that has /// spelling \p Tokens. If there are multiple macros with same spelling, /// return the last one defined. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -828,6 +828,9 @@ /// Specifies the name of the module that will eventually /// re-export the entities in this module. SUBMODULE_EXPORT_AS = 17, + + /// Specifies files included in this module. + SUBMODULE_INCLUDED_FILES = 18, }; /// Record types used within a comments block. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -927,6 +927,10 @@ /// A list of modules that were imported by precompiled headers or /// any other non-module AST file. SmallVector ImportedModules; + + /// Mapping between a (sub)module and deserialized included files set. + llvm::DenseMap> + SubmoduleIncludedFiles; //@} /// The system include root to be used when loading the @@ -1331,7 +1335,8 @@ llvm::Error ReadSourceManagerBlock(ModuleFile &F); llvm::BitstreamCursor &SLocCursorForID(int ID); SourceLocation getImportLocation(ModuleFile *F); - void readIncludedFiles(ModuleFile &F, StringRef Blob, Preprocessor &PP); + llvm::DenseSet readIncludedFiles(ModuleFile &F, + StringRef Blob); ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const ModuleFile *ImportedBy, unsigned ClientLoadCapabilities); @@ -2100,6 +2105,9 @@ /// Module *getSubmodule(serialization::SubmoduleID GlobalID); + /// Return the files directly included in the given (sub)module. + const llvm::DenseSet *getIncludedFiles(Module *M) override; + /// Retrieve the module that corresponds to the given module ID. /// /// Note: overrides method in ExternalASTSource diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -478,7 +478,8 @@ bool Modules); void WriteSourceManagerBlock(SourceManager &SourceMgr, const Preprocessor &PP); - void writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP); + void writeIncludedFiles(raw_ostream &Out, + const llvm::DenseSet &Files); void WritePreprocessor(const Preprocessor &PP, bool IsModule); void WriteHeaderSearch(const HeaderSearch &HS); void WritePreprocessorDetail(PreprocessingRecord &PPRec, diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -394,6 +394,10 @@ /// Remapping table for submodule IDs in this module. ContinuousRangeMap SubmoduleRemap; + /// Mapping between (sub)module names and the serialized version of included + /// files set. + std::map SubmoduleIncludedFiles; + // === Selectors === /// The number of selectors new to this file. diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -650,7 +650,7 @@ return; ImportLocs[ID] = Loc; - Vis(M); + Vis(V.M); // Make any exported modules visible. SmallVector Exports; diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp --- a/clang/lib/Lex/PPLexerChange.cpp +++ b/clang/lib/Lex/PPLexerChange.cpp @@ -690,9 +690,12 @@ bool ForPragma) { if (!getLangOpts().ModulesLocalVisibility) { // Just track that we entered this submodule. - BuildingSubmoduleStack.push_back( - BuildingSubmoduleInfo(M, ImportLoc, ForPragma, CurSubmoduleState, - PendingModuleMacroNames.size())); + BuildingSubmoduleStack.push_back(BuildingSubmoduleInfo( + M, ImportLoc, ForPragma, CurSubmoduleState, + PendingModuleMacroNames.size(), CurSubmoduleIncludeState)); + + CurSubmoduleIncludeState = &SubmoduleIncludeStates[M]; + if (Callbacks) Callbacks->EnteredSubmodule(M, ImportLoc, ForPragma); return; @@ -735,15 +738,16 @@ } // Track that we entered this module. - BuildingSubmoduleStack.push_back( - BuildingSubmoduleInfo(M, ImportLoc, ForPragma, CurSubmoduleState, - PendingModuleMacroNames.size())); + BuildingSubmoduleStack.push_back(BuildingSubmoduleInfo( + M, ImportLoc, ForPragma, CurSubmoduleState, + PendingModuleMacroNames.size(), CurSubmoduleIncludeState)); if (Callbacks) Callbacks->EnteredSubmodule(M, ImportLoc, ForPragma); // Switch to this submodule as the current submodule. CurSubmoduleState = &State; + CurSubmoduleIncludeState = &SubmoduleIncludeStates[M]; // This module is visible to itself. if (FirstTime) @@ -775,6 +779,8 @@ Module *LeavingMod = Info.M; SourceLocation ImportLoc = Info.ImportLoc; + CurSubmoduleIncludeState = Info.OuterSubmoduleIncludeState; + if (!needModuleMacros() || (!getLangOpts().ModulesLocalVisibility && LeavingMod->getTopLevelModuleName() != getLangOpts().CurrentModule)) { diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp --- a/clang/lib/Lex/Preprocessor.cpp +++ b/clang/lib/Lex/Preprocessor.cpp @@ -91,7 +91,8 @@ // deferred to Preprocessor::Initialize(). Identifiers(IILookup), PragmaHandlers(new PragmaNamespace(StringRef())), TUKind(TUKind), SkipMainFilePreamble(0, true), - CurSubmoduleState(&NullSubmoduleState) { + CurSubmoduleState(&NullSubmoduleState), + CurSubmoduleIncludeState(&NullSubmoduleIncludeState) { OwnsHeaderSearch = OwnsHeaders; // Default to discarding comments. @@ -554,6 +555,9 @@ } } + if (Module *M = getCurrentModule()) + CurSubmoduleIncludeState = &SubmoduleIncludeStates[M]; + // Preprocess Predefines to populate the initial preprocessor state. std::unique_ptr SB = llvm::MemoryBuffer::getMemBufferCopy(Predefines, ""); @@ -1317,6 +1321,18 @@ << Message; }); + CurSubmoduleIncludeState->VisibleModules.setVisible( + M, Loc, + [&](Module *M) { + const auto *Includes = getLocalSubmoduleIncludes(M); + if (!Includes) + Includes = getExternalSubmoduleIncludes(M); + if (Includes) + for (const auto &E : *Includes) + incrementTransitiveIncludeCount(E); + }, + [](ArrayRef, Module *, StringRef) {}); + // Add this module to the imports list of the currently-built submodule. if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M) BuildingSubmoduleStack.back().M->Imports.insert(M); @@ -1468,3 +1484,25 @@ Record = new PreprocessingRecord(getSourceManager()); addPPCallbacks(std::unique_ptr(Record)); } + +void Preprocessor::incrementTransitiveIncludeCount(const FileEntry *File) { + // Compared to `incrementIncludeCount`, we're not incrementing the count in + // CurSubmoduleIncludeState`. This is to ensure it always only contains + // **locally** included files and not transitively included ones. + IncludedFiles.insert(File); +} + +const llvm::DenseSet * +Preprocessor::getLocalSubmoduleIncludes(Module *M) const { + auto It = SubmoduleIncludeStates.find(M); + if (It != SubmoduleIncludeStates.end()) + return &It->second.IncludedFiles; + return nullptr; +} + +const llvm::DenseSet * +Preprocessor::getExternalSubmoduleIncludes(Module *M) const { + if (ExternalSource) + return ExternalSource->getIncludedFiles(M); + return nullptr; +} diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2959,10 +2959,12 @@ } } -void ASTReader::readIncludedFiles(ModuleFile &F, StringRef Blob, - Preprocessor &PP) { +llvm::DenseSet ASTReader::readIncludedFiles(ModuleFile &F, + StringRef Blob) { using namespace llvm::support; + llvm::DenseSet Result; + const unsigned char *D = (const unsigned char *)Blob.data(); unsigned FileCount = endian::readNext(D); @@ -2971,8 +2973,10 @@ if (*D & (1 << Bit)) { auto InputFileInfo = readInputFileInfo(F, I); if (auto File = PP.getFileManager().getFile(InputFileInfo.Filename)) - PP.getIncludedFiles().insert(*File); + Result.insert(*File); } + + return Result; } llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, @@ -3714,7 +3718,10 @@ } case PP_INCLUDED_FILES: - readIncludedFiles(F, Blob, PP); + if (F.isModule()) + break; + for (const auto &File : readIncludedFiles(F, Blob)) + PP.incrementTransitiveIncludeCount(File); break; case LATE_PARSED_TEMPLATE: @@ -5727,6 +5734,10 @@ CurrentModule->ExportAsModule = Blob.str(); ModMap.addLinkAsDependency(CurrentModule); break; + + case SUBMODULE_INCLUDED_FILES: + F.SubmoduleIncludedFiles.insert( + {CurrentModule->getFullModuleName(), Blob}); } } } @@ -8612,6 +8623,27 @@ return LocalID + I->second; } +const llvm::DenseSet * +ASTReader::getIncludedFiles(Module *M) { + ModuleFile *F = getModuleManager().lookup(M->getASTFile()); + if (!F) + return nullptr; + + auto ResultIt = SubmoduleIncludedFiles.insert( + {M, llvm::DenseSet{}}); + auto &Result = ResultIt.first->second; + if (!ResultIt.second) + return &Result; + + auto It = F->SubmoduleIncludedFiles.find(M->getFullModuleName()); + if (It == F->SubmoduleIncludedFiles.end()) + return nullptr; + auto &Record = It->second; + + Result = readIncludedFiles(*F, Record); + return &Result; +} + Module *ASTReader::getSubmodule(SubmoduleID GlobalID) { if (GlobalID < NUM_PREDEF_SUBMODULE_IDS) { assert(GlobalID == 0 && "Unhandled global submodule ID"); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2170,11 +2170,12 @@ return false; } -void ASTWriter::writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP) { +void ASTWriter::writeIncludedFiles( + raw_ostream &Out, const llvm::DenseSet &Files) { using namespace llvm::support; std::vector IncludedInputFiles; - for (const auto &File : PP.getIncludedFiles()) { + for (const auto &File : Files) { auto InputFileIt = InputFileIDs.find(File); if (InputFileIt == InputFileIDs.end()) continue; @@ -2406,7 +2407,7 @@ SmallString<2048> Buffer; raw_svector_ostream Out(Buffer); - writeIncludedFiles(Out, PP); + writeIncludedFiles(Out, PP.getNullSubmoduleIncludes()); RecordData::value_type Record[] = {PP_INCLUDED_FILES}; Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(), Buffer.size()); @@ -2666,6 +2667,11 @@ Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Macro name unsigned ExportAsAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); + Abbrev = std::make_shared(); + Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_INCLUDED_FILES)); + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); + unsigned IncludedFilesAbbrev = Stream.EmitAbbrev(std::move(Abbrev)); + // Write the submodule metadata block. RecordData::value_type Record[] = { getNumberOfModules(WritingModule), @@ -2807,6 +2813,15 @@ Stream.EmitRecordWithBlob(ExportAsAbbrev, Record, Mod->ExportAsModule); } + if (const auto *Includes = PP->getLocalSubmoduleIncludes(Mod)) { + SmallString<2048> Buffer; + raw_svector_ostream Out(Buffer); + writeIncludedFiles(Out, *Includes); + RecordData::value_type Record[] = {SUBMODULE_INCLUDED_FILES}; + Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(), + Buffer.size()); + } + // Queue up the submodules of this module. for (auto *M : Mod->submodules()) Q.push(M); diff --git a/clang/test/Modules/import-submodule-visibility.c b/clang/test/Modules/import-submodule-visibility.c new file mode 100644 --- /dev/null +++ b/clang/test/Modules/import-submodule-visibility.c @@ -0,0 +1,99 @@ +// This test checks that imports of headers that appeared in a different submodule than +// what is imported by the current TU don't affect the compilation. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- A.framework/Headers/A.h +#include "Textual.h" +//--- A.framework/Modules/module.modulemap +framework module A { header "A.h" } + +//--- B.framework/Headers/B1.h +#include "Textual.h" +//--- B.framework/Headers/B2.h +//--- B.framework/Modules/module.modulemap +framework module B { + module B1 { header "B1.h" } + module B2 { header "B2.h" } +} + +//--- C/C.h +#include "Textual.h" +//--- C/module.modulemap +module C { header "C.h" } + +//--- D/D1.h +#include "Textual.h" +//--- D/D2.h +//--- D/module.modulemap +module D { + module D1 { header "D1.h" } + module D2 { header "D2.h" } +} + +//--- E/E1.h +#include "E2.h" +//--- E/E2.h +#include "Textual.h" +//--- E/module.modulemap +module E { + module E1 { header "E1.h" } + module E2 { header "E2.h" } +} + +//--- Textual.h +#define MACRO_TEXTUAL 1 + +//--- test.c + +#ifdef A +// +#endif + +#ifdef B +#import +#endif + +#ifdef C +// +#endif + +#ifdef D +#import "D/D2.h" +#endif + +#ifdef E +#import "E/E1.h" +#endif + +#import "Textual.h" + +static int x = MACRO_TEXTUAL; + +// Specifying the PCM file on the command line (without actually importing "A") should not +// prevent "Textual.h" to be included in the TU. +// +// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/A.framework/Modules/module.modulemap -fmodule-name=A -o %t/A.pcm +// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DA -fmodule-file=%t/A.pcm + +// Specifying the PCM file on the command line and importing "B2" in the source does not +// prevent "Textual.h" to be included in the TU. +// +// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/B.framework/Modules/module.modulemap -fmodule-name=B -o %t/B.pcm +// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DB -iframework %t -fmodule-file=%t/B.pcm + +// Module-only version of the test with framework A. +// +// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/C/module.modulemap -fmodule-name=C -o %t/C.pcm +// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DC -fmodule-file=%t/C.pcm + +// Module-only version of the test with framework B. +// +// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/D/module.modulemap -fmodule-name=D -o %t/D.pcm +// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DD -fmodule-file=%t/D.pcm + +// Transitively imported, but not exported. +// +// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/E/module.modulemap -fmodule-name=E -o %t/E.pcm +// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test.c -DE -fmodule-file=%t/E.pcm