diff options
10 files changed, 183 insertions, 57 deletions
diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index be9d2bdbd2d..1c81fc60906 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -56,6 +56,9 @@ def err_imported_module_not_found : Error< def err_imported_module_modmap_changed : Error< "module '%0' imported by AST file '%1' found in a different module map file" " (%2) than when the importing AST file was built (%3)">, DefaultFatal; +def err_module_different_modmap : Error< + "module '%0' %select{uses|does not use}1 additional module map '%2'" + "%select{| not}1 used when the module was built">; def warn_module_conflict : Warning< "module '%0' conflicts with already-imported module '%1': %2">, InGroup<ModuleConflict>; diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index d37f8e833aa..d92272bbe5c 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -107,6 +107,8 @@ public: } }; + typedef llvm::SmallPtrSet<const FileEntry *, 1> AdditionalModMapsSet; + private: typedef llvm::DenseMap<const FileEntry *, SmallVector<KnownHeader, 1> > HeadersMap; @@ -150,6 +152,8 @@ private: /// inference. llvm::DenseMap<const Module *, const FileEntry *> InferredModuleAllowedBy; + llvm::DenseMap<const Module *, AdditionalModMapsSet> AdditionalModMaps; + /// \brief Describes whether we haved parsed a particular file as a module /// map. llvm::DenseMap<const FileEntry *, bool> ParsedModuleMap; @@ -349,7 +353,7 @@ public: /// /// \returns The file entry for the module map file containing the given /// module, or NULL if the module definition was inferred. - const FileEntry *getContainingModuleMapFile(Module *Module) const; + const FileEntry *getContainingModuleMapFile(const Module *Module) const; /// \brief Get the module map file that (along with the module name) uniquely /// identifies this module. @@ -360,10 +364,25 @@ public: /// of inferred modules, returns the module map that allowed the inference /// (e.g. contained 'module *'). Otherwise, returns /// getContainingModuleMapFile(). - const FileEntry *getModuleMapFileForUniquing(Module *M) const; + const FileEntry *getModuleMapFileForUniquing(const Module *M) const; void setInferredModuleAllowedBy(Module *M, const FileEntry *ModuleMap); + /// \brief Get any module map files other than getModuleMapFileForUniquing(M) + /// that define submodules of a top-level module \p M. This is cheaper than + /// getting the module map file for each submodule individually, since the + /// expected number of results is very small. + AdditionalModMapsSet *getAdditionalModuleMapFiles(const Module *M) { + auto I = AdditionalModMaps.find(M); + if (I == AdditionalModMaps.end()) + return nullptr; + return &I->second; + } + + void addAdditionalModuleMapFile(const Module *M, const FileEntry *ModuleMap) { + AdditionalModMaps[M].insert(ModuleMap); + } + /// \brief Resolve all of the unresolved exports in the given module. /// /// \param Mod The module whose exports should be resolved. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index c8701c4fbc7..213eacdfbc5 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1117,6 +1117,9 @@ private: bool ReadSourceManagerBlock(ModuleFile &F); llvm::BitstreamCursor &SLocCursorForID(int ID); SourceLocation getImportLocation(ModuleFile *F); + ASTReadResult ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, + const ModuleFile *ImportedBy, + unsigned ClientLoadCapabilities); ASTReadResult ReadSubmoduleBlock(ModuleFile &F, unsigned ClientLoadCapabilities); static bool ParseLanguageOptions(const RecordData &Record, bool Complain, diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 92de71d8f37..0af63e1bf44 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -797,7 +797,7 @@ void ModuleMap::addHeader(Module *Mod, const FileEntry *Header, } const FileEntry * -ModuleMap::getContainingModuleMapFile(Module *Module) const { +ModuleMap::getContainingModuleMapFile(const Module *Module) const { if (Module->DefinitionLoc.isInvalid()) return nullptr; @@ -805,7 +805,7 @@ ModuleMap::getContainingModuleMapFile(Module *Module) const { SourceMgr.getFileID(Module->DefinitionLoc)); } -const FileEntry *ModuleMap::getModuleMapFileForUniquing(Module *M) const { +const FileEntry *ModuleMap::getModuleMapFileForUniquing(const Module *M) const { if (M->IsInferred) { assert(InferredModuleAllowedBy.count(M) && "missing inferred module map"); return InferredModuleAllowedBy.find(M)->second; @@ -1347,8 +1347,11 @@ void ModuleMapParser::parseModuleDecl() { // This module map defines a submodule. Go find the module of which it // is a submodule. ActiveModule = nullptr; + const Module *TopLevelModule = nullptr; for (unsigned I = 0, N = Id.size() - 1; I != N; ++I) { if (Module *Next = Map.lookupModuleQualified(Id[I].first, ActiveModule)) { + if (I == 0) + TopLevelModule = Next; ActiveModule = Next; continue; } @@ -1363,7 +1366,14 @@ void ModuleMapParser::parseModuleDecl() { HadError = true; return; } - } + + if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) { + assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) && + "submodule defined in same file as 'module *' that allowed its " + "top-level module"); + Map.addAdditionalModuleMapFile(TopLevelModule, ModuleMapFile); + } + } StringRef ModuleName = Id.back().first; SourceLocation ModuleNameLoc = Id.back().second; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index bc7778adc6c..1cce383f2d0 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2468,45 +2468,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, break; case MODULE_MAP_FILE: - F.ModuleMapPath = Blob; - - // Try to resolve ModuleName in the current header search context and - // verify that it is found in the same module map file as we saved. If the - // top-level AST file is a main file, skip this check because there is no - // usable header search context. - assert(!F.ModuleName.empty() && - "MODULE_NAME should come before MOUDLE_MAP_FILE"); - if (F.Kind == MK_Module && - (*ModuleMgr.begin())->Kind != MK_MainFile) { - Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName); - if (!M) { - assert(ImportedBy && "top-level import should be verified"); - if ((ClientLoadCapabilities & ARR_Missing) == 0) - Diag(diag::err_imported_module_not_found) - << F.ModuleName << ImportedBy->FileName; - return Missing; - } - - HeaderSearch &HS = PP.getHeaderSearchInfo(); - const FileEntry *StoredModMap = FileMgr.getFile(F.ModuleMapPath); - const FileEntry *ModMap = - HS.getModuleMap().getModuleMapFileForUniquing(M); - if (StoredModMap == nullptr || StoredModMap != ModMap) { - assert(ModMap && "found module is missing module map file"); - assert(M->Name == F.ModuleName && "found module with different name"); - assert(ImportedBy && "top-level import should be verified"); - if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) - Diag(diag::err_imported_module_modmap_changed) - << F.ModuleName << ImportedBy->FileName - << ModMap->getName() << F.ModuleMapPath; - return OutOfDate; - } - } - - if (Listener) - Listener->ReadModuleMapFile(F.ModuleMapPath); - break; - + if (ASTReadResult Result = + ReadModuleMapFileBlock(Record, F, ImportedBy, ClientLoadCapabilities)) + return Result; case INPUT_FILE_OFFSETS: F.InputFileOffsets = (const uint32_t *)Blob.data(); F.InputFilesLoaded.resize(Record[0]); @@ -3315,6 +3279,89 @@ ASTReader::ReadASTBlock(ModuleFile &F, unsigned ClientLoadCapabilities) { } } +ASTReader::ASTReadResult +ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, + const ModuleFile *ImportedBy, + unsigned ClientLoadCapabilities) { + unsigned Idx = 0; + F.ModuleMapPath = ReadString(Record, Idx); + + // Try to resolve ModuleName in the current header search context and + // verify that it is found in the same module map file as we saved. If the + // top-level AST file is a main file, skip this check because there is no + // usable header search context. + assert(!F.ModuleName.empty() && + "MODULE_NAME should come before MOUDLE_MAP_FILE"); + if (F.Kind == MK_Module && (*ModuleMgr.begin())->Kind != MK_MainFile) { + Module *M = PP.getHeaderSearchInfo().lookupModule(F.ModuleName); + if (!M) { + assert(ImportedBy && "top-level import should be verified"); + if ((ClientLoadCapabilities & ARR_Missing) == 0) + Diag(diag::err_imported_module_not_found) + << F.ModuleName << ImportedBy->FileName; + return Missing; + } + + // Check the primary module map file. + auto &Map = PP.getHeaderSearchInfo().getModuleMap(); + const FileEntry *StoredModMap = FileMgr.getFile(F.ModuleMapPath); + const FileEntry *ModMap = Map.getModuleMapFileForUniquing(M); + if (StoredModMap == nullptr || StoredModMap != ModMap) { + assert(ModMap && "found module is missing module map file"); + assert(M->Name == F.ModuleName && "found module with different name"); + assert(ImportedBy && "top-level import should be verified"); + if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) + Diag(diag::err_imported_module_modmap_changed) + << F.ModuleName << ImportedBy->FileName + << ModMap->getName() << F.ModuleMapPath; + return OutOfDate; + } + + llvm::SmallPtrSet<const FileEntry *, 1> AdditionalStoredMaps; + for (unsigned I = 0, N = Record[Idx++]; I < N; ++I) { + // FIXME: we should use input files rather than storing names. + std::string Filename = ReadString(Record, Idx); + const FileEntry *F = + FileMgr.getFile(Filename, false, false); + if (F == nullptr) { + if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) + Error("could not find file '" + Filename +"' referenced by AST file"); + return OutOfDate; + } + AdditionalStoredMaps.insert(F); + } + + // Check any additional module map files (e.g. module.private.modulemap) + // that are not in the pcm. + if (auto *AdditionalModuleMaps = Map.getAdditionalModuleMapFiles(M)) { + for (const FileEntry *ModMap : *AdditionalModuleMaps) { + // Remove files that match + // Note: SmallPtrSet::erase is really remove + if (!AdditionalStoredMaps.erase(ModMap)) { + if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) + Diag(diag::err_module_different_modmap) + << F.ModuleName << /*new*/0 << ModMap->getName(); + return OutOfDate; + } + } + } + + // Check any additional module map files that are in the pcm, but not + // found in header search. Cases that match are already removed. + for (const FileEntry *ModMap : AdditionalStoredMaps) { + if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) + Diag(diag::err_module_different_modmap) + << F.ModuleName << /*not new*/1 << ModMap->getName(); + return OutOfDate; + } + } + + if (Listener) + Listener->ReadModuleMapFile(F.ModuleMapPath); + return Success; +} + + /// \brief Move the given method to the back of the global list of methods. static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) { // Find the entry for this selector in the method pool. @@ -4136,9 +4183,11 @@ bool ASTReader::readASTFileControlBlock(StringRef Filename, case MODULE_NAME: Listener.ReadModuleName(Blob); break; - case MODULE_MAP_FILE: - Listener.ReadModuleMapFile(Blob); + case MODULE_MAP_FILE: { + unsigned Idx = 0; + Listener.ReadModuleMapFile(ReadString(Record, Idx)); break; + } case LANGUAGE_OPTIONS: if (ParseLanguageOptions(Record, false, Listener)) return true; diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index dadbd12b23d..ccd57086c38 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1134,18 +1134,28 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context, // Module map file if (WritingModule) { - BitCodeAbbrev *Abbrev = new BitCodeAbbrev(); - Abbrev->Add(BitCodeAbbrevOp(MODULE_MAP_FILE)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Filename - unsigned AbbrevCode = Stream.EmitAbbrev(Abbrev); + Record.clear(); + auto addModMap = [&](const FileEntry *F) { + SmallString<128> ModuleMap(F->getName()); + llvm::sys::fs::make_absolute(ModuleMap); + AddString(ModuleMap.str(), Record); + }; - const FileEntry *ModMapFile = PP.getHeaderSearchInfo().getModuleMap(). - getModuleMapFileForUniquing(WritingModule); - SmallString<128> ModuleMap(ModMapFile->getName()); - llvm::sys::fs::make_absolute(ModuleMap); - RecordData Record; - Record.push_back(MODULE_MAP_FILE); - Stream.EmitRecordWithBlob(AbbrevCode, Record, ModuleMap.str()); + auto &Map = PP.getHeaderSearchInfo().getModuleMap(); + + // Primary module map file. + addModMap(Map.getModuleMapFileForUniquing(WritingModule)); + + // Additional module map files. + if (auto *AdditionalModMaps = Map.getAdditionalModuleMapFiles(WritingModule)) { + Record.push_back(AdditionalModMaps->size()); + for (const FileEntry *F : *AdditionalModMaps) + addModMap(F); + } else { + Record.push_back(0); + } + + Stream.EmitRecord(MODULE_MAP_FILE, Record); } // Imports diff --git a/clang/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h new file mode 100644 index 00000000000..3ab77431aa2 --- /dev/null +++ b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Headers/AddRemovePrivate.h @@ -0,0 +1 @@ +// AddRemovePrivate.h diff --git a/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap new file mode 100644 index 00000000000..7d84297a970 --- /dev/null +++ b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.modulemap @@ -0,0 +1 @@ +framework module AddRemovePrivate { umbrella header "AddRemovePrivate.h" } diff --git a/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap new file mode 100644 index 00000000000..69b67c2cee7 --- /dev/null +++ b/clang/test/Modules/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap @@ -0,0 +1 @@ +explicit module AddRemovePrivate.Private { } diff --git a/clang/test/Modules/add-remove-private.m b/clang/test/Modules/add-remove-private.m new file mode 100644 index 00000000000..7aa1b39fc22 --- /dev/null +++ b/clang/test/Modules/add-remove-private.m @@ -0,0 +1,29 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: rm -rf %t.mcp +// RUN: mkdir -p %t +// RUN: cp -r %S/Inputs/AddRemovePrivate.framework %t/AddRemovePrivate.framework + +// Build with module.private.modulemap +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP +// RUN: cp %t.mcp/AddRemovePrivate.pcm %t/with.pcm + +// Build without module.private.modulemap +// RUN: rm %t/AddRemovePrivate.framework/Modules/module.private.modulemap +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify +// RUN: not diff %t.mcp/AddRemovePrivate.pcm %t/with.pcm +// RUN: cp %t.mcp/AddRemovePrivate.pcm %t/without.pcm +// RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -DP 2>&1 | FileCheck %s +// CHECK: no submodule named 'Private' + +// Build with module.private.modulemap (again) +// RUN: cp %S/Inputs/AddRemovePrivate.framework/Modules/module.private.modulemap %t/AddRemovePrivate.framework/Modules/module.private.modulemap +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.mcp -fdisable-module-hash -F %t %s -verify -DP +// RUN: not diff %t.mcp/AddRemovePrivate.pcm %t/without.pcm + +// expected-no-diagnostics + +@import AddRemovePrivate; +#ifdef P +@import AddRemovePrivate.Private; +#endif |
