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,6 +13,8 @@ #ifndef LLVM_CLANG_LEX_EXTERNALPREPROCESSORSOURCE_H #define LLVM_CLANG_LEX_EXTERNALPREPROCESSORSOURCE_H +#include "clang/Lex/Preprocessor.h" + namespace clang { class IdentifierInfo; @@ -40,6 +42,9 @@ /// Map a module ID to a module. virtual Module *getModule(unsigned ModuleID) = 0; + + /// Return the set of files directly included in the given (sub)module. + virtual const Preprocessor::IncludedFilesSet *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 @@ -767,7 +767,12 @@ /// in a submodule. SubmoduleState *CurSubmoduleState; - /// The files that have been included. + /// The set of files that have been included in each submodule. + /// Files included outside of any module (e.g. in PCH) have nullptr key. + llvm::DenseMap IncludedFilesPerSubmodule; + + /// The global set of files that have been included. + // TODO: Move this into SubmoduleState. IncludedFilesSet IncludedFiles; /// The set of known macros exported from modules. @@ -925,6 +930,9 @@ void updateOutOfDateIdentifier(IdentifierInfo &II) const; + /// Get the external include information for the given (sub)module. + const IncludedFilesSet *getExternalSubmoduleIncludes(Module *M) const; + public: Preprocessor(std::shared_ptr PPOpts, DiagnosticsEngine &diags, LangOptions &opts, SourceManager &SM, @@ -1231,19 +1239,25 @@ /// Mark the file as included. /// Returns true if this is the first time the file was included. - bool markIncluded(const FileEntry *File) { - HeaderInfo.getFileInfo(File); - return IncludedFiles.insert(File).second; - } + bool markIncluded(const FileEntry *File); + + /// Mark the file as transitively included. + void markTransitivelyIncluded(const FileEntry *File); /// Return true if this header has already been included. - bool alreadyIncluded(const FileEntry *File) const { - return IncludedFiles.count(File); + bool alreadyIncluded(const FileEntry *File) const; + + /// Get the set of files included outside of any (sub)module. + const IncludedFilesSet *getNullSubmoduleIncludes() const { + auto It = IncludedFilesPerSubmodule.find(nullptr); + return It == IncludedFilesPerSubmodule.end() ? nullptr : &It->second; } - /// Get the set of included files. - IncludedFilesSet &getIncludedFiles() { return IncludedFiles; } - const IncludedFilesSet &getIncludedFiles() const { return IncludedFiles; } + /// Get the set of files included in the given (sub)module. + const IncludedFilesSet *getLocalSubmoduleIncludes(Module *M) const { + auto It = IncludedFilesPerSubmodule.find(M); + return It == IncludedFilesPerSubmodule.end() ? nullptr : &It->second; + } /// Return the name of the macro defined before \p Loc that has /// spelling \p Tokens. If there are multiple macros with same spelling, 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 @@ -925,6 +925,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 set of included files. + llvm::DenseMap + SubmoduleIncludedFiles; //@} /// The system include root to be used when loading the @@ -1329,7 +1333,8 @@ llvm::Error ReadSourceManagerBlock(ModuleFile &F); llvm::BitstreamCursor &SLocCursorForID(int ID); SourceLocation getImportLocation(ModuleFile *F); - void readIncludedFiles(ModuleFile &F, StringRef Blob, Preprocessor &PP); + Preprocessor::IncludedFilesSet readIncludedFiles(ModuleFile &F, + StringRef Blob); ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, const ModuleFile *ImportedBy, unsigned ClientLoadCapabilities); @@ -2098,6 +2103,9 @@ /// Module *getSubmodule(serialization::SubmoduleID GlobalID); + /// Return the set of files directly included in the given (sub)module. + const Preprocessor::IncludedFilesSet *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 @@ -19,6 +19,7 @@ #include "clang/AST/Type.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Sema/Sema.h" #include "clang/Sema/SemaConsumer.h" #include "clang/Serialization/ASTBitCodes.h" @@ -465,7 +466,8 @@ std::set &AffectingModuleMaps); void WriteSourceManagerBlock(SourceManager &SourceMgr, const Preprocessor &PP); - void writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP); + void writeIncludedFiles(raw_ostream &Out, + const Preprocessor::IncludedFilesSet &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 @@ -111,7 +111,8 @@ class ModuleFile { public: ModuleFile(ModuleKind Kind, unsigned Generation) - : Kind(Kind), Generation(Generation) {} + : Kind(Kind), Generation(Generation), + SubmoduleIncludedFiles(SubmoduleIncludedFilesAlloc) {} ~ModuleFile(); // === General information === @@ -394,6 +395,12 @@ /// Remapping table for submodule IDs in this module. ContinuousRangeMap SubmoduleRemap; + /// Allocator for the serialized set of included files. + llvm::BumpPtrAllocator SubmoduleIncludedFilesAlloc; + /// Mapping between (sub)module names and the serialized set of included + /// files. Initialized by the allocator above. + llvm::StringMap 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 @@ -688,6 +688,10 @@ void Preprocessor::EnterSubmodule(Module *M, SourceLocation ImportLoc, bool ForPragma) { + // Ensure that even if this submodule doesn't include anything, it's present + // in the map. + IncludedFilesPerSubmodule[M]; + if (!getLangOpts().ModulesLocalVisibility) { // Just track that we entered this submodule. BuildingSubmoduleStack.push_back( 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 @@ -1304,7 +1304,16 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc) { CurSubmoduleState->VisibleModules.setVisible( - M, Loc, [](Module *) {}, + M, Loc, + [&](Module *M) { + const Preprocessor::IncludedFilesSet *Includes = + getLocalSubmoduleIncludes(M); + if (!Includes) + Includes = getExternalSubmoduleIncludes(M); + if (Includes) + for (const FileEntry *E : *Includes) + markTransitivelyIncluded(E); + }, [&](ArrayRef Path, Module *Conflict, StringRef Message) { // FIXME: Include the path in the diagnostic. // FIXME: Include the import location for the conflicting module. @@ -1465,3 +1474,28 @@ Record = new PreprocessingRecord(getSourceManager()); addPPCallbacks(std::unique_ptr(Record)); } + +bool Preprocessor::markIncluded(const FileEntry *File) { + HeaderInfo.getFileInfo(File); + + Module *CurrentSubmodule = getCurrentModule(); + if (!BuildingSubmoduleStack.empty()) + CurrentSubmodule = BuildingSubmoduleStack.back().M; + IncludedFilesPerSubmodule[CurrentSubmodule].insert(File); + + return IncludedFiles.insert(File).second; +} + +void Preprocessor::markTransitivelyIncluded(const FileEntry *File) { + HeaderInfo.getFileInfo(File); + IncludedFiles.insert(File); +} + +bool Preprocessor::alreadyIncluded(const FileEntry *File) const { + return IncludedFiles.count(File); +} + +const Preprocessor::IncludedFilesSet * +Preprocessor::getExternalSubmoduleIncludes(Module *M) const { + return ExternalSource ? ExternalSource->getIncludedFiles(M) : 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 @@ -2958,10 +2958,12 @@ } } -void ASTReader::readIncludedFiles(ModuleFile &F, StringRef Blob, - Preprocessor &PP) { +Preprocessor::IncludedFilesSet ASTReader::readIncludedFiles(ModuleFile &F, + StringRef Blob) { using namespace llvm::support; + Preprocessor::IncludedFilesSet Result; + const unsigned char *D = (const unsigned char *)Blob.data(); unsigned FileCount = endian::readNext(D); @@ -2970,8 +2972,10 @@ InputFileInfo IFI = readInputFileInfo(F, ID); if (llvm::ErrorOr File = PP.getFileManager().getFile(IFI.Filename)) - PP.getIncludedFiles().insert(*File); + Result.insert(*File); } + + return Result; } llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, @@ -3713,7 +3717,9 @@ } case PP_INCLUDED_FILES: - readIncludedFiles(F, Blob, PP); + if (F.Kind == MK_PCH || F.Kind == MK_Preamble || F.Kind == MK_MainFile) + for (const FileEntry *File : readIncludedFiles(F, Blob)) + PP.markTransitivelyIncluded(File); break; case LATE_PARSED_TEMPLATE: @@ -5726,6 +5732,10 @@ CurrentModule->ExportAsModule = Blob.str(); ModMap.addLinkAsDependency(CurrentModule); break; + + case SUBMODULE_INCLUDED_FILES: + F.SubmoduleIncludedFiles.insert( + {CurrentModule->getFullModuleName(), Blob}); } } } @@ -8618,6 +8628,26 @@ return LocalID + I->second; } +const Preprocessor::IncludedFilesSet *ASTReader::getIncludedFiles(Module *M) { + ModuleFile *F = getModuleManager().lookup(M->getASTFile()); + if (!F) + return nullptr; + + auto ResultIt = + SubmoduleIncludedFiles.insert({M, Preprocessor::IncludedFilesSet{}}); + Preprocessor::IncludedFilesSet &Result = ResultIt.first->second; + if (!ResultIt.second) + return &Result; + + auto It = F->SubmoduleIncludedFiles.find(M->getFullModuleName()); + if (It == F->SubmoduleIncludedFiles.end()) + return nullptr; + StringRef 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 @@ -2254,15 +2254,14 @@ return false; } -void ASTWriter::writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP) { +void ASTWriter::writeIncludedFiles( + raw_ostream &Out, const Preprocessor::IncludedFilesSet &Files) { using namespace llvm::support; - const Preprocessor::IncludedFilesSet &IncludedFiles = PP.getIncludedFiles(); - std::vector IncludedInputFileIDs; - IncludedInputFileIDs.reserve(IncludedFiles.size()); + IncludedInputFileIDs.reserve(Files.size()); - for (const FileEntry *File : IncludedFiles) { + for (const FileEntry *File : Files) { auto InputFileIt = InputFileIDs.find(File); if (InputFileIt == InputFileIDs.end()) continue; @@ -2486,7 +2485,8 @@ Stream.EmitRecordWithBlob(MacroOffsetAbbrev, Record, bytes(MacroOffsets)); } - { + if (const Preprocessor::IncludedFilesSet *Includes = + PP.getNullSubmoduleIncludes()) { auto Abbrev = std::make_shared(); Abbrev->Add(BitCodeAbbrevOp(PP_INCLUDED_FILES)); Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); @@ -2494,7 +2494,7 @@ SmallString<2048> Buffer; raw_svector_ostream Out(Buffer); - writeIncludedFiles(Out, PP); + writeIncludedFiles(Out, *Includes); RecordData::value_type Record[] = {PP_INCLUDED_FILES}; Stream.EmitRecordWithBlob(IncludedFilesAbbrev, Record, Buffer.data(), Buffer.size()); @@ -2754,6 +2754,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), @@ -2895,6 +2900,16 @@ Stream.EmitRecordWithBlob(ExportAsAbbrev, Record, Mod->ExportAsModule); } + if (const Preprocessor::IncludedFilesSet *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