https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/185994
>From 2a23310ecc3313413395ce7cfb6432f8a84ccbc7 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 13:56:04 -0700 Subject: [PATCH 1/7] [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 | 211 ++++++++++-------- 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, 398 insertions(+), 240 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 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<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 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..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 b4ad911545774..a6dacc99e7b83 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -659,11 +659,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. @@ -1398,7 +1397,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(); @@ -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<std::string, std::string, std::less<>> &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::make_explicit(BuiltModuleIt->second); return MS_ModuleBuildPragma; } @@ -1788,9 +1791,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; } @@ -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..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 7bcb4d7d89db1..625b6c8f4d7b4 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 515eaf8d1caed..8713b17c91386 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3477,8 +3477,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; @@ -3500,17 +3501,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)) { @@ -4913,7 +4920,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 +5197,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 @@ -6142,14 +6146,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 c4815e956e53f..9fe1a1afe7679 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..3cf1bd75a7190 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,88 @@ 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<ModuleFileKey> 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; + 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) { + ErrorStr = "module file not found"; + return Missing; } + ModuleEntry = lookup(*FileKey2); + if (ModuleEntry) + ModuleEntry->Keys.insert(*FileKey2); } - // 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; + if (ModuleEntry) { + // 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; + } // 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 +234,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 +265,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 +316,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 +479,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 f784fc41732d66571b965dfd976471fac052608e Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 14:13:08 -0700 Subject: [PATCH 2/7] Remove unnecessary constructor call --- clang/lib/Frontend/CompilerInstance.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index a6dacc99e7b83..8d0813d6c72d4 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1791,9 +1791,9 @@ static ModuleSource selectModuleSource( const HeaderSearchOptions &HSOpts = HS.getHeaderSearchOpts(); if (!HSOpts.PrebuiltModuleFiles.empty() || !HSOpts.PrebuiltModulePaths.empty()) { - ModuleFilename = ModuleFileName(HS.getPrebuiltModuleFileName(ModuleName)); + ModuleFilename = HS.getPrebuiltModuleFileName(ModuleName); if (HSOpts.EnablePrebuiltImplicitModules && ModuleFilename.empty()) - ModuleFilename = ModuleFileName(HS.getPrebuiltImplicitModuleFileName(M)); + ModuleFilename = HS.getPrebuiltImplicitModuleFileName(M); if (!ModuleFilename.empty()) return MS_PrebuiltModulePath; } >From 86ee9919673be5a650f94c13a3f33882b6899b35 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 15:46:42 -0700 Subject: [PATCH 3/7] Undo unnecessary lookup with explicit file name --- clang/lib/Serialization/ModuleManager.cpp | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 3cf1bd75a7190..d60350d3e0574 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -150,25 +150,9 @@ ModuleManager::AddModuleResult ModuleManager::addModule( return Missing; } - ModuleFile *ModuleEntry = lookup(*FileKey); - if (ModuleEntry) + if (ModuleFile *ModuleEntry = lookup(*FileKey)) { 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) { - ErrorStr = "module file not found"; - return Missing; - } - ModuleEntry = lookup(*FileKey2); - if (ModuleEntry) - ModuleEntry->Keys.insert(*FileKey2); - } - - if (ModuleEntry) { // Check file properties. if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, ErrorStr)) >From a109526c24ebff990aaeb9153a74da5a7ab0fc4d Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 15:53:53 -0700 Subject: [PATCH 4/7] Only store one key for module file --- clang/include/clang/Serialization/ModuleFile.h | 10 ++++++---- clang/lib/Serialization/ModuleManager.cpp | 9 +++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 4915ad0634fcd..7037bd18e3cc0 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 === @@ -159,8 +161,8 @@ 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 key ModuleManager used for the module file. + ModuleFileKey FileKey; /// The name of the module. std::string ModuleName; diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index d60350d3e0574..35c44eaedb80f 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -151,8 +151,6 @@ ModuleManager::AddModuleResult ModuleManager::addModule( } if (ModuleFile *ModuleEntry = lookup(*FileKey)) { - ModuleEntry->Keys.insert(*FileKey); - // Check file properties. if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, ErrorStr)) @@ -229,7 +227,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( } // Allocate a new module. - auto NewModule = std::make_unique<ModuleFile>(Type, *Entry, Generation); + auto NewModule = + std::make_unique<ModuleFile>(Type, *FileKey, *Entry, Generation); NewModule->Index = Chain.size(); NewModule->FileName = FileName.str(); NewModule->ImportLoc = ImportLoc; @@ -237,7 +236,6 @@ ModuleManager::AddModuleResult ModuleManager::addModule( 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. @@ -302,8 +300,7 @@ void ModuleManager::removeModules(ModuleIterator First) { // Delete the modules. for (ModuleIterator victim = First; victim != Last; ++victim) { Modules.erase(ModuleFileKey(victim->File)); - for (ModuleFileKey Key : victim->Keys) - Modules.erase(Key); + Modules.erase(victim->FileKey); } Chain.erase(Chain.begin() + (First - begin()), Chain.end()); >From ea1d91d1ce99d88b5a82b42fe4b6bbb3b35a6be9 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 15:59:54 -0700 Subject: [PATCH 5/7] Add back a comment --- clang/lib/Serialization/ModuleManager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 35c44eaedb80f..7d583b2a429a5 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -150,6 +150,7 @@ ModuleManager::AddModuleResult ModuleManager::addModule( return Missing; } + // Check whether we already loaded this module, before if (ModuleFile *ModuleEntry = lookup(*FileKey)) { // Check file properties. if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, >From 2cbc34336ff6aef8a32e7c3c290b6dd99f5434d5 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 15:33:01 -0700 Subject: [PATCH 6/7] [clang][modules] Remove `Module::ASTFile` --- .../ExpandModularHeadersPPCallbacks.cpp | 2 +- clang/include/clang/Basic/Module.h | 33 ++++++++++++++----- .../DependencyScanning/ModuleDepCollector.h | 2 +- .../include/clang/Serialization/ModuleFile.h | 2 +- clang/lib/Basic/ASTSourceDescriptor.cpp | 4 +-- .../DependencyScanning/ModuleDepCollector.cpp | 6 ++-- clang/lib/Frontend/CompilerInstance.cpp | 10 +++--- clang/lib/Frontend/FrontendAction.cpp | 2 +- clang/lib/Index/IndexingAction.cpp | 3 +- clang/lib/Sema/SemaModule.cpp | 4 +-- clang/lib/Serialization/ASTReader.cpp | 23 ++++++------- clang/lib/Serialization/ASTWriter.cpp | 6 ++-- clang/lib/Serialization/ModuleManager.cpp | 10 +++--- clang/test/Modules/DebugInfoSubmoduleImport.c | 2 +- .../test/Modules/DebugInfoTransitiveImport.m | 2 +- clang/tools/libclang/CIndex.cpp | 7 +--- clang/tools/libclang/CXIndexDataConsumer.cpp | 6 +++- 17 files changed, 68 insertions(+), 56 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp index cb6de9962956d..c989b4ad3859e 100644 --- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp +++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp @@ -164,7 +164,7 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective( if (ModuleImported) { serialization::ModuleFile *MF = Compiler.getASTReader()->getModuleManager().lookup( - *SuggestedModule->getASTFile()); + *SuggestedModule->getASTFileKey()); handleModuleFile(MF); } parseToLocation(DirectiveLoc); diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index c512249e5e945..43280906c7242 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -145,6 +145,10 @@ class ModuleFileName { /// Checks whether the module file name is empty. bool empty() const { return Path.empty(); } + bool isInModuleCache(StringRef ModuleCache) const { + return Separator && ModuleCache == StringRef(Path).substr(0, *Separator); + } + /// Creates the deduplication key for use in \c ModuleManager. std::optional<ModuleFileKey> makeKey(FileManager &FileMgr) const; }; @@ -353,9 +357,10 @@ class alignas(8) Module { /// \c SubModules vector at which that submodule resides. mutable llvm::StringMap<unsigned> SubModuleIndex; - /// The AST file if this is a top-level module which has a + /// The AST file name if this is a top-level module which has a /// corresponding serialized AST file, or null otherwise. - OptionalFileEntryRef ASTFile; + std::optional<ModuleFileName> ASTFileName; + std::optional<ModuleFileKey> ASTFileKey; /// The top-level headers associated with this module. llvm::SmallSetVector<FileEntryRef, 2> TopHeaders; @@ -828,15 +833,25 @@ class alignas(8) Module { return getTopLevelModule()->Name; } - /// The serialized AST file for this module, if one was created. - OptionalFileEntryRef getASTFile() const { - return getTopLevelModule()->ASTFile; + /// The serialized AST file name for this module, if one was created. + const ModuleFileName *getASTFileName() const { + const Module *TopLevel = getTopLevelModule(); + return TopLevel->ASTFileName ? &*TopLevel->ASTFileName : nullptr; + } + + /// The serialized AST file key for this module, if one was created. + const ModuleFileKey *getASTFileKey() const { + const Module *TopLevel = getTopLevelModule(); + return TopLevel->ASTFileKey ? &*TopLevel->ASTFileKey : nullptr; } - /// Set the serialized AST file for the top-level module of this module. - void setASTFile(OptionalFileEntryRef File) { - assert((!getASTFile() || getASTFile() == File) && "file path changed"); - getTopLevelModule()->ASTFile = File; + /// Set the serialized module file for the top-level module of this module. + void setASTFileNameAndKey(ModuleFileName NewName, ModuleFileKey NewKey) { + assert((!getASTFileName() && !getASTFileKey()) || + *getASTFileKey() == NewKey && "file path changed"); + Module *TopLevel = getTopLevelModule(); + TopLevel->ASTFileName = NewName; + TopLevel->ASTFileKey = NewKey; } /// Retrieve the umbrella directory as written. diff --git a/clang/include/clang/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/DependencyScanning/ModuleDepCollector.h index 52035dde4a757..bbb0f5b4d659b 100644 --- a/clang/include/clang/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/DependencyScanning/ModuleDepCollector.h @@ -42,7 +42,7 @@ struct PrebuiltModuleDep { explicit PrebuiltModuleDep(const Module *M) : ModuleName(M->getTopLevelModuleName()), - PCMFile(M->getASTFile()->getName()), + PCMFile(M->getASTFileName()->str()), ModuleMapFile(M->PresumedModuleMapFile) {} }; diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 7037bd18e3cc0..e761cadfcd86f 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -159,7 +159,7 @@ 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; diff --git a/clang/lib/Basic/ASTSourceDescriptor.cpp b/clang/lib/Basic/ASTSourceDescriptor.cpp index 8072c08a51d3a..933e79613992c 100644 --- a/clang/lib/Basic/ASTSourceDescriptor.cpp +++ b/clang/lib/Basic/ASTSourceDescriptor.cpp @@ -19,8 +19,8 @@ ASTSourceDescriptor::ASTSourceDescriptor(Module &M) : Signature(M.Signature), ClangModule(&M) { if (M.Directory) Path = M.Directory->getName(); - if (auto File = M.getASTFile()) - ASTFile = File->getName(); + if (auto FileKey = M.getASTFileName()) + ASTFile = FileKey->str(); } std::string ASTSourceDescriptor::getModuleName() const { diff --git a/clang/lib/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/DependencyScanning/ModuleDepCollector.cpp index 812881b23b0bd..35c9ee64c262f 100644 --- a/clang/lib/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/DependencyScanning/ModuleDepCollector.cpp @@ -682,7 +682,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { // -fmodule-name is used to compile a translation unit that imports this // module. In that case it can be skipped. The appropriate header // dependencies will still be reported as expected. - if (!M->getASTFile()) + if (!M->getASTFileKey()) return {}; // If this module has been handled already, just return its ID. @@ -716,7 +716,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) { serialization::ModuleFile *MF = MDC.ScanInstance.getASTReader()->getModuleManager().lookup( - *M->getASTFile()); + *M->getASTFileKey()); llvm::SmallString<256> Storage; MD.FileDepsBaseDir = @@ -938,7 +938,7 @@ bool ModuleDepCollector::isPrebuiltModule(const Module *M) { if (PrebuiltModuleFileIt == PrebuiltModuleFiles.end()) return false; assert("Prebuilt module came from the expected AST file" && - PrebuiltModuleFileIt->second == M->getASTFile()->getName()); + PrebuiltModuleFileIt->second == M->getASTFileName()->str()); return true; } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 8d0813d6c72d4..367369ecce02c 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1883,10 +1883,9 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( M = HS.lookupModule(ModuleName, ImportLoc, true, !IsInclusionDirective); // Check whether M refers to the file in the prebuilt module path. - if (M && M->getASTFile()) - if (auto ModuleFile = FileMgr->getOptionalFileRef(ModuleFilename)) - if (*ModuleFile == M->getASTFile()) - return M; + if (M && M->getASTFileKey() && + *M->getASTFileKey() == ModuleFilename.makeKey(*FileMgr)) + return M; getDiagnostics().Report(ModuleNameLoc, diag::err_module_prebuilt) << ModuleName; @@ -2295,8 +2294,7 @@ GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex( for (ModuleMap::module_iterator I = MMap.module_begin(), E = MMap.module_end(); I != E; ++I) { Module *TheModule = I->second; - OptionalFileEntryRef Entry = TheModule->getASTFile(); - if (!Entry) { + if (!TheModule->getASTFileKey()) { SmallVector<IdentifierLoc, 2> Path; Path.emplace_back(TriggerLoc, getPreprocessor().getIdentifierInfo(TheModule->Name)); diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index b82296c9c52f9..0cd60938bb1ec 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( diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp index 8118ceda9cd23..1d98179111e78 100644 --- a/clang/lib/Index/IndexingAction.cpp +++ b/clang/lib/Index/IndexingAction.cpp @@ -254,7 +254,8 @@ static void indexPreprocessorModuleMacros(Preprocessor &PP, if (M.second.getLatest() == nullptr) { for (auto *MM : PP.getLeafModuleMacros(M.first)) { auto *OwningMod = MM->getOwningModule(); - if (OwningMod && OwningMod->getASTFile() == Mod.File) { + if (OwningMod && OwningMod->getASTFileKey() && + *OwningMod->getASTFileKey() == Mod.FileKey) { if (auto *MI = MM->getMacroInfo()) { indexPreprocessorMacro(M.first, MI, MacroDirective::MD_Define, MI->getDefinitionLoc(), DataConsumer); diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp index 8fc3464ed3e0c..581163b9f0d56 100644 --- a/clang/lib/Sema/SemaModule.cpp +++ b/clang/lib/Sema/SemaModule.cpp @@ -386,9 +386,9 @@ Sema::ActOnModuleDecl(SourceLocation StartLoc, SourceLocation ModuleLoc, Diag(Path[0].getLoc(), diag::err_module_redefinition) << ModuleName; if (M->DefinitionLoc.isValid()) Diag(M->DefinitionLoc, diag::note_prev_module_definition); - else if (OptionalFileEntryRef FE = M->getASTFile()) + else if (const ModuleFileName *FileName = M->getASTFileName()) Diag(M->DefinitionLoc, diag::note_prev_module_definition_from_ast_file) - << FE->getName(); + << *FileName; Mod = M; break; } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 8713b17c91386..30d4435e4a789 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); } @@ -4654,17 +4654,17 @@ ASTReader::ReadModuleMapFileBlock(RecordData &Record, ModuleFile &F, << F.ModuleName << F.BaseDirectory << M->Directory->getName(); if (!canRecoverFromOutOfDate(F.FileName, ClientLoadCapabilities)) { - if (auto ASTFE = M ? M->getASTFile() : std::nullopt) { + if (auto ASTFileName = M ? M->getASTFileName() : nullptr) { // This module was defined by an imported (explicit) module. - Diag(diag::err_module_file_conflict) << F.ModuleName << F.FileName - << ASTFE->getName(); + Diag(diag::err_module_file_conflict) + << F.ModuleName << F.FileName << *ASTFileName; // TODO: Add a note with the module map paths if they differ. } else { // 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. @@ -6340,15 +6340,16 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, "too many submodules"); if (!ParentModule) { - if (OptionalFileEntryRef CurFile = CurrentModule->getASTFile()) { + if (const ModuleFileKey *CurFileKey = CurrentModule->getASTFileKey()) { // Don't emit module relocation error if we have -fno-validate-pch if (!bool(PP.getPreprocessorOpts().DisablePCHOrModuleValidation & DisableValidationForModuleKind::Module)) { - assert(CurFile != F.File && "ModuleManager did not de-duplicate"); + assert(*CurFileKey != F.FileKey && + "ModuleManager did not de-duplicate"); Diag(diag::err_module_file_conflict) - << CurrentModule->getTopLevelModuleName() << CurFile->getName() - << F.File.getName(); + << CurrentModule->getTopLevelModuleName() + << *CurrentModule->getASTFileName() << F.FileName; auto CurModMapFile = ModMap.getContainingModuleMapFile(CurrentModule); @@ -6362,7 +6363,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, } F.DidReadTopLevelSubmodule = true; - CurrentModule->setASTFile(F.File); + CurrentModule->setASTFileNameAndKey(F.FileName, F.FileKey); CurrentModule->PresumedModuleMapFile = F.ModuleMapPath; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 9fe1a1afe7679..e264ed12773ba 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1607,11 +1607,11 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { PP.getHeaderSearchInfo().getNormalizedModuleCachePath(); unsigned ModuleCacheLen = 0; if (M.Kind == MK_ImplicitModule && - StringRef(M.FileName).starts_with(NormalizedModuleCache)) + M.FileName.isInModuleCache(NormalizedModuleCache)) ModuleCacheLen = NormalizedModuleCache.size(); Record.push_back(ModuleCacheLen); - AddStringBlob(M.FileName, Record, Blob); + AddStringBlob(M.FileName.str(), Record, Blob); } Stream.EmitRecordWithBlob(AbbrevCode, Record, Blob); @@ -6161,7 +6161,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<uint16_t>(Name.size()); Out.write(Name.data(), Name.size()); diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 7d583b2a429a5..abb5b7955c8fa 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -51,8 +51,8 @@ ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const { ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const { if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name)) - if (OptionalFileEntryRef File = Mod->getASTFile()) - return lookup(*File); + if (const ModuleFileName *FileName = Mod->getASTFileName()) + return lookupByFileName(*FileName); return nullptr; } @@ -231,7 +231,7 @@ ModuleManager::AddModuleResult ModuleManager::addModule( auto NewModule = std::make_unique<ModuleFile>(Type, *FileKey, *Entry, Generation); NewModule->Index = Chain.size(); - NewModule->FileName = FileName.str(); + NewModule->FileName = FileName; NewModule->ImportLoc = ImportLoc; NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; NewModule->Buffer = ModuleBuffer; @@ -299,10 +299,8 @@ void ModuleManager::removeModules(ModuleIterator First) { } // Delete the modules. - for (ModuleIterator victim = First; victim != Last; ++victim) { - Modules.erase(ModuleFileKey(victim->File)); + for (ModuleIterator victim = First; victim != Last; ++victim) Modules.erase(victim->FileKey); - } Chain.erase(Chain.begin() + (First - begin()), Chain.end()); } diff --git a/clang/test/Modules/DebugInfoSubmoduleImport.c b/clang/test/Modules/DebugInfoSubmoduleImport.c index 9b00a9a95c537..91cb11ef5958a 100644 --- a/clang/test/Modules/DebugInfoSubmoduleImport.c +++ b/clang/test/Modules/DebugInfoSubmoduleImport.c @@ -5,7 +5,7 @@ // RUN: %s -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s // // RUN: %clang_cc1 -fmodules -fmodule-format=obj -debug-info-kind=limited -dwarf-ext-refs \ -// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t -I %S/Inputs \ +// RUN: -fimplicit-module-maps -x c -fmodules-cache-path=%t/cache -I %S/Inputs \ // RUN: -fmodules-local-submodule-visibility \ // RUN: %s -emit-llvm -debugger-tuning=lldb -o - | FileCheck %s #include "DebugSubmoduleA.h" diff --git a/clang/test/Modules/DebugInfoTransitiveImport.m b/clang/test/Modules/DebugInfoTransitiveImport.m index 4e4ba1eeafe68..425ae0001507d 100644 --- a/clang/test/Modules/DebugInfoTransitiveImport.m +++ b/clang/test/Modules/DebugInfoTransitiveImport.m @@ -1,7 +1,7 @@ // UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}} // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules -fmodule-format=obj -debug-info-kind=limited -dwarf-ext-refs \ -// RUN: -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %S/Inputs \ // RUN: %s -mllvm -debug-only=pchcontainer -debugger-tuning=lldb 2>&1 | FileCheck %s // REQUIRES: asserts diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp index 31b6a3222d916..3ee37ed2dfc27 100644 --- a/clang/tools/libclang/CIndex.cpp +++ b/clang/tools/libclang/CIndex.cpp @@ -9396,12 +9396,7 @@ CXModule clang_getModuleForFile(CXTranslationUnit TU, CXFile File) { return Header.getModule(); } -CXFile clang_Module_getASTFile(CXModule CXMod) { - if (!CXMod) - return nullptr; - Module *Mod = static_cast<Module *>(CXMod); - return cxfile::makeCXFile(Mod->getASTFile()); -} +CXFile clang_Module_getASTFile(CXModule CXMod) { return nullptr; } CXModule clang_Module_getParent(CXModule CXMod) { if (!CXMod) diff --git a/clang/tools/libclang/CXIndexDataConsumer.cpp b/clang/tools/libclang/CXIndexDataConsumer.cpp index c97aefaf87a8e..c707e79d6a597 100644 --- a/clang/tools/libclang/CXIndexDataConsumer.cpp +++ b/clang/tools/libclang/CXIndexDataConsumer.cpp @@ -505,7 +505,11 @@ void CXIndexDataConsumer::importedModule(const ImportDecl *ImportD) { if (SrcMod->getTopLevelModule() == Mod->getTopLevelModule()) return; - OptionalFileEntryRef FE = Mod->getASTFile(); + OptionalFileEntryRef FE; + if (auto ASTFileName = Mod->getASTFileName()) { + FileManager &FileMgr = Ctx->getSourceManager().getFileManager(); + FE = FileMgr.getOptionalFileRef(*ASTFileName); + } CXIdxImportedASTFileInfo Info = {cxfile::makeCXFile(FE), Mod, getIndexLoc(ImportD->getLocation()), ImportD->isImplicit()}; >From b62d6d6db1a8114e43ed130b31599482d979e70c Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 13 Mar 2026 13:51:00 -0700 Subject: [PATCH 7/7] Deprecate the `clang_Module_getASTFile()` API --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang-c/Index.h | 2 ++ clang/test/Index/annotate-module.m | 2 +- clang/tools/c-index-test/c-index-test.c | 9 ++------- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 85b3f3e6e3f41..a4f2dd1a40ece 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -484,6 +484,7 @@ libclang - Visit constraints of `auto` type to properly visit concept usages (#GH166580) - Visit switch initializer statements (https://bugs.kde.org/show_bug.cgi?id=415537#c2) - Fix crash in clang_getBinaryOperatorKindSpelling and clang_getUnaryOperatorKindSpelling +- The clang_Module_getASTFile API is deprecated and now always returns nullptr Code Completion --------------- diff --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h index 203634c80d82a..ca15fbafaac3e 100644 --- a/clang/include/clang-c/Index.h +++ b/clang/include/clang-c/Index.h @@ -4651,6 +4651,8 @@ CINDEX_LINKAGE CXModule clang_getModuleForFile(CXTranslationUnit, CXFile); * \param Module a module object. * * \returns the module file where the provided module object came from. + * + * @deprecated: module files are longer guaranteed to be loaded from a CXFile */ CINDEX_LINKAGE CXFile clang_Module_getASTFile(CXModule Module); diff --git a/clang/test/Index/annotate-module.m b/clang/test/Index/annotate-module.m index 24dce3f290bfe..c8b719550d030 100644 --- a/clang/test/Index/annotate-module.m +++ b/clang/test/Index/annotate-module.m @@ -46,6 +46,6 @@ // RUN: c-index-test -cursor-at=%s:3:11 %s -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs \ // RUN: | FileCheck %s -check-prefix=CHECK-CURSOR -// CHECK-CURSOR: 3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(2): +// CHECK-CURSOR: 3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule system=0 Headers(2): // CHECK-CURSOR-NEXT: {{.*}}other.h // CHECK-CURSOR-NEXT: {{.*}}DependsOnModule.h diff --git a/clang/tools/c-index-test/c-index-test.c b/clang/tools/c-index-test/c-index-test.c index cb3245756a394..4b3e105aa7aff 100644 --- a/clang/tools/c-index-test/c-index-test.c +++ b/clang/tools/c-index-test/c-index-test.c @@ -3099,19 +3099,14 @@ static void inspect_print_cursor(CXCursor Cursor) { { CXModule mod = clang_Cursor_getModule(Cursor); - CXFile astFile; - CXString name, astFilename; + CXString name; unsigned i, numHeaders; if (mod) { - astFile = clang_Module_getASTFile(mod); - astFilename = clang_getFileName(astFile); name = clang_Module_getFullName(mod); numHeaders = clang_Module_getNumTopLevelHeaders(TU, mod); - printf(" ModuleName=%s (%s) system=%d Headers(%d):", - clang_getCString(name), clang_getCString(astFilename), + printf(" ModuleName=%s system=%d Headers(%d):", clang_getCString(name), clang_Module_isSystem(mod), numHeaders); clang_disposeString(name); - clang_disposeString(astFilename); for (i = 0; i < numHeaders; ++i) { CXFile file = clang_Module_getTopLevelHeader(TU, mod, i); CXString filename = clang_getFileName(file); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
