https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/185765
>From c599af32a82eb924c88fb9114a0df082e48e1117 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 10 Mar 2026 13:17:39 -0700 Subject: [PATCH 1/4] [clang][modules] Provide normalized module cache path from `HeaderSearch` --- .../include/clang/Frontend/CompilerInstance.h | 5 --- clang/include/clang/Lex/HeaderSearch.h | 25 +++++++++---- clang/include/clang/Serialization/ASTReader.h | 9 ++--- .../DependencyScannerImpl.cpp | 3 +- clang/lib/Frontend/ASTUnit.cpp | 23 ++++++------ clang/lib/Frontend/CompilerInstance.cpp | 22 ++--------- clang/lib/Frontend/FrontendAction.cpp | 5 ++- clang/lib/Frontend/FrontendActions.cpp | 6 ++- clang/lib/Lex/HeaderSearch.cpp | 28 ++++++++++++++ clang/lib/Serialization/ASTReader.cpp | 37 ++++++++++--------- clang/lib/Serialization/ASTWriter.cpp | 10 ++--- 11 files changed, 95 insertions(+), 78 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 266e0826b38f4..f7a0d1064110a 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -746,11 +746,6 @@ class CompilerInstance : public ModuleLoader { GetDependencyDirectives = std::move(Getter); } - std::string getSpecificModuleCachePath(StringRef ContextHash); - std::string getSpecificModuleCachePath() { - return getSpecificModuleCachePath(getInvocation().computeContextHash()); - } - /// Create the AST context. void createASTContext(); diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index d7000da682c6e..2bb78fbf608f3 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -282,6 +282,10 @@ class HeaderSearch { /// The specific module cache path containing ContextHash (unless suppressed). std::string SpecificModuleCachePath; + /// The length of the normalized module cache path at the start of \c + /// SpecificModuleCachePath. + size_t NormalizedModuleCachePathLen = 0; + /// All of the preprocessor-specific data about files that are /// included, indexed by the FileEntry's UID. mutable std::vector<HeaderFileInfo> FileInfo; @@ -467,20 +471,20 @@ class HeaderSearch { return {}; } - /// Set the context hash to use for module cache paths. - void setContextHash(StringRef Hash) { ContextHash = std::string(Hash); } + /// Initialize the module cache path. + void initializeModuleCachePath(std::string ContextHash); - /// Set the module cache path with the context hash (unless suppressed). - void setSpecificModuleCachePath(StringRef Path) { - SpecificModuleCachePath = std::string(Path); + /// Retrieve the module cache path with the context hash (unless suppressed). + StringRef getSpecificModuleCachePath() const { + return SpecificModuleCachePath; } /// Retrieve the context hash. StringRef getContextHash() const { return ContextHash; } - /// Retrieve the module cache path with the context hash (unless suppressed). - StringRef getSpecificModuleCachePath() const { - return SpecificModuleCachePath; + /// Retrieve the normalized module cache path. + StringRef getNormalizedModuleCachePath() const { + return getSpecificModuleCachePath().substr(0, NormalizedModuleCachePathLen); } /// Forget everything we know about headers so far. @@ -1042,6 +1046,11 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS, void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, SmallVectorImpl<char> &NormalizedPath); +std::string createSpecificModuleCachePath(FileManager &FileMgr, + StringRef ModuleCachePath, + bool DisableModuleHash, + std::string ContextHash); + } // namespace clang #endif // LLVM_CLANG_LEX_HEADERSEARCH_H diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index f254459ce933d..14f7d8a69a1b3 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -185,8 +185,7 @@ class ASTReaderListener { /// otherwise. virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, - bool Complain) { + StringRef ContextHash, bool Complain) { return false; } @@ -304,8 +303,7 @@ class ChainedASTReaderListener : public ASTReaderListener { bool Complain) override; bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override; bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, StringRef ModuleFilename, bool ReadMacros, @@ -349,8 +347,7 @@ class PCHValidator : public ASTReaderListener { bool Complain, std::string &SuggestedPredefines) override; bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override; void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override; }; diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 0e345af8817ae..a38ee690d42de 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -150,8 +150,7 @@ class PrebuiltModuleListener : public ASTReaderListener { /// Check the header search options for a given module when considering /// if the module comes from stable directories. bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override { auto PrebuiltMapEntry = PrebuiltModulesASTMap.try_emplace(CurrentFile); diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 9fcaf1806fcb1..1d249ebaa1492 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -505,7 +505,7 @@ namespace { /// a Preprocessor. class ASTInfoCollector : public ASTReaderListener { HeaderSearchOptions &HSOpts; - std::string &SpecificModuleCachePath; + std::string &ContextHash; PreprocessorOptions &PPOpts; LangOptions &LangOpts; CodeGenOptions &CodeGenOpts; @@ -513,14 +513,13 @@ class ASTInfoCollector : public ASTReaderListener { uint32_t &Counter; public: - ASTInfoCollector(HeaderSearchOptions &HSOpts, - std::string &SpecificModuleCachePath, + ASTInfoCollector(HeaderSearchOptions &HSOpts, std::string &ContextHash, PreprocessorOptions &PPOpts, LangOptions &LangOpts, CodeGenOptions &CodeGenOpts, TargetOptions &TargetOpts, uint32_t &Counter) - : HSOpts(HSOpts), SpecificModuleCachePath(SpecificModuleCachePath), - PPOpts(PPOpts), LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), - TargetOpts(TargetOpts), Counter(Counter) {} + : HSOpts(HSOpts), ContextHash(ContextHash), PPOpts(PPOpts), + LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), + Counter(Counter) {} bool ReadLanguageOptions(const LangOptions &NewLangOpts, StringRef ModuleFilename, bool Complain, @@ -538,10 +537,10 @@ class ASTInfoCollector : public ASTReaderListener { bool ReadHeaderSearchOptions(const HeaderSearchOptions &NewHSOpts, StringRef ModuleFilename, - StringRef NewSpecificModuleCachePath, + StringRef NewContextHash, bool Complain) override { HSOpts = NewHSOpts; - SpecificModuleCachePath = NewSpecificModuleCachePath; + ContextHash = NewContextHash; return false; } @@ -733,13 +732,13 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->ModCache = createCrossProcessModuleCache(); // Gather info for preprocessor construction later on. - std::string SpecificModuleCachePath; + std::string ContextHash; unsigned Counter = 0; // Using a temporary FileManager since the AST file might specify custom // HeaderSearchOptions::VFSOverlayFiles that affect the underlying VFS. FileManager TmpFileMgr(FileSystemOpts, VFS); - ASTInfoCollector Collector(*AST->HSOpts, SpecificModuleCachePath, - *AST->PPOpts, *AST->LangOpts, *AST->CodeGenOpts, + ASTInfoCollector Collector(*AST->HSOpts, ContextHash, *AST->PPOpts, + *AST->LangOpts, *AST->CodeGenOpts, *AST->TargetOpts, Counter); if (ASTReader::readASTFileControlBlock( Filename, TmpFileMgr, *AST->ModCache, PCHContainerRdr, @@ -763,7 +762,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->getHeaderSearchOpts(), AST->getSourceManager(), AST->getDiagnostics(), AST->getLangOpts(), /*Target=*/nullptr); - AST->HeaderInfo->setSpecificModuleCachePath(SpecificModuleCachePath); + AST->HeaderInfo->initializeModuleCachePath(std::move(ContextHash)); AST->PP = std::make_shared<Preprocessor>( *AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts, diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 60914d9b2cbc7..4c7a5a66254a5 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -486,12 +486,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP); - if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) { - std::string ContextHash = getInvocation().computeContextHash(); - PP->getHeaderSearchInfo().setContextHash(ContextHash); - PP->getHeaderSearchInfo().setSpecificModuleCachePath( - getSpecificModuleCachePath(ContextHash)); - } + if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) + PP->getHeaderSearchInfo().initializeModuleCachePath( + getInvocation().computeContextHash()); // Handle generating dependencies, if requested. const DependencyOutputOptions &DepOpts = getDependencyOutputOpts(); @@ -546,19 +543,6 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { PP->setDependencyDirectivesGetter(*GetDependencyDirectives); } -std::string -CompilerInstance::getSpecificModuleCachePath(StringRef ContextHash) { - assert(FileMgr && "Specific module cache path requires a FileManager"); - - // Set up the module path, including the hash for the module-creation options. - SmallString<256> SpecificModuleCache; - normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath, - SpecificModuleCache); - if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash) - llvm::sys::path::append(SpecificModuleCache, ContextHash); - return std::string(SpecificModuleCache); -} - // ASTContext void CompilerInstance::createASTContext() { diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 73f092521546f..84ceb22208801 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1040,7 +1040,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, llvm::sys::path::native(PCHDir->getName(), DirNative); bool Found = false; llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem(); - std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath(); + std::string SpecificModuleCachePath = createSpecificModuleCachePath( + CI.getFileManager(), CI.getHeaderSearchOpts().ModuleCachePath, + CI.getHeaderSearchOpts().DisableModuleHash, + CI.getInvocation().computeContextHash()); for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd; Dir != DirEnd && !EC; Dir.increment(EC)) { diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 492f7b1742bee..15d19dae966ac 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -716,8 +716,12 @@ namespace { bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ContextHash, bool Complain) override { + SmallString<128> SpecificModuleCachePath(HSOpts.ModuleCachePath); + if (!SpecificModuleCachePath.empty() && !HSOpts.DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + Out.indent(2) << "Header search options:\n"; Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n"; Out.indent(4) << "Resource dir [ -resource-dir=]: '" << HSOpts.ResourceDir << "'\n"; diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 5aee19cd14c51..de26b999683d1 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -2471,3 +2471,31 @@ void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, llvm::sys::path::remove_dots(NormalizedPath); } } + +static std::string createSpecificModuleCachePathImpl( + FileManager &FileMgr, StringRef ModuleCachePath, bool DisableModuleHash, + std::string ContextHash, size_t &NormalizedModuleCachePathLen) { + SmallString<256> SpecificModuleCachePath; + normalizeModuleCachePath(FileMgr, ModuleCachePath, SpecificModuleCachePath); + NormalizedModuleCachePathLen = SpecificModuleCachePath.size(); + if (!SpecificModuleCachePath.empty() && !DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + return std::string(SpecificModuleCachePath); +} + +void HeaderSearch::initializeModuleCachePath(std::string NewContextHash) { + ContextHash = std::move(NewContextHash); + SpecificModuleCachePath = createSpecificModuleCachePathImpl( + FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash, ContextHash, + NormalizedModuleCachePathLen); +} + +std::string clang::createSpecificModuleCachePath(FileManager &FileMgr, + StringRef ModuleCachePath, + bool DisableModuleHash, + std::string ContextHash) { + size_t NormalizedModuleCachePathLen; + return createSpecificModuleCachePathImpl( + FileMgr, ModuleCachePath, DisableModuleHash, std::move(ContextHash), + NormalizedModuleCachePathLen); +} diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index b82ae971bc84d..812f27989c129 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -208,11 +208,11 @@ ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts, bool ChainedASTReaderListener::ReadHeaderSearchOptions( const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, bool Complain) { - return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain) || - Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain); + StringRef ContextHash, bool Complain) { + return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain) || + Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain); } bool ChainedASTReaderListener::ReadPreprocessorOptions( @@ -961,11 +961,15 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions( /// \param Diags If non-null, produce diagnostics for any mismatches incurred. /// \returns true when the module cache paths differ. static bool checkModuleCachePath( - llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath, + llvm::vfs::FileSystem &VFS, StringRef ContextHash, StringRef ExistingSpecificModuleCachePath, StringRef ASTFilename, DiagnosticsEngine *Diags, const LangOptions &LangOpts, const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts, const HeaderSearchOptions &ASTFileHSOpts) { + SmallString<128> SpecificModuleCachePath(ASTFileHSOpts.ModuleCachePath); + if (!SpecificModuleCachePath.empty() && !ASTFileHSOpts.DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath || SpecificModuleCachePath == ExistingSpecificModuleCachePath) return false; @@ -991,11 +995,11 @@ static bool checkModuleCachePath( bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ASTFilename, - StringRef SpecificModuleCachePath, + StringRef ContextHash, bool Complain) { const HeaderSearch &HeaderSearchInfo = PP.getHeaderSearchInfo(); return checkModuleCachePath( - Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath, + Reader.getFileManager().getVirtualFileSystem(), ContextHash, HeaderSearchInfo.getSpecificModuleCachePath(), ASTFilename, Complain ? &Reader.Diags : nullptr, PP.getLangOpts(), PP.getPreprocessorOpts(), HeaderSearchInfo.getHeaderSearchOpts(), HSOpts); @@ -5877,13 +5881,12 @@ namespace { } bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ASTFilename, - StringRef SpecificModuleCachePath, + StringRef ASTFilename, StringRef ContextHash, bool Complain) override { - return checkModuleCachePath( - FileMgr.getVirtualFileSystem(), SpecificModuleCachePath, - ExistingSpecificModuleCachePath, ASTFilename, nullptr, - ExistingLangOpts, ExistingPPOpts, ExistingHSOpts, HSOpts); + return checkModuleCachePath(FileMgr.getVirtualFileSystem(), ContextHash, + ExistingSpecificModuleCachePath, ASTFilename, + nullptr, ExistingLangOpts, ExistingPPOpts, + ExistingHSOpts, HSOpts); } bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, @@ -6705,10 +6708,10 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record, HSOpts.UseStandardSystemIncludes = Record[Idx++]; HSOpts.UseStandardCXXIncludes = Record[Idx++]; HSOpts.UseLibcxx = Record[Idx++]; - std::string SpecificModuleCachePath = ReadString(Record, Idx); + std::string ContextHash = ReadString(Record, Idx); - return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain); + return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain); } bool ASTReader::ParseHeaderSearchPaths(const RecordData &Record, bool Complain, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index ec718169550aa..2744d70b89aac 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1694,9 +1694,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); - SmallString<256> HSOpts_ModuleCachePath; - normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath, - HSOpts_ModuleCachePath); + StringRef HSOpts_ModuleCachePath = + PP.getHeaderSearchInfo().getNormalizedModuleCachePath(); AddString(HSOpts.Sysroot, Record); AddString(HSOpts.ResourceDir, Record); @@ -1710,10 +1709,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Record.push_back(HSOpts.UseStandardSystemIncludes); Record.push_back(HSOpts.UseStandardCXXIncludes); Record.push_back(HSOpts.UseLibcxx); - // Write out the specific module cache path that contains the module files. - // FIXME: We already wrote out the normalized cache path. Just write the - // context hash (unless suppressed). - AddString(PP.getHeaderSearchInfo().getSpecificModuleCachePath(), Record); + AddString(PP.getHeaderSearchInfo().getContextHash(), Record); Stream.EmitRecord(HEADER_SEARCH_OPTIONS, Record); // Preprocessor options. >From 5a48f462ab14af12783bf0f0c575127f3e5d4e84 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 10 Mar 2026 13:26:58 -0700 Subject: [PATCH 2/4] Bump AST file version --- clang/include/clang/Serialization/ASTBitCodes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 752e7fd288aa6..5db0b08f877ce 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 = 35; +const unsigned VERSION_MAJOR = 36; /// AST file minor version number supported by this version of /// Clang. >From 164206bdc541f465acdf7afeb74aceb68687f1c1 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 9 Mar 2026 14:38:24 -0700 Subject: [PATCH 3/4] [clang][modules] Stop uniquing implicit modules via `FileEntry` --- clang-tools-extra/clangd/ModulesBuilder.cpp | 4 +- clang/include/clang/Basic/Module.h | 114 ++++++++++ .../include/clang/Frontend/CompilerInstance.h | 3 +- clang/include/clang/Lex/HeaderSearch.h | 20 +- .../include/clang/Serialization/ASTBitCodes.h | 2 +- clang/include/clang/Serialization/ASTReader.h | 4 +- .../include/clang/Serialization/ModuleFile.h | 3 + .../clang/Serialization/ModuleManager.h | 44 ++-- clang/lib/Basic/Module.cpp | 17 ++ .../DependencyScannerImpl.cpp | 2 +- clang/lib/Frontend/ASTUnit.cpp | 5 +- clang/lib/Frontend/ChainedIncludesSource.cpp | 3 +- clang/lib/Frontend/CompilerInstance.cpp | 43 ++-- clang/lib/Frontend/FrontendAction.cpp | 2 +- clang/lib/Frontend/FrontendActions.cpp | 14 +- clang/lib/Lex/HeaderSearch.cpp | 37 ++-- clang/lib/Serialization/ASTReader.cpp | 43 ++-- clang/lib/Serialization/ASTWriter.cpp | 12 +- clang/lib/Serialization/GlobalModuleIndex.cpp | 1 + clang/lib/Serialization/ModuleManager.cpp | 209 ++++++++++-------- clang/test/Modules/DebugInfoNamespace.cpp | 2 +- clang/test/Modules/ExtDebugInfo.cpp | 2 +- clang/test/Modules/ExtDebugInfo.m | 2 +- clang/test/Modules/ModuleDebugInfo.cpp | 2 +- clang/test/Modules/ModuleDebugInfo.m | 2 +- clang/test/Modules/ModuleModuleDebugInfo.cpp | 2 +- clang/test/Modules/cxx20-hu-04.cpp | 2 +- clang/test/Modules/debug-info-moduleimport.m | 8 +- clang/test/Modules/global_index.m | 10 +- clang/test/Modules/load-after-failure.m | 4 +- clang/test/Modules/module-debuginfo-compdir.m | 2 +- clang/test/Modules/module-debuginfo-prefix.m | 2 +- clang/test/Modules/relocatable-modules.cpp | 8 +- .../Serialization/ForceCheckFileInputTest.cpp | 6 +- 34 files changed, 395 insertions(+), 241 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 524ec620c4076..b5a0127d48b7e 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::make_explicit(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..c512249e5e945 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -54,6 +54,101 @@ class TargetInfo; /// Describes the name of a module. using ModuleId = SmallVector<std::pair<std::string, SourceLocation>, 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 PathSuffix; + + friend class serialization::ModuleManager; + friend class ModuleFileName; + friend llvm::DenseMapInfo<ModuleFileKey>; + + ModuleFileKey(const void *Ptr) : Ptr(Ptr) {} + + ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {} + + ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix) + : Ptr(ModuleCacheDir), PathSuffix(PathSuffix) {} + +public: + bool operator==(const ModuleFileKey &Other) const { + return Ptr == Other.Ptr && PathSuffix == Other.PathSuffix; + } + + 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; + std::optional<uint32_t> Separator; + +public: + /// Creates an empty module file name. + ModuleFileName() = default; + + /// Creates a file name for an explicit module. + static ModuleFileName make_explicit(std::string Name) { + ModuleFileName File; + File.Path = std::move(Name); + return File; + } + + /// Creates a file name for an explicit module. + static ModuleFileName make_explicit(StringRef Name) { + return make_explicit(Name.str()); + } + + /// Creates a file name for an implicit module. + static ModuleFileName make_implicit(std::string Name, uint32_t Separator) { + ModuleFileName File; + File.Path = std::move(Name); + File.Separator = Separator; + return File; + } + + /// Creates a file name for an implicit module. + static ModuleFileName make_implicit(StringRef Name, uint32_t Separator) { + return make_implicit(Name.str(), Separator); + } + + /// 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. + std::optional<ModuleFileKey> makeKey(FileManager &FileMgr) const; +}; + /// The signature of a module, which is a hash of the AST content. struct ASTFileSignature : std::array<uint8_t, 20> { using BaseT = std::array<uint8_t, 20>; @@ -926,4 +1021,23 @@ class VisibleModuleSet { } // namespace clang +template <> struct llvm::DenseMapInfo<clang::ModuleFileKey> { + static clang::ModuleFileKey getEmptyKey() { + return DenseMapInfo<const void *>::getEmptyKey(); + } + + static clang::ModuleFileKey getTombstoneKey() { + return DenseMapInfo<const void *>::getTombstoneKey(); + } + + static unsigned getHashValue(const clang::ModuleFileKey &Val) { + return hash_combine(Val.Ptr, Val.PathSuffix); + } + + 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 2bb78fbf608f3..fd52a0ad5b007 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -661,7 +661,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. @@ -673,8 +673,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. @@ -683,7 +683,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. @@ -695,8 +695,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. /// @@ -812,13 +812,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<ImportedModule> &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..4915ad0634fcd 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -159,6 +159,9 @@ class ModuleFile { /// The file name of the module file. std::string FileName; + /// All keys ModuleManager used for the module file. + llvm::DenseSet<ModuleFileKey> Keys; + /// 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<ModuleFile *, 2> Roots; - /// All loaded modules, indexed by name. - llvm::DenseMap<const FileEntry *, ModuleFile *> Modules; + /// All loaded modules. + llvm::DenseMap<ModuleFileKey, ModuleFile *> 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<llvm::MemoryBuffer> 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<bool(ModuleFile &M)> Visitor, llvm::SmallPtrSetImpl<ModuleFile *> *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..bfc29e32c3672 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -33,6 +33,23 @@ using namespace clang; +std::optional<ModuleFileKey> +ModuleFileName::makeKey(FileManager &FileMgr) const { + if (Separator) { + StringRef ModuleCachePath = StringRef(Path).substr(0, *Separator); + StringRef ModuleFilePathSuffix = StringRef(Path).substr(*Separator); + if (auto ModuleCache = FileMgr.getOptionalDirectoryRef( + ModuleCachePath, /*CacheFailure=*/false)) + return ModuleFileKey(*ModuleCache, ModuleFilePathSuffix); + } 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 a38ee690d42de..f982e2328537c 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -607,7 +607,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..32769e14a3fcc 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -835,8 +835,9 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->HSOpts->ForceCheckCXX20ModulesInputFiles = HSOpts.ForceCheckCXX20ModulesInputFiles; - switch (AST->Reader->ReadAST(Filename, serialization::MK_MainFile, - SourceLocation(), ASTReader::ARR_None)) { + switch (AST->Reader->ReadAST(ModuleFileName::make_explicit(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..1d853da65f1ca 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::make_explicit(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 4c7a5a66254a5..6aa24d380031e 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -655,11 +655,10 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource( ASTReader::ListenerScope ReadModuleNamesListener(*Reader, std::move(Listener)); - switch (Reader->ReadAST(Path, + switch (Reader->ReadAST(ModuleFileName::make_explicit(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. @@ -1393,7 +1392,8 @@ std::unique_ptr<CompilerInstance> 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(); @@ -1435,7 +1435,7 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, - StringRef ModuleFileName) { + ModuleFileName ModuleFileName) { { auto Instance = ImportingInstance.cloneForModuleCompile( ModuleNameLoc, Module, ModuleFileName); @@ -1471,9 +1471,11 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, /// multiple instances will compete to create the same module. On timeout, /// deletes the lock file in order to avoid deadlock from crashing processes or /// bugs in the lock file manager. -static bool compileModuleAndReadASTBehindLock( - CompilerInstance &ImportingInstance, SourceLocation ImportLoc, - SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) { +static bool +compileModuleAndReadASTBehindLock(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, + SourceLocation ModuleNameLoc, Module *Module, + ModuleFileName ModuleFileName) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); Diags.Report(ModuleNameLoc, diag::remark_module_lock) @@ -1543,7 +1545,8 @@ static bool compileModuleAndReadASTBehindLock( static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, - Module *Module, StringRef ModuleFileName) { + Module *Module, + ModuleFileName ModuleFileName) { return ImportingInstance.getInvocation() .getFrontendOpts() .BuildingImplicitModuleUsesLock @@ -1682,11 +1685,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. @@ -1743,7 +1746,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<std::string, std::string, std::less<>> &BuiltModules, HeaderSearch &HS) { assert(ModuleFilename.empty() && "Already has a module source?"); @@ -1752,7 +1755,7 @@ static ModuleSource selectModuleSource( // via a module build pragma. auto BuiltModuleIt = BuiltModules.find(ModuleName); if (BuiltModuleIt != BuiltModules.end()) { - ModuleFilename = BuiltModuleIt->second; + ModuleFilename = ModuleFileName::make_explicit(BuiltModuleIt->second); return MS_ModuleBuildPragma; } @@ -1760,9 +1763,9 @@ static ModuleSource selectModuleSource( const HeaderSearchOptions &HSOpts = HS.getHeaderSearchOpts(); if (!HSOpts.PrebuiltModuleFiles.empty() || !HSOpts.PrebuiltModulePaths.empty()) { - ModuleFilename = HS.getPrebuiltModuleFileName(ModuleName); + ModuleFilename = ModuleFileName(HS.getPrebuiltModuleFileName(ModuleName)); if (HSOpts.EnablePrebuiltImplicitModules && ModuleFilename.empty()) - ModuleFilename = HS.getPrebuiltImplicitModuleFileName(M); + ModuleFilename = ModuleFileName(HS.getPrebuiltImplicitModuleFileName(M)); if (!ModuleFilename.empty()) return MS_PrebuiltModulePath; } @@ -1792,7 +1795,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(); @@ -1821,8 +1824,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); @@ -2042,7 +2045,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..b82296c9c52f9 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -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::make_explicit(ModuleFile), Loaded)) return false; if (Loaded && Loaded->StandardCXXModule) diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 15d19dae966ac..e371c68162b31 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::make_explicit(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..8d6bed45f5e45 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::make_explicit(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::make_explicit(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::make_explicit(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 Separator = CachePath.size(); if (HSOpts.DisableModuleHash) { llvm::sys::path::append(Result, ModuleName + ".pcm"); } else { + llvm::sys::path::append(Result, ContextHash); + // Construct the name <ModuleName>-<hash of ModuleMapPath>.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::make_implicit(Result, Separator); } Module *HeaderSearch::lookupModule(StringRef ModuleName, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 812f27989c129..637cbc02c085d 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3475,8 +3475,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, off_t StoredSize = 0; time_t StoredModTime = 0; + unsigned ModuleCacheLen = 0; ASTFileSignature StoredSignature; - std::string ImportedFile; + ModuleFileName ImportedFile; std::string StoredFile; bool IgnoreImportedByNote = false; @@ -3498,17 +3499,23 @@ ASTReader::ReadControlBlock(ModuleFile &F, if (!IsImportingStdCXXModule) { StoredSize = (off_t)Record[Idx++]; StoredModTime = (time_t)Record[Idx++]; + ModuleCacheLen = (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); + StoredFile = ReadStringBlob(Record, Idx, Blob); if (ImportedFile.empty()) { - ImportedFile = StoredFile; + if (ImportedKind == MK_ImplicitModule) { + assert(ModuleCacheLen != 0); + ImportedFile = + ModuleFileName::make_implicit(StoredFile, ModuleCacheLen); + } else { + assert(ModuleCacheLen == 0); + ImportedFile = ModuleFileName::make_explicit(StoredFile); + } } else if (!getDiags().isIgnored( diag::warn_module_file_mapping_mismatch, CurrentImportLoc)) { @@ -4911,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) { @@ -5187,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<ImportedModule> &Loaded, - off_t ExpectedSize, time_t ExpectedModTime, - ASTFileSignature ExpectedSignature, - unsigned ClientLoadCapabilities) { +ASTReader::ASTReadResult ASTReader::ReadASTCore( + ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, + ModuleFile *ImportedBy, SmallVectorImpl<ImportedModule> &Loaded, + off_t ExpectedSize, time_t ExpectedModTime, + ASTFileSignature ExpectedSignature, unsigned ClientLoadCapabilities) { ModuleFile *M; std::string ErrorStr; ModuleManager::AddModuleResult AddResult @@ -6141,14 +6145,13 @@ bool ASTReader::readASTFileControlBlock( continue; } - // Skip Size and ModTime. - Idx += 1 + 1; + // Skip Size, ModTime and ModuleCacheLen. + Idx += 1 + 1 + 1; // Skip signature. Blob = Blob.substr(ASTFileSignature::size); - StringRef FilenameStr = ReadStringBlob(Record, Idx, Blob); - auto Filename = ResolveImportedPath(PathBuf, FilenameStr, ModuleDir); - Listener.visitImport(ModuleName, *Filename); + StringRef Filename = ReadStringBlob(Record, Idx, Blob); + Listener.visitImport(ModuleName, Filename); break; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 2744d70b89aac..eb310f30078b0 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)); // Module cache 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,7 +1603,15 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { llvm::append_range(Blob, M.Signature); - AddPathBlob(M.FileName, Record, Blob); + StringRef NormalizedModuleCache = + PP.getHeaderSearchInfo().getNormalizedModuleCachePath(); + unsigned ModuleCacheLen = 0; + if (M.Kind == MK_ImplicitModule && + StringRef(M.FileName).starts_with(NormalizedModuleCache)) + ModuleCacheLen = NormalizedModuleCache.size(); + Record.push_back(ModuleCacheLen); + + AddStringBlob(M.FileName, Record, Blob); } Stream.EmitRecordWithBlob(AbbrevCode, Record, Blob); diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index 2246a3ac0a57e..f173441c6e81b 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++]; // ModuleCacheLen // 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..bfb42a194a604 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<ModuleFileKey> Key = Name.makeKey(FileMgr); + return Key ? lookup(*Key) : nullptr; +} + +ModuleFile *ModuleManager::lookup(ModuleFileKey Key) const { + return Modules.lookup(Key); } std::unique_ptr<llvm::MemoryBuffer> @@ -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,84 @@ 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) { - ErrorStr = "module file not found"; + std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr); + if (!FileKey) return Missing; + + ModuleFile *ModuleEntry = lookup(*FileKey); + if (ModuleEntry) + ModuleEntry->Keys.insert(*FileKey); + + // TODO: Remove this. + if (!ModuleEntry) { + // Try looking up with the plain file name. + auto FileName2 = ModuleFileName::make_explicit(FileName); + std::optional<ModuleFileKey> FileKey2 = FileName2.makeKey(FileMgr); + if (!FileKey2) + return Missing; + ModuleEntry = lookup(*FileKey2); + if (ModuleEntry) + ModuleEntry->Keys.insert(*FileKey2); } - // 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; - }; + if (ModuleEntry) { + // Check file properties. + if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, + ErrorStr)) + return OutOfDate; - // 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; + // Check the stored signature. + if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) + return OutOfDate; - Module = ModuleEntry; - updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); - return AlreadyLoaded; - } + Module = ModuleEntry; + updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); + return AlreadyLoaded; } - // Allocate a new module. - auto NewModule = std::make_unique<ModuleFile>(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<llvm::MemoryBuffer> NewFileBuffer = nullptr; if (std::unique_ptr<llvm::MemoryBuffer> 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,11 +230,26 @@ 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<ModuleFile>(Type, *Entry, Generation); + NewModule->Index = Chain.size(); + NewModule->FileName = FileName.str(); + NewModule->ImportLoc = ImportLoc; + NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; + NewModule->Buffer = ModuleBuffer; // Initialize the stream. NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); + NewModule->Keys.insert(*FileKey); // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. @@ -233,8 +261,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 +312,11 @@ 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)); + for (ModuleFileKey Key : victim->Keys) + Modules.erase(Key); + } Chain.erase(Chain.begin() + (First - begin()), Chain.end()); } @@ -439,29 +475,6 @@ void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> 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/cxx20-hu-04.cpp b/clang/test/Modules/cxx20-hu-04.cpp index 8546b33572dc8..075b44289f100 100644 --- a/clang/test/Modules/cxx20-hu-04.cpp +++ b/clang/test/Modules/cxx20-hu-04.cpp @@ -102,4 +102,4 @@ int success(int x) { } // CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-02.h' from 'hu-02.pcm' -// CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-01.h' into '.{{/|\\\\?}}hu-02.h' from '[[TDIR]]{{[/\\]}}hu-01.pcm' +// CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-01.h' into '.{{/|\\\\?}}hu-02.h' from 'hu-01.pcm' 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/relocatable-modules.cpp b/clang/test/Modules/relocatable-modules.cpp index c8d1e6d455666..e047a79c17d3b 100644 --- a/clang/test/Modules/relocatable-modules.cpp +++ b/clang/test/Modules/relocatable-modules.cpp @@ -13,11 +13,9 @@ // RUN: -Wno-experimental-header-units -fmodule-file=hu-01.pcm -o hu-02-rel.pcm \ // RUN: -fmodule-file-home-is-cwd -// RUN: %clang -module-file-info hu-02-abs.pcm | FileCheck %s --check-prefix=IMPORT-ABS -DPREFIX=%t -// IMPORT-ABS: Imports module 'hu-01': [[PREFIX]]{{/|\\}}hu-01.pcm - -// RUN: %clang -module-file-info hu-02-rel.pcm | FileCheck %s --check-prefix=IMPORT-REL -// IMPORT-REL: Imports module 'hu-01': hu-01.pcm +// RUN: %clang -module-file-info hu-02-abs.pcm | FileCheck %s --check-prefix=IMPORT +// RUN: %clang -module-file-info hu-02-rel.pcm | FileCheck %s --check-prefix=IMPORT +// IMPORT: Imports module 'hu-01': hu-01.pcm // RUN: llvm-bcanalyzer --dump --disable-histogram %t/hu-02-abs.pcm \ // RUN: | FileCheck %s --check-prefix=INPUT-ABS -DPREFIX=%t diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp index b76dcfec96063..81f5d8d3a81b6 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::make_explicit(BMIPath), serialization::MK_MainFile, + SourceLocation(), ASTReader::ARR_None); // We shall be able to detect the content change here. EXPECT_NE(ReadResult, ASTReader::Success); >From e71185b2aec326876405d7d869c442976b738f8a Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 10 Mar 2026 15:46:04 -0700 Subject: [PATCH 4/4] Emit expected error details (test is X86-specific?) --- clang/lib/Serialization/ModuleManager.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index bfb42a194a604..3cf1bd75a7190 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -145,8 +145,10 @@ ModuleManager::AddModuleResult ModuleManager::addModule( } std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr); - if (!FileKey) + if (!FileKey) { + ErrorStr = "module file not found"; return Missing; + } ModuleFile *ModuleEntry = lookup(*FileKey); if (ModuleEntry) @@ -157,8 +159,10 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // Try looking up with the plain file name. auto FileName2 = ModuleFileName::make_explicit(FileName); std::optional<ModuleFileKey> FileKey2 = FileName2.makeKey(FileMgr); - if (!FileKey2) + if (!FileKey2) { + ErrorStr = "module file not found"; return Missing; + } ModuleEntry = lookup(*FileKey2); if (ModuleEntry) ModuleEntry->Keys.insert(*FileKey2); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
