diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 524ec620c4076..950392b91ac7c 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -275,8 +275,8 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath, // listener. Reader.setListener(nullptr); - if (Reader.ReadAST(ModuleFilePath, serialization::MK_MainFile, - SourceLocation(), + if (Reader.ReadAST(ModuleFileName::makeExplicit(ModuleFilePath), + serialization::MK_MainFile, SourceLocation(), ASTReader::ARR_None) != ASTReader::Success) return false; diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 69a1de6f79b35..d2aea2f09353b 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -54,6 +54,113 @@ class TargetInfo; /// Describes the name of a module. using ModuleId = SmallVector, 2>; +namespace serialization { +class ModuleManager; +} // namespace serialization + +/// Deduplication key for a loaded module file in \c ModuleManager. +/// +/// For implicitly-built modules, this is the \c DirectoryEntry of the module +/// cache and the module file name with the (optional) context hash. +/// This enables using \c FileManager's inode-based canonicalization of the +/// user-provided module cache path without hitting issues on file systems that +/// recycle inodes for recompiled module files. +/// +/// For explicitly-built modules, this is \c FileEntry. +/// This uses \c FileManager's inode-based canonicalization of the user-provided +/// module file path. Because input explicitly-built modules do not change +/// during the lifetime of the compiler, inode recycling is not of concern here. +class ModuleFileKey { + /// The FileManager entity used for deduplication. + const void *Ptr; + /// The path relative to the module cache path for implicit module file, empty + /// for other kinds of module files. + std::string ImplicitModulePathSuffix; + + friend class serialization::ModuleManager; + friend class ModuleFileName; + friend llvm::DenseMapInfo; + + ModuleFileKey(const void *Ptr) : Ptr(Ptr) {} + + ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {} + + ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix) + : Ptr(ModuleCacheDir), ImplicitModulePathSuffix(PathSuffix) {} + +public: + bool operator==(const ModuleFileKey &Other) const { + return Ptr == Other.Ptr && + ImplicitModulePathSuffix == Other.ImplicitModulePathSuffix; + } + + bool operator!=(const ModuleFileKey &Other) const { + return !operator==(Other); + } +}; + +/// Identifies a module file to be loaded. +/// +/// For implicitly-built module files, the path is split into the module cache +/// path and the module file name with the (optional) context hash. For all +/// other types of module files, this is just the file system path. +class ModuleFileName { + std::string Path; + unsigned ImplicitModuleSuffixLength = 0; + +public: + /// Creates an empty module file name. + ModuleFileName() = default; + + /// Creates a file name for an explicit module. + static ModuleFileName makeExplicit(std::string Name) { + ModuleFileName File; + File.Path = std::move(Name); + return File; + } + + /// Creates a file name for an explicit module. + static ModuleFileName makeExplicit(StringRef Name) { + return makeExplicit(Name.str()); + } + + /// Creates a file name for an implicit module. + static ModuleFileName makeImplicit(std::string Name, unsigned SuffixLength) { + assert(SuffixLength != 0 && "Empty suffix for implicit module file name"); + assert(SuffixLength <= Name.size() && + "Suffix for implicit module file name out-of-bounds"); + ModuleFileName File; + File.Path = std::move(Name); + File.ImplicitModuleSuffixLength = SuffixLength; + return File; + } + + /// Creates a file name for an implicit module. + static ModuleFileName makeImplicit(StringRef Name, unsigned SuffixLength) { + return makeImplicit(Name.str(), SuffixLength); + } + + /// Returns the suffix length for an implicit module name, zero otherwise. + unsigned getImplicitModuleSuffixLength() const { + return ImplicitModuleSuffixLength; + } + + /// Returns the plain module file name. + StringRef str() const { return Path; } + + /// Converts to StringRef representing the plain module file name. + operator StringRef() const { return Path; } + + /// Checks whether the module file name is empty. + bool empty() const { return Path.empty(); } + + /// Creates the deduplication key for use in \c ModuleManager. + /// Returns an empty optional if: + /// * the module cache does not exist for an implicit module name, + /// * the module file does not exist for an explicit module name. + std::optional makeKey(FileManager &FileMgr) const; +}; + /// The signature of a module, which is a hash of the AST content. struct ASTFileSignature : std::array { using BaseT = std::array; @@ -926,4 +1033,23 @@ class VisibleModuleSet { } // namespace clang +template <> struct llvm::DenseMapInfo { + static clang::ModuleFileKey getEmptyKey() { + return DenseMapInfo::getEmptyKey(); + } + + static clang::ModuleFileKey getTombstoneKey() { + return DenseMapInfo::getTombstoneKey(); + } + + static unsigned getHashValue(const clang::ModuleFileKey &Val) { + return hash_combine(Val.Ptr, Val.ImplicitModulePathSuffix); + } + + static bool isEqual(const clang::ModuleFileKey &LHS, + const clang::ModuleFileKey &RHS) { + return LHS == RHS; + } +}; + #endif // LLVM_CLANG_BASIC_MODULE_H diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index f7a0d1064110a..0d684d5c7f9fe 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -17,6 +17,7 @@ #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" #include "clang/Lex/DependencyDirectivesScanner.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/ModuleLoader.h" #include "llvm/ADT/ArrayRef.h" @@ -867,7 +868,7 @@ class CompilerInstance : public ModuleLoader { void createASTReader(); - bool loadModuleFile(StringRef FileName, + bool loadModuleFile(ModuleFileName FileName, serialization::ModuleFile *&LoadedModuleFile); /// Configuration object for making the result of \c cloneForModuleCompile() diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 6ca607f83966d..48dd58ceed090 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -664,7 +664,7 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileName(Module *Module); + ModuleFileName getCachedModuleFileName(Module *Module); /// Retrieve the name of the prebuilt module file that should be used /// to load a module with the given name. @@ -676,8 +676,8 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getPrebuiltModuleFileName(StringRef ModuleName, - bool FileMapOnly = false); + ModuleFileName getPrebuiltModuleFileName(StringRef ModuleName, + bool FileMapOnly = false); /// Retrieve the name of the prebuilt module file that should be used /// to load the given module. @@ -686,7 +686,7 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getPrebuiltImplicitModuleFileName(Module *Module); + ModuleFileName getPrebuiltImplicitModuleFileName(Module *Module); /// Retrieve the name of the (to-be-)cached module file that should /// be used to load a module with the given name. @@ -698,8 +698,8 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileName(StringRef ModuleName, - StringRef ModuleMapPath); + ModuleFileName getCachedModuleFileName(StringRef ModuleName, + StringRef ModuleMapPath); /// Lookup a module Search for a module with the given name. /// @@ -815,13 +815,13 @@ class HeaderSearch { /// \param ModuleMapPath A path that when combined with \c ModuleName /// uniquely identifies this module. See Module::ModuleMap. /// - /// \param CachePath A path to the module cache. + /// \param NormalizedCachePath The normalized path to the module cache. /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileNameImpl(StringRef ModuleName, - StringRef ModuleMapPath, - StringRef CachePath); + ModuleFileName getCachedModuleFileNameImpl(StringRef ModuleName, + StringRef ModuleMapPath, + StringRef NormalizedCachePath); /// Retrieve a module with the given name, which may be part of the /// given framework. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 5db0b08f877ce..4e8fe1d32d42e 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -44,7 +44,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 36; +const unsigned VERSION_MAJOR = 37; /// AST file minor version number supported by this version of /// Clang. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 14f7d8a69a1b3..e9706d0ea2f2b 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1549,7 +1549,7 @@ class ASTReader : Mod(Mod), ImportedBy(ImportedBy), ImportLoc(ImportLoc) {} }; - ASTReadResult ReadASTCore(StringRef FileName, ModuleKind Type, + ASTReadResult ReadASTCore(ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, ModuleFile *ImportedBy, SmallVectorImpl &Loaded, off_t ExpectedSize, time_t ExpectedModTime, @@ -1885,7 +1885,7 @@ class ASTReader /// NewLoadedModuleFile would refer to the address of the new loaded top level /// module. The state of NewLoadedModuleFile is unspecified if the AST file /// isn't loaded successfully. - ASTReadResult ReadAST(StringRef FileName, ModuleKind Type, + ASTReadResult ReadAST(ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, unsigned ClientLoadCapabilities, ModuleFile **NewLoadedModuleFile = nullptr); diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 519a74d920129..e761cadfcd86f 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -144,8 +144,10 @@ enum class InputFilesValidation { /// other modules. class ModuleFile { public: - ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation) - : Kind(Kind), File(File), Generation(Generation) {} + ModuleFile(ModuleKind Kind, ModuleFileKey FileKey, FileEntryRef File, + unsigned Generation) + : Kind(Kind), FileKey(std::move(FileKey)), File(File), + Generation(Generation) {} ~ModuleFile(); // === General information === @@ -157,7 +159,10 @@ class ModuleFile { ModuleKind Kind; /// The file name of the module file. - std::string FileName; + ModuleFileName FileName; + + /// The key ModuleManager used for the module file. + ModuleFileKey FileKey; /// The name of the module. std::string ModuleName; diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 8ab70b6630f47..162856f2f14c0 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -16,6 +16,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Serialization/ModuleFile.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" @@ -56,8 +57,8 @@ class ModuleManager { // to implement short-circuiting logic when running DFS over the dependencies. SmallVector Roots; - /// All loaded modules, indexed by name. - llvm::DenseMap Modules; + /// All loaded modules. + llvm::DenseMap Modules; /// FileManager that handles translating between filenames and /// FileEntry *. @@ -172,14 +173,22 @@ class ModuleManager { ModuleFile &operator[](unsigned Index) const { return *Chain[Index]; } /// Returns the module associated with the given file name. + /// Soon to be removed. ModuleFile *lookupByFileName(StringRef FileName) const; /// Returns the module associated with the given module name. ModuleFile *lookupByModuleName(StringRef ModName) const; /// Returns the module associated with the given module file. + /// Soon to be removed. ModuleFile *lookup(const FileEntry *File) const; + /// Returns the module associated with the given module file name. + ModuleFile *lookupByFileName(ModuleFileName FileName) const; + + /// Returns the module associated with the given module file key. + ModuleFile *lookup(ModuleFileKey Key) const; + /// Returns the in-memory (virtual file) buffer with the given name std::unique_ptr lookupBuffer(StringRef Name); @@ -237,14 +246,13 @@ class ModuleManager { /// /// \return A pointer to the module that corresponds to this file name, /// and a value indicating whether the module was loaded. - AddModuleResult addModule(StringRef FileName, ModuleKind Type, - SourceLocation ImportLoc, - ModuleFile *ImportedBy, unsigned Generation, - off_t ExpectedSize, time_t ExpectedModTime, + AddModuleResult addModule(ModuleFileName FileName, ModuleKind Type, + SourceLocation ImportLoc, ModuleFile *ImportedBy, + unsigned Generation, off_t ExpectedSize, + time_t ExpectedModTime, ASTFileSignature ExpectedSignature, ASTFileSignatureReader ReadSignature, - ModuleFile *&Module, - std::string &ErrorStr); + ModuleFile *&Module, std::string &ErrorStr); /// Remove the modules starting from First (to the end). void removeModules(ModuleIterator First); @@ -282,26 +290,6 @@ class ModuleManager { void visit(llvm::function_ref Visitor, llvm::SmallPtrSetImpl *ModuleFilesHit = nullptr); - /// Attempt to resolve the given module file name to a file entry. - /// - /// \param FileName The name of the module file. - /// - /// \param ExpectedSize The size that the module file is expected to have. - /// If the actual size differs, the resolver should return \c true. - /// - /// \param ExpectedModTime The modification time that the module file is - /// expected to have. If the actual modification time differs, the resolver - /// should return \c true. - /// - /// \param File Will be set to the file if there is one, or null - /// otherwise. - /// - /// \returns True if a file exists but does not meet the size/ - /// modification time criteria, false if the file is either available and - /// suitable, or is missing. - bool lookupModuleFile(StringRef FileName, off_t ExpectedSize, - time_t ExpectedModTime, OptionalFileEntryRef &File); - /// View the graphviz representation of the module graph. void viewGraph(); diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 97f742d292224..81e28e46d36ca 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -33,6 +33,25 @@ using namespace clang; +std::optional +ModuleFileName::makeKey(FileManager &FileMgr) const { + if (ImplicitModuleSuffixLength) { + StringRef ModuleCachePath = + StringRef(Path).drop_back(ImplicitModuleSuffixLength); + StringRef ImplicitModuleSuffix = + StringRef(Path).take_back(ImplicitModuleSuffixLength); + if (auto ModuleCache = FileMgr.getOptionalDirectoryRef( + ModuleCachePath, /*CacheFailure=*/false)) + return ModuleFileKey(*ModuleCache, ImplicitModuleSuffix); + } else { + if (auto ModuleFile = FileMgr.getOptionalFileRef(Path, /*OpenFile=*/true, + /*CacheFailure=*/false)) + return ModuleFileKey(*ModuleFile); + } + + return std::nullopt; +} + Module::Module(ModuleConstructorTag, StringRef Name, SourceLocation DefinitionLoc, Module *Parent, bool IsFramework, bool IsExplicit, unsigned VisibilityID) diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index d1aced2592be1..20284c0d9165a 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -614,7 +614,7 @@ struct AsyncModuleCompile : PPCallbacks { HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo(); ModuleCache &ModCache = CI.getModuleCache(); - std::string ModuleFileName = HS.getCachedModuleFileName(M); + ModuleFileName ModuleFileName = HS.getCachedModuleFileName(M); uint64_t Timestamp = ModCache.getModuleTimestamp(ModuleFileName); // Someone else already built/validated the PCM. diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 1d249ebaa1492..1e10178285bbd 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -835,8 +835,9 @@ std::unique_ptr ASTUnit::LoadFromASTFile( AST->HSOpts->ForceCheckCXX20ModulesInputFiles = HSOpts.ForceCheckCXX20ModulesInputFiles; - switch (AST->Reader->ReadAST(Filename, serialization::MK_MainFile, - SourceLocation(), ASTReader::ARR_None)) { + switch (AST->Reader->ReadAST(ModuleFileName::makeExplicit(Filename), + serialization::MK_MainFile, SourceLocation(), + ASTReader::ARR_None)) { case ASTReader::Success: break; diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp index 2338698cee9d1..aa93785322a5c 100644 --- a/clang/lib/Frontend/ChainedIncludesSource.cpp +++ b/clang/lib/Frontend/ChainedIncludesSource.cpp @@ -69,7 +69,8 @@ createASTReader(CompilerInstance &CI, StringRef pchFile, Reader->addInMemoryBuffer(sr, std::move(MemBufs[ti])); } Reader->setDeserializationListener(deserialListener); - switch (Reader->ReadAST(pchFile, serialization::MK_PCH, SourceLocation(), + switch (Reader->ReadAST(ModuleFileName::makeExplicit(pchFile), + serialization::MK_PCH, SourceLocation(), ASTReader::ARR_None)) { case ASTReader::Success: // Set the predefines buffer as suggested by the PCH reader. diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index b4ad911545774..ca0bcd2dc2ee3 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -659,11 +659,10 @@ IntrusiveRefCntPtr CompilerInstance::createPCHExternalASTSource( ASTReader::ListenerScope ReadModuleNamesListener(*Reader, std::move(Listener)); - switch (Reader->ReadAST(Path, + switch (Reader->ReadAST(ModuleFileName::makeExplicit(Path), Preamble ? serialization::MK_Preamble : serialization::MK_PCH, - SourceLocation(), - ASTReader::ARR_None)) { + SourceLocation(), ASTReader::ARR_None)) { case ASTReader::Success: // Set the predefines buffer as suggested by the PCH reader. Typically, the // predefines buffer will be empty. @@ -1398,7 +1397,8 @@ std::unique_ptr CompilerInstance::cloneForModuleCompile( static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, - Module *Module, StringRef ModuleFileName, + Module *Module, + ModuleFileName ModuleFileName, bool *OutOfDate, bool *Missing) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); @@ -1439,7 +1439,7 @@ static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance, static bool compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, - StringRef ModuleFileName) { + ModuleFileName ModuleFileName) { { auto Instance = ImportingInstance.cloneForModuleCompile( ModuleNameLoc, Module, ModuleFileName); @@ -1480,9 +1480,11 @@ enum class CompileOrReadResult : uint8_t { /// Attempt to compile the module in a separate compiler instance behind a lock /// (to avoid building the same module in multiple compiler instances), or read /// the AST produced by another compiler instance. -static CompileOrReadResult compileModuleBehindLockOrRead( - CompilerInstance &ImportingInstance, SourceLocation ImportLoc, - SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) { +static CompileOrReadResult +compileModuleBehindLockOrRead(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, + SourceLocation ModuleNameLoc, Module *Module, + ModuleFileName ModuleFileName) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); Diags.Report(ModuleNameLoc, diag::remark_module_lock) @@ -1556,7 +1558,8 @@ static CompileOrReadResult compileModuleBehindLockOrRead( static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, - Module *Module, StringRef ModuleFileName) { + Module *Module, + ModuleFileName ModuleFileName) { if (ImportingInstance.getInvocation() .getFrontendOpts() .BuildingImplicitModuleUsesLock) { @@ -1710,11 +1713,11 @@ void CompilerInstance::createASTReader() { } bool CompilerInstance::loadModuleFile( - StringRef FileName, serialization::ModuleFile *&LoadedModuleFile) { + ModuleFileName FileName, serialization::ModuleFile *&LoadedModuleFile) { llvm::Timer Timer; if (timerGroup) - Timer.init("preloading." + FileName.str(), "Preloading " + FileName.str(), - *timerGroup); + Timer.init("preloading." + std::string(FileName.str()), + "Preloading " + std::string(FileName.str()), *timerGroup); llvm::TimeRegion TimeLoading(timerGroup ? &Timer : nullptr); // If we don't already have an ASTReader, create one now. @@ -1771,7 +1774,7 @@ enum ModuleSource { /// Select a source for loading the named module and compute the filename to /// load it from. static ModuleSource selectModuleSource( - Module *M, StringRef ModuleName, std::string &ModuleFilename, + Module *M, StringRef ModuleName, ModuleFileName &ModuleFilename, const std::map> &BuiltModules, HeaderSearch &HS) { assert(ModuleFilename.empty() && "Already has a module source?"); @@ -1780,7 +1783,7 @@ static ModuleSource selectModuleSource( // via a module build pragma. auto BuiltModuleIt = BuiltModules.find(ModuleName); if (BuiltModuleIt != BuiltModules.end()) { - ModuleFilename = BuiltModuleIt->second; + ModuleFilename = ModuleFileName::makeExplicit(BuiltModuleIt->second); return MS_ModuleBuildPragma; } @@ -1820,7 +1823,7 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( checkConfigMacros(getPreprocessor(), M, ImportLoc); // Select the source and filename for loading the named module. - std::string ModuleFilename; + ModuleFileName ModuleFilename; ModuleSource Source = selectModuleSource(M, ModuleName, ModuleFilename, BuiltModules, HS); SourceLocation ModuleNameLoc = ModuleNameRange.getBegin(); @@ -1849,8 +1852,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( // Time how long it takes to load the module. llvm::Timer Timer; if (timerGroup) - Timer.init("loading." + ModuleFilename, "Loading " + ModuleFilename, - *timerGroup); + Timer.init("loading." + std::string(ModuleFilename.str()), + "Loading " + std::string(ModuleFilename.str()), *timerGroup); llvm::TimeRegion TimeLoading(timerGroup ? &Timer : nullptr); llvm::TimeTraceScope TimeScope("Module Load", ModuleName); @@ -2070,7 +2073,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, PrivateModule, PP->getIdentifierInfo(Module->Name)->getTokenID()); PrivPath.emplace_back(Path[0].getLoc(), &II); - std::string FileName; + ModuleFileName FileName; // If there is a modulemap module or prebuilt module, load it. if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, ImportLoc, true, !IsInclusionDirective) || diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 84ceb22208801..81788d17acc4d 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -892,7 +892,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, for (serialization::ModuleFile &MF : MM) if (&MF != &PrimaryModule) - CI.getFrontendOpts().ModuleFiles.push_back(MF.FileName); + CI.getFrontendOpts().ModuleFiles.emplace_back(MF.FileName.str()); ASTReader->visitTopLevelModuleMaps(PrimaryModule, [&](FileEntryRef FE) { CI.getFrontendOpts().ModuleMapFiles.push_back( @@ -1288,7 +1288,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, // If we were asked to load any module files, do so now. for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles) { serialization::ModuleFile *Loaded = nullptr; - if (!CI.loadModuleFile(ModuleFile, Loaded)) + if (!CI.loadModuleFile(ModuleFileName::makeExplicit(ModuleFile), Loaded)) return false; if (Loaded && Loaded->StandardCXXModule) diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 7bcb4d7d89db1..e5eaab0da7adb 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -251,9 +251,9 @@ GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI, ModuleMapFile = InFile; HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo(); - CI.getFrontendOpts().OutputFile = - HS.getCachedModuleFileName(CI.getLangOpts().CurrentModule, - ModuleMapFile); + ModuleFileName FileName = HS.getCachedModuleFileName( + CI.getLangOpts().CurrentModule, ModuleMapFile); + CI.getFrontendOpts().OutputFile = FileName.str(); } // Because this is exposed via libclang we must disable RemoveFileOnSignal. @@ -367,11 +367,9 @@ void VerifyPCHAction::ExecuteAction() { /*AllowConfigurationMismatch*/ true, /*ValidateSystemInputs*/ true, /*ForceValidateUserInputs*/ true)); - Reader->ReadAST(getCurrentFile(), - Preamble ? serialization::MK_Preamble - : serialization::MK_PCH, - SourceLocation(), - ASTReader::ARR_ConfigurationMismatch); + Reader->ReadAST(ModuleFileName::makeExplicit(getCurrentFile()), + Preamble ? serialization::MK_Preamble : serialization::MK_PCH, + SourceLocation(), ASTReader::ARR_ConfigurationMismatch); } namespace { diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index de26b999683d1..3eaad16d8ba04 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -197,7 +197,7 @@ void HeaderSearch::getHeaderMapFileNames( Names.push_back(std::string(HM.first.getName())); } -std::string HeaderSearch::getCachedModuleFileName(Module *Module) { +ModuleFileName HeaderSearch::getCachedModuleFileName(Module *Module) { OptionalFileEntryRef ModuleMap = getModuleMap().getModuleMapFileForUniquing(Module); // The ModuleMap maybe a nullptr, when we load a cached C++ module without @@ -207,12 +207,12 @@ std::string HeaderSearch::getCachedModuleFileName(Module *Module) { return getCachedModuleFileName(Module->Name, ModuleMap->getNameAsRequested()); } -std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, - bool FileMapOnly) { +ModuleFileName HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, + bool FileMapOnly) { // First check the module name to pcm file map. auto i(HSOpts.PrebuiltModuleFiles.find(ModuleName)); if (i != HSOpts.PrebuiltModuleFiles.end()) - return i->second; + return ModuleFileName::makeExplicit(i->second); if (FileMapOnly || HSOpts.PrebuiltModulePaths.empty()) return {}; @@ -232,49 +232,52 @@ std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, else llvm::sys::path::append(Result, ModuleName + ".pcm"); if (getFileMgr().getOptionalFileRef(Result)) - return std::string(Result); + return ModuleFileName::makeExplicit(Result); } return {}; } -std::string HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) { +ModuleFileName HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) { OptionalFileEntryRef ModuleMap = getModuleMap().getModuleMapFileForUniquing(Module); StringRef ModuleName = Module->Name; StringRef ModuleMapPath = ModuleMap->getName(); - StringRef ContextHash = HSOpts.DisableModuleHash ? "" : getContextHash(); for (const std::string &Dir : HSOpts.PrebuiltModulePaths) { SmallString<256> CachePath(Dir); FileMgr.makeAbsolutePath(CachePath); - llvm::sys::path::append(CachePath, ContextHash); - std::string FileName = + ModuleFileName FileName = getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, CachePath); if (!FileName.empty() && getFileMgr().getOptionalFileRef(FileName)) - return FileName; + return ModuleFileName::makeExplicit(FileName); } return {}; } -std::string HeaderSearch::getCachedModuleFileName(StringRef ModuleName, - StringRef ModuleMapPath) { +ModuleFileName HeaderSearch::getCachedModuleFileName(StringRef ModuleName, + StringRef ModuleMapPath) { return getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, - getSpecificModuleCachePath()); + getNormalizedModuleCachePath()); } -std::string HeaderSearch::getCachedModuleFileNameImpl(StringRef ModuleName, - StringRef ModuleMapPath, - StringRef CachePath) { +ModuleFileName HeaderSearch::getCachedModuleFileNameImpl( + StringRef ModuleName, StringRef ModuleMapPath, StringRef CachePath) { // If we don't have a module cache path or aren't supposed to use one, we // can't do anything. if (CachePath.empty()) return {}; + // Note: This re-implements part of createSpecificModuleCachePathImpl() in + // order to be able to correctly construct ModuleFileName. + SmallString<256> Result(CachePath); + unsigned SuffixBegin = Result.size(); if (HSOpts.DisableModuleHash) { llvm::sys::path::append(Result, ModuleName + ".pcm"); } else { + llvm::sys::path::append(Result, ContextHash); + // Construct the name -.pcm which should // ideally be globally unique to this particular module. Name collisions // in the hash are safe (because any translation unit can only import one @@ -292,7 +295,7 @@ std::string HeaderSearch::getCachedModuleFileNameImpl(StringRef ModuleName, llvm::APInt(64, Hash).toStringUnsigned(HashStr, /*Radix*/36); llvm::sys::path::append(Result, ModuleName + "-" + HashStr + ".pcm"); } - return Result.str().str(); + return ModuleFileName::makeImplicit(Result, Result.size() - SuffixBegin); } Module *HeaderSearch::lookupModule(StringRef ModuleName, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 515eaf8d1caed..e9c4ff3a75801 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2880,7 +2880,7 @@ InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) { std::string ErrorStr = "could not find file '"; ErrorStr += *Filename; ErrorStr += "' referenced by AST file '"; - ErrorStr += F.FileName; + ErrorStr += F.FileName.str(); ErrorStr += "'"; Error(ErrorStr); } @@ -3477,8 +3477,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, off_t StoredSize = 0; time_t StoredModTime = 0; + unsigned ImplicitModuleSuffixLength = 0; ASTFileSignature StoredSignature; - std::string ImportedFile; + ModuleFileName ImportedFile; std::string StoredFile; bool IgnoreImportedByNote = false; @@ -3500,17 +3501,21 @@ ASTReader::ReadControlBlock(ModuleFile &F, if (!IsImportingStdCXXModule) { StoredSize = (off_t)Record[Idx++]; StoredModTime = (time_t)Record[Idx++]; + ImplicitModuleSuffixLength = (unsigned)Record[Idx++]; StringRef SignatureBytes = Blob.substr(0, ASTFileSignature::size); StoredSignature = ASTFileSignature::create(SignatureBytes.begin(), SignatureBytes.end()); Blob = Blob.substr(ASTFileSignature::size); - // Use BaseDirectoryAsWritten to ensure we use the same path in the - // ModuleCache as when writing. StoredFile = ReadPathBlob(BaseDirectoryAsWritten, Record, Idx, Blob); if (ImportedFile.empty()) { - ImportedFile = StoredFile; + ImportedFile = ImplicitModuleSuffixLength + ? ModuleFileName::makeImplicit( + StoredFile, ImplicitModuleSuffixLength) + : ModuleFileName::makeExplicit(StoredFile); + assert((ImportedKind == MK_ImplicitModule) == + (ImplicitModuleSuffixLength != 0)); } else if (!getDiags().isIgnored( diag::warn_module_file_mapping_mismatch, CurrentImportLoc)) { @@ -4656,8 +4661,8 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, // This module was built with a different module map. Diag(diag::err_imported_module_not_found) << F.ModuleName << F.FileName - << (ImportedBy ? ImportedBy->FileName : "") << F.ModuleMapPath - << !ImportedBy; + << (ImportedBy ? ImportedBy->FileName.str() : "") + << F.ModuleMapPath << !ImportedBy; // In case it was imported by a PCH, there's a chance the user is // just missing to include the search path to the directory containing // the modulemap. @@ -4913,7 +4918,8 @@ static bool SkipCursorToBlock(BitstreamCursor &Cursor, unsigned BlockID) { } } -ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, +ASTReader::ASTReadResult ASTReader::ReadAST(ModuleFileName FileName, + ModuleKind Type, SourceLocation ImportLoc, unsigned ClientLoadCapabilities, ModuleFile **NewLoadedModuleFile) { @@ -5189,15 +5195,11 @@ static unsigned moduleKindForDiagnostic(ModuleKind Kind) { llvm_unreachable("unknown module kind"); } -ASTReader::ASTReadResult -ASTReader::ReadASTCore(StringRef FileName, - ModuleKind Type, - SourceLocation ImportLoc, - ModuleFile *ImportedBy, - SmallVectorImpl &Loaded, - off_t ExpectedSize, time_t ExpectedModTime, - ASTFileSignature ExpectedSignature, - unsigned ClientLoadCapabilities) { +ASTReader::ASTReadResult ASTReader::ReadASTCore( + ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, + ModuleFile *ImportedBy, SmallVectorImpl &Loaded, + off_t ExpectedSize, time_t ExpectedModTime, + ASTFileSignature ExpectedSignature, unsigned ClientLoadCapabilities) { ModuleFile *M; std::string ErrorStr; ModuleManager::AddModuleResult AddResult @@ -6142,8 +6144,8 @@ bool ASTReader::readASTFileControlBlock( continue; } - // Skip Size and ModTime. - Idx += 1 + 1; + // Skip Size, ModTime and ImplicitModuleSuffix. + Idx += 1 + 1 + 1; // Skip signature. Blob = Blob.substr(ASTFileSignature::size); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index c4815e956e53f..2fb204ce5342b 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1567,6 +1567,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Standard C++ mod Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File size Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File timestamp + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Implicit suff len Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File name len Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Strings unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev)); @@ -1593,6 +1594,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Record.push_back(0); Record.push_back(0); Record.push_back(0); + Record.push_back(0); } else { // If we have calculated signature, there is no need to store // the size or timestamp. @@ -1601,6 +1603,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { llvm::append_range(Blob, M.Signature); + Record.push_back(M.FileName.getImplicitModuleSuffixLength()); AddPathBlob(M.FileName, Record, Blob); } @@ -6151,7 +6154,7 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot, // relocatable files. We probably should call // `PreparePathForOutput(M.FileName)` to properly support relocatable // PCHs. - StringRef Name = M.isModule() ? M.ModuleName : M.FileName; + StringRef Name = M.isModule() ? M.ModuleName : M.FileName.str(); LE.write(Name.size()); Out.write(Name.data(), Name.size()); diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index 2246a3ac0a57e..8ab8717776c3d 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -638,6 +638,7 @@ llvm::Error GlobalModuleIndexBuilder::loadModuleFile(FileEntryRef File) { // Load stored size/modification time. off_t StoredSize = (off_t)Record[Idx++]; time_t StoredModTime = (time_t)Record[Idx++]; + (void)Record[Idx++]; // ImplicitModuleSuffixLength // Skip the stored signature. // FIXME: we could read the signature out of the import and validate it. diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 16c3a1e04f649..d0bf4bc74e8ab 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -58,7 +58,16 @@ ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const { } ModuleFile *ModuleManager::lookup(const FileEntry *File) const { - return Modules.lookup(File); + return lookup(ModuleFileKey(File)); +} + +ModuleFile *ModuleManager::lookupByFileName(ModuleFileName Name) const { + std::optional Key = Name.makeKey(FileMgr); + return Key ? lookup(*Key) : nullptr; +} + +ModuleFile *ModuleManager::lookup(ModuleFileKey Key) const { + return Modules.lookup(Key); } std::unique_ptr @@ -70,6 +79,23 @@ ModuleManager::lookupBuffer(StringRef Name) { return std::move(InMemoryBuffers[*Entry]); } +static bool checkModuleFile(const FileEntry *File, off_t ExpectedSize, + time_t ExpectedModTime, std::string &ErrorStr) { + assert(File && "Checking expectations of a non-existent module file"); + + if (ExpectedSize && ExpectedSize != File->getSize()) { + ErrorStr = "module file has a different size than expected"; + return true; + } + + if (ExpectedModTime && ExpectedModTime != File->getModificationTime()) { + ErrorStr = "module file has a different modification time than expected"; + return true; + } + + return false; +} + static bool checkSignature(ASTFileSignature Signature, ASTFileSignature ExpectedSignature, std::string &ErrorStr) { @@ -94,24 +120,18 @@ static void updateModuleImports(ModuleFile &MF, ModuleFile *ImportedBy, } } -ModuleManager::AddModuleResult -ModuleManager::addModule(StringRef FileName, ModuleKind Type, - SourceLocation ImportLoc, ModuleFile *ImportedBy, - unsigned Generation, - off_t ExpectedSize, time_t ExpectedModTime, - ASTFileSignature ExpectedSignature, - ASTFileSignatureReader ReadSignature, - ModuleFile *&Module, - std::string &ErrorStr) { +ModuleManager::AddModuleResult ModuleManager::addModule( + ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, + ModuleFile *ImportedBy, unsigned Generation, off_t ExpectedSize, + time_t ExpectedModTime, ASTFileSignature ExpectedSignature, + ASTFileSignatureReader ReadSignature, ModuleFile *&Module, + std::string &ErrorStr) { Module = nullptr; uint64_t InputFilesValidationTimestamp = 0; if (Type == MK_ImplicitModule) InputFilesValidationTimestamp = ModCache.getModuleTimestamp(FileName); - // Look for the file entry. This only fails if the expected size or - // modification time differ. - OptionalFileEntryRef Entry; bool IgnoreModTime = Type == MK_ExplicitModule || Type == MK_PrebuiltModule; if (ImportedBy) IgnoreModTime &= ImportedBy->Kind == MK_ExplicitModule || @@ -123,91 +143,71 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // contents, but we can't check that.) ExpectedModTime = 0; } - // Note: ExpectedSize and ExpectedModTime will be 0 for MK_ImplicitModule - // when using an ASTFileSignature. - if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) { - ErrorStr = IgnoreModTime ? "module file has a different size than expected" - : "module file has a different size or " - "modification time than expected"; - return OutOfDate; - } - if (!Entry) { + std::optional FileKey = FileName.makeKey(FileMgr); + if (!FileKey) { ErrorStr = "module file not found"; return Missing; } - // The ModuleManager's use of FileEntry nodes as the keys for its map of - // loaded modules is less than ideal. Uniqueness for FileEntry nodes is - // maintained by FileManager, which in turn uses inode numbers on hosts - // that support that. When coupled with the module cache's proclivity for - // turning over and deleting stale PCMs, this means entries for different - // module files can wind up reusing the same underlying inode. When this - // happens, subsequent accesses to the Modules map will disagree on the - // ModuleFile associated with a given file. In general, it is not sufficient - // to resolve this conundrum with a type like FileEntryRef that stores the - // name of the FileEntry node on first access because of path canonicalization - // issues. However, the paths constructed for implicit module builds are - // fully under Clang's control. We *can*, therefore, rely on their structure - // being consistent across operating systems and across subsequent accesses - // to the Modules map. - auto implicitModuleNamesMatch = [](ModuleKind Kind, const ModuleFile *MF, - FileEntryRef Entry) -> bool { - if (Kind != MK_ImplicitModule) - return true; - return Entry.getName() == MF->FileName; - }; - // Check whether we already loaded this module, before - if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) { - if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) { - // Check the stored signature. - if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) - return OutOfDate; - - Module = ModuleEntry; - updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); - return AlreadyLoaded; - } + if (ModuleFile *ModuleEntry = lookup(*FileKey)) { + // Check file properties. + if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, + ErrorStr)) + return OutOfDate; + + // Check the stored signature. + if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) + return OutOfDate; + + Module = ModuleEntry; + updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); + return AlreadyLoaded; } - // Allocate a new module. - auto NewModule = std::make_unique(Type, *Entry, Generation); - NewModule->Index = Chain.size(); - NewModule->FileName = FileName.str(); - NewModule->ImportLoc = ImportLoc; - NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; - // Load the contents of the module + OptionalFileEntryRef Entry; + llvm::MemoryBuffer *ModuleBuffer = nullptr; std::unique_ptr NewFileBuffer = nullptr; if (std::unique_ptr Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - NewModule->Buffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( + ModuleBuffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( FileName, std::move(Buffer)); - // Since the cached buffer is reused, it is safe to close the file - // descriptor that was opened while stat()ing the PCM in - // lookupModuleFile() above, it won't be needed any longer. - Entry->closeFile(); } else if (llvm::MemoryBuffer *Buffer = getModuleCache().getInMemoryModuleCache().lookupPCM( FileName)) { - NewModule->Buffer = Buffer; - // As above, the file descriptor is no longer needed. - Entry->closeFile(); + ModuleBuffer = Buffer; } else if (getModuleCache().getInMemoryModuleCache().shouldBuildPCM( FileName)) { // Report that the module is out of date, since we tried (and failed) to // import it earlier. - Entry->closeFile(); return OutOfDate; } else { + Entry = + expectedToOptional(FileName == StringRef("-") + ? FileMgr.getSTDIN() + : FileMgr.getFileRef(FileName, /*OpenFile=*/true, + /*CacheFailure=*/false)); + if (!Entry) { + ErrorStr = "module file not found"; + return Missing; + } + + // FIXME: Consider moving this after this else branch so that we check + // size/mtime expectations even when pulling the module file out of the + // in-memory module cache or the provided in-memory buffers. + // Check file properties. + if (checkModuleFile(*Entry, ExpectedSize, ExpectedModTime, ErrorStr)) + return OutOfDate; + // Get a buffer of the file and close the file descriptor when done. // The file is volatile because in a parallel build we expect multiple // compiler processes to use the same module file rebuilding it if needed. // // RequiresNullTerminator is false because module files don't need it, and // this allows the file to still be mmapped. - auto Buf = FileMgr.getBufferForFile(NewModule->File, + auto Buf = FileMgr.getBufferForFile(*Entry, /*IsVolatile=*/true, /*RequiresNullTerminator=*/false); @@ -217,9 +217,24 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, } NewFileBuffer = std::move(*Buf); - NewModule->Buffer = NewFileBuffer.get(); + ModuleBuffer = NewFileBuffer.get(); } + if (!Entry) { + // Unless we loaded the buffer from a freshly open file (else branch above), + // we don't have any FileEntry for this ModuleFile. Make one up. + // FIXME: Make it so that ModuleFile is not tied to a FileEntry. + Entry = FileMgr.getVirtualFileRef(FileName, ExpectedSize, ExpectedModTime); + } + + // Allocate a new module. + auto NewModule = + std::make_unique(Type, *FileKey, *Entry, Generation); + NewModule->Index = Chain.size(); + NewModule->FileName = FileName; + NewModule->ImportLoc = ImportLoc; + NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; + NewModule->Buffer = ModuleBuffer; // Initialize the stream. NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); @@ -233,8 +248,13 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, getModuleCache().getInMemoryModuleCache().addPCM(FileName, std::move(NewFileBuffer)); - // We're keeping this module. Store it everywhere. - Module = Modules[*Entry] = NewModule.get(); + // We're keeping this module. Store it in the map. + Module = Modules[*FileKey] = NewModule.get(); + + // Support clients that still rely on being able to look up ModuleFile with + // normal FileEntry. + // TODO: Remove this. + Modules[ModuleFileKey(*Entry)] = Module; updateModuleImports(*NewModule, ImportedBy, ImportLoc); @@ -279,8 +299,10 @@ void ModuleManager::removeModules(ModuleIterator First) { } // Delete the modules. - for (ModuleIterator victim = First; victim != Last; ++victim) - Modules.erase(victim->File); + for (ModuleIterator victim = First; victim != Last; ++victim) { + Modules.erase(ModuleFileKey(victim->File)); + Modules.erase(victim->FileKey); + } Chain.erase(Chain.begin() + (First - begin()), Chain.end()); } @@ -439,29 +461,6 @@ void ModuleManager::visit(llvm::function_ref Visitor, returnVisitState(std::move(State)); } -bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize, - time_t ExpectedModTime, - OptionalFileEntryRef &File) { - if (FileName == "-") { - File = expectedToOptional(FileMgr.getSTDIN()); - return false; - } - - // Open the file immediately to ensure there is no race between stat'ing and - // opening the file. - File = FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, - /*CacheFailure=*/false); - - if (File && - ((ExpectedSize && ExpectedSize != File->getSize()) || - (ExpectedModTime && ExpectedModTime != File->getModificationTime()))) - // Do not destroy File, as it may be referenced. If we need to rebuild it, - // it will be destroyed by removeModules. - return true; - - return false; -} - #ifndef NDEBUG namespace llvm { diff --git a/clang/test/Modules/DebugInfoNamespace.cpp b/clang/test/Modules/DebugInfoNamespace.cpp index b2095adf68675..03523b2e69e59 100644 --- a/clang/test/Modules/DebugInfoNamespace.cpp +++ b/clang/test/Modules/DebugInfoNamespace.cpp @@ -3,7 +3,7 @@ // RUN: %clang_cc1 -x objective-c++ -std=c++11 -debug-info-kind=standalone \ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps \ -// RUN: -triple %itanium_abi_triple -fmodules-cache-path=%t \ +// RUN: -triple %itanium_abi_triple -fmodules-cache-path=%t/cache \ // RUN: %s -I %S/Inputs/DebugInfoNamespace -I %t -emit-llvm -o - \ // RUN: | FileCheck %s diff --git a/clang/test/Modules/ExtDebugInfo.cpp b/clang/test/Modules/ExtDebugInfo.cpp index 184973bc1783c..0c49f8bad8668 100644 --- a/clang/test/Modules/ExtDebugInfo.cpp +++ b/clang/test/Modules/ExtDebugInfo.cpp @@ -7,7 +7,7 @@ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ // RUN: -triple %itanium_abi_triple \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // PCH: diff --git a/clang/test/Modules/ExtDebugInfo.m b/clang/test/Modules/ExtDebugInfo.m index e2611ae530063..5b19be7bf370c 100644 --- a/clang/test/Modules/ExtDebugInfo.m +++ b/clang/test/Modules/ExtDebugInfo.m @@ -5,7 +5,7 @@ // Modules: // RUN: %clang_cc1 -x objective-c -debug-info-kind=limited -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // RUN: cat %t-mod.ll | FileCheck %s --check-prefix=DWOID diff --git a/clang/test/Modules/ModuleDebugInfo.cpp b/clang/test/Modules/ModuleDebugInfo.cpp index 4d78324867bcb..720444802ddd3 100644 --- a/clang/test/Modules/ModuleDebugInfo.cpp +++ b/clang/test/Modules/ModuleDebugInfo.cpp @@ -6,7 +6,7 @@ // Modules: // RUN: rm -rf %t -// RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debugger-tuning=lldb -debug-info-kind=limited -dwarf-version=5 -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll +// RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debugger-tuning=lldb -debug-info-kind=limited -dwarf-version=5 -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-MOD %s diff --git a/clang/test/Modules/ModuleDebugInfo.m b/clang/test/Modules/ModuleDebugInfo.m index c527c43a0f4a2..0e92831ae5414 100644 --- a/clang/test/Modules/ModuleDebugInfo.m +++ b/clang/test/Modules/ModuleDebugInfo.m @@ -7,7 +7,7 @@ // Modules: // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/ModuleModuleDebugInfo.cpp b/clang/test/Modules/ModuleModuleDebugInfo.cpp index 61449643937a7..aa45fe9da0698 100644 --- a/clang/test/Modules/ModuleModuleDebugInfo.cpp +++ b/clang/test/Modules/ModuleModuleDebugInfo.cpp @@ -5,7 +5,7 @@ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ // RUN: -triple %itanium_abi_triple \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o - \ +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o - \ // RUN: | FileCheck %s #include "DebugNestedB.h" diff --git a/clang/test/Modules/debug-info-moduleimport.m b/clang/test/Modules/debug-info-moduleimport.m index acb7dbd48bdd0..bdeb05bc08a02 100644 --- a/clang/test/Modules/debug-info-moduleimport.m +++ b/clang/test/Modules/debug-info-moduleimport.m @@ -2,7 +2,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -debug-info-kind=limited -fmodules \ // RUN: -DGREETING="Hello World" -UNDEBUG \ -// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm -o - \ // RUN: | FileCheck %s --check-prefix=NOIMPORT @@ -12,7 +12,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -debug-info-kind=limited -fmodules \ // RUN: -DGREETING="Hello World" -UNDEBUG \ -// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm \ // RUN: -debugger-tuning=lldb -o - | FileCheck %s @@ -28,13 +28,13 @@ // CHECK: ![[F]] = !DIFile(filename: {{.*}}debug-info-moduleimport.m // RUN: %clang_cc1 -debug-info-kind=limited -fmodules -fimplicit-module-maps \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -isysroot /tmp/.. -I %t \ +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -isysroot /tmp/.. -I %t \ // RUN: -emit-llvm -o - | FileCheck %s --check-prefix=NO-SKEL-CHECK // NO-SKEL-CHECK: distinct !DICompileUnit // NO-SKEL-CHECK-NOT: distinct !DICompileUnit // RUN: %clang_cc1 -debug-info-kind=limited -fmodules -fimplicit-module-maps \ -// RUN: -fmodules-cache-path=%t -fdebug-prefix-map=%t=/MODULE-CACHE \ +// RUN: -fmodules-cache-path=%t/cache -fdebug-prefix-map=%t/cache=/MODULE-CACHE \ // RUN: -fdebug-prefix-map=%S=/SRCDIR \ // RUN: -fmodule-format=obj -dwarf-ext-refs \ // RUN: %s -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm -o - \ diff --git a/clang/test/Modules/global_index.m b/clang/test/Modules/global_index.m index 521914702d1b1..3857e10c2e922 100644 --- a/clang/test/Modules/global_index.m +++ b/clang/test/Modules/global_index.m @@ -1,12 +1,12 @@ // RUN: rm -rf %t // Run without global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify -// RUN: ls %t|not grep modules.idx +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify +// RUN: ls %t/cache | not grep modules.idx // Run and create the global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -// RUN: ls %t|grep modules.idx +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify +// RUN: ls %t/cache | grep modules.idx // Run and use the global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s // expected-no-diagnostics @import DependsOnModule; diff --git a/clang/test/Modules/load-after-failure.m b/clang/test/Modules/load-after-failure.m index 73ed0e79708b9..c0425365bb9d6 100644 --- a/clang/test/Modules/load-after-failure.m +++ b/clang/test/Modules/load-after-failure.m @@ -11,9 +11,9 @@ // RUN: echo 'module C { header "C.h" }' >> %t/module.modulemap // RUN: echo 'module D { header "D.h" }' >> %t/module.modulemap -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t %s -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %t %s -verify // RUN: echo " " >> %t/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t %s -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %t %s -verify // expected-no-diagnostics diff --git a/clang/test/Modules/module-debuginfo-compdir.m b/clang/test/Modules/module-debuginfo-compdir.m index fced242624f75..885a896d45918 100644 --- a/clang/test/Modules/module-debuginfo-compdir.m +++ b/clang/test/Modules/module-debuginfo-compdir.m @@ -5,7 +5,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ // RUN: -fdebug-compilation-dir=/OVERRIDE \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/module-debuginfo-prefix.m b/clang/test/Modules/module-debuginfo-prefix.m index 0245ff1cabf7d..7270160417dd9 100644 --- a/clang/test/Modules/module-debuginfo-prefix.m +++ b/clang/test/Modules/module-debuginfo-prefix.m @@ -5,7 +5,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ // RUN: -fdebug-prefix-map=%S/Inputs=/OVERRIDE \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/relative-module-cache.c b/clang/test/Modules/relative-module-cache.c new file mode 100644 index 0000000000000..66136639af42a --- /dev/null +++ b/clang/test/Modules/relative-module-cache.c @@ -0,0 +1,60 @@ +// This test checks that module cache populated using different spellings of the +// same underlying directory works consistently (specifically the IMPORT records.) + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: mkdir %t/tmp + +// Module cache path absolute. +// RUN: cd %t/tmp && %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only -I %t %t/tu1.c -Rmodule-build -Rmodule-import -verify -fmodules-cache-path=%t/cache +//--- tu1.c +#include "b.h" // expected-remark{{building module 'b'}} \ + // expected-remark{{finished building module 'b'}} \ + // expected-remark{{importing module 'b'}} \ + // expected-remark{{importing module 'a' into 'b'}} + +// Module cache path relative to CWD. +// RUN: cd %t && %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only -I %t %t/tu2.c -Rmodule-build -Rmodule-import -verify -fmodules-cache-path=cache +//--- tu2.c +#include "c.h" // expected-remark{{building module 'c'}} \ + // expected-remark{{finished building module 'c'}} \ + // expected-remark{{importing module 'c'}} \ + // expected-remark{{importing module 'b' into 'c'}} \ + // expected-remark{{importing module 'a' into 'b'}} + +// Module cache path relative to -working-directory. +// RUN: cd %t/tmp && %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only -I %t %t/tu3.c -Rmodule-build -Rmodule-import -verify -fmodules-cache-path=cache -working-directory %t +//--- tu3.c +#include "d.h" // expected-remark{{building module 'd'}} \ + // expected-remark{{finished building module 'd'}} \ + // expected-remark{{importing module 'd'}} \ + // expected-remark{{importing module 'c' into 'd'}} \ + // expected-remark{{importing module 'b' into 'c'}} \ + // expected-remark{{importing module 'a' into 'b'}} + +// Module cache path absolute again. +// RUN: cd %t/tmp && %clang_cc1 -fmodules -fimplicit-module-maps -fsyntax-only -I %t %t/tu4.c -Rmodule-build -Rmodule-import -verify -fmodules-cache-path=%t/cache +//--- tu4.c +#include "e.h" // expected-remark{{building module 'e'}} \ + // expected-remark{{finished building module 'e'}} \ + // expected-remark{{importing module 'e'}} \ + // expected-remark{{importing module 'd' into 'e'}} \ + // expected-remark{{importing module 'c' into 'd'}} \ + // expected-remark{{importing module 'b' into 'c'}} \ + // expected-remark{{importing module 'a' into 'b'}} + +//--- module.modulemap +module a { header "a.h" } +module b { header "b.h" } +module c { header "c.h" } +module d { header "d.h" } +module e { header "e.h" } +//--- a.h +//--- b.h +#include "a.h" +//--- c.h +#include "b.h" +//--- d.h +#include "c.h" +//--- e.h +#include "d.h" diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp index b76dcfec96063..79d48ca0a66ca 100644 --- a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp +++ b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp @@ -137,9 +137,9 @@ export module a; export int aa = 44; )cpp"); - auto ReadResult = - Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile, - SourceLocation(), ASTReader::ARR_None); + auto ReadResult = Clang.getASTReader()->ReadAST( + ModuleFileName::makeExplicit(BMIPath), serialization::MK_MainFile, + SourceLocation(), ASTReader::ARR_None); // We shall be able to detect the content change here. EXPECT_NE(ReadResult, ASTReader::Success);