https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/185995
>From f92608f3d87dd35bcc8610fe3a5af985c1728d3c Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 9 Mar 2026 20:52:07 -0700 Subject: [PATCH 1/7] [clang][modules] Remove `ModuleFile::File` --- clang/include/clang/Basic/Module.h | 6 +-- clang/include/clang/Frontend/ASTUnit.h | 2 +- clang/include/clang/Serialization/ASTReader.h | 4 +- clang/include/clang/Serialization/ASTWriter.h | 2 +- .../include/clang/Serialization/ModuleFile.h | 13 ++--- .../clang/Serialization/ModuleManager.h | 8 --- .../DependencyScannerImpl.cpp | 9 ++-- clang/lib/Frontend/ASTUnit.cpp | 4 +- clang/lib/Frontend/DependencyFile.cpp | 2 +- clang/lib/Frontend/FrontendAction.cpp | 18 ++++++- .../lib/Frontend/Rewrite/FrontendActions.cpp | 15 +++--- clang/lib/Serialization/ASTReader.cpp | 14 ++--- clang/lib/Serialization/ASTWriter.cpp | 14 ++--- clang/lib/Serialization/GlobalModuleIndex.cpp | 3 +- clang/lib/Serialization/ModuleManager.cpp | 52 ++++++------------- .../prebuilt-modules-in-stable-dirs.c | 1 + clang/tools/libclang/CXIndexDataConsumer.cpp | 5 +- clang/tools/libclang/CXIndexDataConsumer.h | 2 +- clang/tools/libclang/Indexing.cpp | 9 ++-- 19 files changed, 82 insertions(+), 101 deletions(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 016cc2ce684b8..cb996430076e2 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -54,10 +54,6 @@ 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 @@ -77,7 +73,7 @@ class ModuleFileKey { /// for other kinds of module files. std::string ImplicitModulePathSuffix; - friend class serialization::ModuleManager; + friend class ASTReader; friend class ModuleFileName; friend llvm::DenseMapInfo<ModuleFileKey>; diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index 7f307d1670dc6..b187494e449f2 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -679,7 +679,7 @@ class ASTUnit { bool visitLocalTopLevelDecls(void *context, DeclVisitorFn Fn); /// Get the PCH file if one was included. - OptionalFileEntryRef getPCHFile(); + std::optional<StringRef> getPCHFile(); /// Returns true if the ASTUnit was constructed from a serialized /// module file. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index e9706d0ea2f2b..d6f75e5973c45 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -223,7 +223,7 @@ class ASTReaderListener { } /// This is called for each AST file loaded. - virtual void visitModuleFile(StringRef Filename, + virtual void visitModuleFile(ModuleFileName Filename, serialization::ModuleKind Kind) {} /// Returns true if this \c ASTReaderListener wants to receive the @@ -313,7 +313,7 @@ class ChainedASTReaderListener : public ASTReaderListener { void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override; bool needsInputFileVisitation() override; bool needsSystemInputFileVisitation() override; - void visitModuleFile(StringRef Filename, + void visitModuleFile(ModuleFileName Filename, serialization::ModuleKind Kind) override; bool visitInputFile(StringRef Filename, bool isSystem, bool isOverridden, bool isExplicitModule) override; diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 0f3993ad01693..be2fbdf1ade83 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -702,7 +702,7 @@ class ASTWriter : public ASTDeserializationListener, /// Get a timestamp for output into the AST file. The actual timestamp /// of the specified file may be ignored if we have been instructed to not /// include timestamps in the output file. - time_t getTimestampForOutput(const FileEntry *E) const; + time_t getTimestampForOutput(time_t ModTime) const; /// Write a precompiled header or a module with the AST produced by the /// \c Sema object, or a dependency scanner module with the preprocessor state diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index e761cadfcd86f..303bd65a8aad0 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -144,10 +144,8 @@ enum class InputFilesValidation { /// other modules. class ModuleFile { public: - ModuleFile(ModuleKind Kind, ModuleFileKey FileKey, FileEntryRef File, - unsigned Generation) - : Kind(Kind), FileKey(std::move(FileKey)), File(File), - Generation(Generation) {} + ModuleFile(ModuleKind Kind, ModuleFileKey FileKey, unsigned Generation) + : Kind(Kind), FileKey(std::move(FileKey)), Generation(Generation) {} ~ModuleFile(); // === General information === @@ -201,8 +199,11 @@ class ModuleFile { /// Whether the top-level module has been read from the AST file. bool DidReadTopLevelSubmodule = false; - /// The file entry for the module file. - FileEntryRef File; + /// Size of the module file. + off_t Size = 0; + + /// Modification of the module file. + time_t ModTime = 0; /// The signature of the module file, which may be used instead of the size /// and modification time to identify this particular file. diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 162856f2f14c0..1ef9aeee7e1fd 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -172,17 +172,9 @@ class ModuleManager { /// Returns the module associated with the given index 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; diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 20284c0d9165a..f882713a8b76d 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -135,7 +135,7 @@ class PrebuiltModuleListener : public ASTReaderListener { } /// Update which module that is being actively traversed. - void visitModuleFile(StringRef Filename, + void visitModuleFile(ModuleFileName Filename, serialization::ModuleKind Kind) override { // If the CurrentFile is not // considered stable, update any of it's transitive dependents. @@ -144,7 +144,7 @@ class PrebuiltModuleListener : public ASTReaderListener { !PrebuiltEntryIt->second.isInStableDir()) PrebuiltEntryIt->second.updateDependentsNotInStableDirs( PrebuiltModulesASTMap); - CurrentFile = Filename; + CurrentFile = Filename.str(); } /// Check the header search options for a given module when considering @@ -206,7 +206,7 @@ static bool visitPrebuiltModule(StringRef PrebuiltModuleFilename, CI.getHeaderSearchOpts(), CI.getLangOpts(), Diags, StableDirs); - Listener.visitModuleFile(PrebuiltModuleFilename, + Listener.visitModuleFile(ModuleFileName::makeExplicit(PrebuiltModuleFilename), serialization::MK_ExplicitModule); if (ASTReader::readASTFileControlBlock( PrebuiltModuleFilename, CI.getFileManager(), CI.getModuleCache(), @@ -216,7 +216,8 @@ static bool visitPrebuiltModule(StringRef PrebuiltModuleFilename, return true; while (!Worklist.empty()) { - Listener.visitModuleFile(Worklist.back(), serialization::MK_ExplicitModule); + Listener.visitModuleFile(ModuleFileName::makeExplicit(Worklist.back()), + serialization::MK_ExplicitModule); if (ASTReader::readASTFileControlBlock( Worklist.pop_back_val(), CI.getFileManager(), CI.getModuleCache(), CI.getPCHContainerReader(), diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 1e10178285bbd..05ae1f348f920 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -2443,7 +2443,7 @@ bool ASTUnit::visitLocalTopLevelDecls(void *context, DeclVisitorFn Fn) { return true; } -OptionalFileEntryRef ASTUnit::getPCHFile() { +std::optional<StringRef> ASTUnit::getPCHFile() { if (!Reader) return std::nullopt; @@ -2466,7 +2466,7 @@ OptionalFileEntryRef ASTUnit::getPCHFile() { return true; }); if (Mod) - return Mod->File; + return Mod->FileName; return std::nullopt; } diff --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp index 25584b4900228..64629abcaeb52 100644 --- a/clang/lib/Frontend/DependencyFile.cpp +++ b/clang/lib/Frontend/DependencyFile.cpp @@ -154,7 +154,7 @@ struct DepCollectorASTListener : public ASTReaderListener { bool needsSystemInputFileVisitation() override { return DepCollector.needSystemDependencies(); } - void visitModuleFile(StringRef Filename, + void visitModuleFile(ModuleFileName Filename, serialization::ModuleKind Kind) override { DepCollector.maybeAddDependency(Filename, /*FromModule*/ true, /*IsSystem*/ false, /*IsModuleFile*/ true, diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 81788d17acc4d..c3d1d802dfe04 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -850,6 +850,11 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, if (!BeginInvocation(CI)) return false; + // The list of module files the input AST file depends on. This is separate + // from FrontendOptions::ModuleFiles, because those only represent explicit + // modules, while this is capable of representing implicit ones too. + SmallVector<ModuleFileName> ModuleFiles; + // If we're replaying the build of an AST file, import it and set up // the initial state from its build. if (ReplayASTFile) { @@ -892,7 +897,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, for (serialization::ModuleFile &MF : MM) if (&MF != &PrimaryModule) - CI.getFrontendOpts().ModuleFiles.emplace_back(MF.FileName.str()); + ModuleFiles.emplace_back(MF.FileName); ASTReader->visitTopLevelModuleMaps(PrimaryModule, [&](FileEntryRef FE) { CI.getFrontendOpts().ModuleMapFiles.push_back( @@ -1296,6 +1301,17 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, diag::warn_eagerly_load_for_standard_cplusplus_modules); } + // If we were asked to load any module files by the ASTUnit, do so now. + for (const auto &ModuleFile : ModuleFiles) { + serialization::ModuleFile *Loaded = nullptr; + if (!CI.loadModuleFile(ModuleFile, Loaded)) + return false; + + if (Loaded && Loaded->StandardCXXModule) + CI.getDiagnostics().Report( + diag::warn_eagerly_load_for_standard_cplusplus_modules); + } + // If there is a layout overrides file, attach an external AST source that // provides the layouts from that file. if (!CI.getFrontendOpts().OverrideRecordLayoutsFile.empty() && diff --git a/clang/lib/Frontend/Rewrite/FrontendActions.cpp b/clang/lib/Frontend/Rewrite/FrontendActions.cpp index ef6f9ccf87848..1e12d3a6ea3df 100644 --- a/clang/lib/Frontend/Rewrite/FrontendActions.cpp +++ b/clang/lib/Frontend/Rewrite/FrontendActions.cpp @@ -205,25 +205,22 @@ class RewriteIncludesAction::RewriteImportsListener : public ASTReaderListener { CompilerInstance &CI; std::weak_ptr<raw_ostream> Out; - llvm::DenseSet<const FileEntry*> Rewritten; + llvm::DenseSet<const serialization::ModuleFile *> Rewritten; public: RewriteImportsListener(CompilerInstance &CI, std::shared_ptr<raw_ostream> Out) : CI(CI), Out(Out) {} - void visitModuleFile(StringRef Filename, + void visitModuleFile(ModuleFileName Filename, serialization::ModuleKind Kind) override { - auto File = CI.getFileManager().getOptionalFileRef(Filename); - assert(File && "missing file for loaded module?"); + serialization::ModuleFile *MF = + CI.getASTReader()->getModuleManager().lookupByFileName(Filename); + assert(MF && "missing module file for loaded module?"); // Only rewrite each module file once. - if (!Rewritten.insert(*File).second) + if (!Rewritten.insert(MF).second) return; - serialization::ModuleFile *MF = - CI.getASTReader()->getModuleManager().lookup(*File); - assert(MF && "missing module file for loaded module?"); - // Not interested in PCH / preambles. if (!MF->isModule()) return; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 03b1b02859b81..9274d4cc6f0fa 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -240,7 +240,7 @@ bool ChainedASTReaderListener::needsSystemInputFileVisitation() { Second->needsSystemInputFileVisitation(); } -void ChainedASTReaderListener::visitModuleFile(StringRef Filename, +void ChainedASTReaderListener::visitModuleFile(ModuleFileName Filename, ModuleKind Kind) { First->visitModuleFile(Filename, Kind); Second->visitModuleFile(Filename, Kind); @@ -3193,8 +3193,7 @@ ASTReader::getModuleForRelocationChecks(ModuleFile &F, bool DirectoryCheck) { if (HSOpts.ModulesValidateOncePerBuildSession && IsImplicitModule) { if (F.InputFilesValidationTimestamp >= HSOpts.BuildSessionTimestamp) return {std::nullopt, IgnoreError}; - if (static_cast<uint64_t>(F.File.getModificationTime()) >= - HSOpts.BuildSessionTimestamp) + if (static_cast<uint64_t>(F.ModTime) >= HSOpts.BuildSessionTimestamp) return {std::nullopt, IgnoreError}; } @@ -4593,10 +4592,11 @@ void ASTReader::ReadModuleOffsetMap(ModuleFile &F) const { uint16_t Len = endian::readNext<uint16_t, llvm::endianness::little>(Data); StringRef Name = StringRef((const char*)Data, Len); Data += Len; - ModuleFile *OM = (Kind == MK_PrebuiltModule || Kind == MK_ExplicitModule || - Kind == MK_ImplicitModule - ? ModuleMgr.lookupByModuleName(Name) - : ModuleMgr.lookupByFileName(Name)); + ModuleFile *OM = + (Kind == MK_PrebuiltModule || Kind == MK_ExplicitModule || + Kind == MK_ImplicitModule + ? ModuleMgr.lookupByModuleName(Name) + : ModuleMgr.lookupByFileName(ModuleFileName::makeExplicit(Name))); if (!OM) { std::string Msg = "refers to unknown module, cannot find "; Msg.append(std::string(Name)); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index e3db39a1acb74..2b0808ad5ad5e 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1598,8 +1598,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { } else { // If we have calculated signature, there is no need to store // the size or timestamp. - Record.push_back(M.Signature ? 0 : M.File.getSize()); - Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File)); + Record.push_back(M.Signature ? 0 : M.Size); + Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.ModTime)); llvm::append_range(Blob, M.Signature); @@ -1963,7 +1963,7 @@ void ASTWriter::WriteInputFiles(SourceManager &SourceMgr) { INPUT_FILE, InputFileOffsets.size(), (uint64_t)Entry.File.getSize(), - (uint64_t)getTimestampForOutput(Entry.File), + (uint64_t)getTimestampForOutput(Entry.File.getModificationTime()), Entry.BufferOverridden, Entry.IsTransient, Entry.IsTopLevel, @@ -2280,8 +2280,8 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) { bool Included = HFI->IsLocallyIncluded || PP->alreadyIncluded(*File); HeaderFileInfoTrait::key_type Key = { - Filename, File->getSize(), getTimestampForOutput(*File) - }; + Filename, File->getSize(), + getTimestampForOutput(File->getModificationTime())}; HeaderFileInfoTrait::data_type Data = { *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(*File), {} }; @@ -5488,8 +5488,8 @@ const LangOptions &ASTWriter::getLangOpts() const { return PP->getLangOpts(); } -time_t ASTWriter::getTimestampForOutput(const FileEntry *E) const { - return IncludeTimestamps ? E->getModificationTime() : 0; +time_t ASTWriter::getTimestampForOutput(time_t ModTime) const { + return IncludeTimestamps ? ModTime : 0; } ASTFileSignature diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index 8ab8717776c3d..3f6b8e68e38f3 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -342,8 +342,7 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) { // If the size and modification time match what we expected, record this // module file. bool Failed = true; - if (File->File.getSize() == Info.Size && - File->File.getModificationTime() == Info.ModTime) { + if (File->Size == Info.Size && File->ModTime == Info.ModTime) { Info.File = File; ModulesByFile[File] = Known->second; diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index abb5b7955c8fa..6768028cd3ae3 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -40,15 +40,6 @@ using namespace clang; using namespace serialization; -ModuleFile *ModuleManager::lookupByFileName(StringRef Name) const { - auto Entry = FileMgr.getOptionalFileRef(Name, /*OpenFile=*/false, - /*CacheFailure=*/false); - if (Entry) - return lookup(*Entry); - - return nullptr; -} - ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const { if (const Module *Mod = HeaderSearchInfo.getModuleMap().findModule(Name)) if (const ModuleFileName *FileName = Mod->getASTFileName()) @@ -57,10 +48,6 @@ ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const { return nullptr; } -ModuleFile *ModuleManager::lookup(const FileEntry *File) const { - return lookup(ModuleFileKey(File)); -} - ModuleFile *ModuleManager::lookupByFileName(ModuleFileName Name) const { std::optional<ModuleFileKey> Key = Name.makeKey(FileMgr); return Key ? lookup(*Key) : nullptr; @@ -79,16 +66,14 @@ ModuleManager::lookupBuffer(StringRef Name) { return std::move(InMemoryBuffers[*Entry]); } -static bool checkModuleFile(const FileEntry *File, off_t ExpectedSize, +static bool checkModuleFile(off_t Size, time_t ModTime, off_t ExpectedSize, time_t ExpectedModTime, std::string &ErrorStr) { - assert(File && "Checking expectations of a non-existent module file"); - - if (ExpectedSize && ExpectedSize != File->getSize()) { + if (ExpectedSize && ExpectedSize != Size) { ErrorStr = "module file has a different size than expected"; return true; } - if (ExpectedModTime && ExpectedModTime != File->getModificationTime()) { + if (ExpectedModTime && ExpectedModTime != ModTime) { ErrorStr = "module file has a different modification time than expected"; return true; } @@ -153,8 +138,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // Check whether we already loaded this module, before if (ModuleFile *ModuleEntry = lookup(*FileKey)) { // Check file properties. - if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, - ErrorStr)) + if (checkModuleFile(ModuleEntry->Size, ModuleEntry->ModTime, ExpectedSize, + ExpectedModTime, ErrorStr)) return OutOfDate; // Check the stored signature. @@ -167,7 +152,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( } // Load the contents of the module - OptionalFileEntryRef Entry; + off_t Size = ExpectedSize; + time_t ModTime = ExpectedModTime; llvm::MemoryBuffer *ModuleBuffer = nullptr; std::unique_ptr<llvm::MemoryBuffer> NewFileBuffer = nullptr; if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { @@ -184,7 +170,7 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // import it earlier. return OutOfDate; } else { - Entry = + OptionalFileEntryRef Entry = expectedToOptional(FileName == StringRef("-") ? FileMgr.getSTDIN() : FileMgr.getFileRef(FileName, /*OpenFile=*/true, @@ -198,7 +184,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // 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)) + if (checkModuleFile(Entry->getSize(), Entry->getModificationTime(), + ExpectedSize, ExpectedModTime, ErrorStr)) return OutOfDate; // Get a buffer of the file and close the file descriptor when done. @@ -216,24 +203,20 @@ ModuleManager::AddModuleResult ModuleManager::addModule( return Missing; } + Size = Entry->getSize(); + ModTime = Entry->getModificationTime(); NewFileBuffer = std::move(*Buf); 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, *FileKey, *Entry, Generation); + auto NewModule = std::make_unique<ModuleFile>(Type, *FileKey, Generation); NewModule->Index = Chain.size(); NewModule->FileName = FileName; NewModule->ImportLoc = ImportLoc; NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; + NewModule->Size = Size; + NewModule->ModTime = ModTime; NewModule->Buffer = ModuleBuffer; // Initialize the stream. NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); @@ -251,11 +234,6 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // 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); if (!NewModule->isModule()) diff --git a/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c b/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c index 39b2863d966c3..29fc9dda2953c 100644 --- a/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c +++ b/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c @@ -15,6 +15,7 @@ // RUN: sed -e "s|DIR|%/t|g" %t/compile-pch.json.in > %t/compile-pch.json // RUN: clang-scan-deps -compilation-database %t/compile-pch.json \ // RUN: -j 1 -format experimental-full > %t/deps_pch.db +// FIXME: We can't just build the PCH implicitly and use it in the scan. // RUN: %clang -x c-header -c %t/prebuild.h -isysroot %t/MacOSX.sdk \ // RUN: -I%t/BuildDir -ivfsoverlay %t/overlay.json \ // RUN: -I %t/MacOSX.sdk/usr/include -fmodules -fmodules-cache-path=%t/module-cache \ diff --git a/clang/tools/libclang/CXIndexDataConsumer.cpp b/clang/tools/libclang/CXIndexDataConsumer.cpp index 265d5876ee7a6..8babcccf38c51 100644 --- a/clang/tools/libclang/CXIndexDataConsumer.cpp +++ b/clang/tools/libclang/CXIndexDataConsumer.cpp @@ -517,10 +517,13 @@ void CXIndexDataConsumer::importedModule(const ImportDecl *ImportD) { (void)astFile; } -void CXIndexDataConsumer::importedPCH(FileEntryRef File) { +void CXIndexDataConsumer::importedPCH(StringRef FileName) { if (!CB.importedASTFile) return; + FileManager &FileMgr = cxtu::getASTUnit(CXTU)->getFileManager(); + OptionalFileEntryRef File = FileMgr.getOptionalFileRef(FileName); + CXIdxImportedASTFileInfo Info = { cxfile::makeCXFile(File), /*module=*/nullptr, diff --git a/clang/tools/libclang/CXIndexDataConsumer.h b/clang/tools/libclang/CXIndexDataConsumer.h index b207db7cde6d7..3608f76a6fb8f 100644 --- a/clang/tools/libclang/CXIndexDataConsumer.h +++ b/clang/tools/libclang/CXIndexDataConsumer.h @@ -367,7 +367,7 @@ class CXIndexDataConsumer : public index::IndexDataConsumer { bool isModuleImport); void importedModule(const ImportDecl *ImportD); - void importedPCH(FileEntryRef File); + void importedPCH(StringRef FileName); void startedTranslationUnit(); diff --git a/clang/tools/libclang/Indexing.cpp b/clang/tools/libclang/Indexing.cpp index 75323d70afcfe..4e57b63490112 100644 --- a/clang/tools/libclang/Indexing.cpp +++ b/clang/tools/libclang/Indexing.cpp @@ -351,11 +351,8 @@ class IndexingFrontendAction : public ASTFrontendAction { StringRef InFile) override { PreprocessorOptions &PPOpts = CI.getPreprocessorOpts(); - if (!PPOpts.ImplicitPCHInclude.empty()) { - if (auto File = - CI.getFileManager().getOptionalFileRef(PPOpts.ImplicitPCHInclude)) - DataConsumer->importedPCH(*File); - } + if (!PPOpts.ImplicitPCHInclude.empty()) + DataConsumer->importedPCH(PPOpts.ImplicitPCHInclude); DataConsumer->setASTContext(CI.getASTContextPtr()); Preprocessor &PP = CI.getPreprocessor(); @@ -695,7 +692,7 @@ static CXErrorCode clang_indexTranslationUnit_Impl( ASTUnit::ConcurrencyCheck Check(*Unit); - if (OptionalFileEntryRef PCHFile = Unit->getPCHFile()) + if (std::optional<StringRef> PCHFile = Unit->getPCHFile()) DataConsumer.importedPCH(*PCHFile); FileManager &FileMgr = Unit->getFileManager(); >From 8abf3cc48d6ddff6e1182168920765b3ea725f46 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 19 Mar 2026 16:02:40 -0700 Subject: [PATCH 2/7] End an unnecessary friendship --- clang/include/clang/Basic/Module.h | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index cb996430076e2..70668860dadc2 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -73,7 +73,6 @@ class ModuleFileKey { /// for other kinds of module files. std::string ImplicitModulePathSuffix; - friend class ASTReader; friend class ModuleFileName; friend llvm::DenseMapInfo<ModuleFileKey>; >From 82f3ff21b646139a62906ec79d1d55d336238967 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 20 Mar 2026 14:48:20 -0700 Subject: [PATCH 3/7] Fix "clang/test/ClangScanDeps/modules-pch-common-stale.c" --- clang/lib/Serialization/ModuleManager.cpp | 30 +++++++++++++++++------ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 6768028cd3ae3..ed7b6cf67674e 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -164,6 +164,24 @@ ModuleManager::AddModuleResult ModuleManager::addModule( getModuleCache().getInMemoryModuleCache().lookupPCM( FileName)) { ModuleBuffer = Buffer; + if (!FileName.getImplicitModuleSuffixLength()) { + // Explicitly-built PCM files maintain consistency via mtime/size + // expectations on their imports. Even if we've previously successfully + // loaded a PCM file and stored it in the in-memory module cache, that + // does not mean its mtime/size matches current importer's expectations. + // Get that information so that it can be checked below. + // FIXME: Even though this FileManager access is likely already cached, we + // should store this directly in the in-memory module cache. + OptionalFileEntryRef Entry = + FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, + /*CacheFailure=*/false); + if (!Entry) { + ErrorStr = "module file not found"; + return Missing; + } + ModTime = Entry->getModificationTime(); + Size = Entry->getSize(); + } } else if (getModuleCache().getInMemoryModuleCache().shouldBuildPCM( FileName)) { // Report that the module is out of date, since we tried (and failed) to @@ -180,14 +198,6 @@ ModuleManager::AddModuleResult ModuleManager::addModule( 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->getSize(), Entry->getModificationTime(), - 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. @@ -221,6 +231,10 @@ ModuleManager::AddModuleResult ModuleManager::addModule( // Initialize the stream. NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); + // Check file properties. + if (checkModuleFile(Size, ModTime, ExpectedSize, ExpectedModTime, ErrorStr)) + return OutOfDate; + // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. if (ExpectedSignature && checkSignature(ReadSignature(NewModule->Data), >From 985cd156486a9576e95bc2f92daebe0bb9283d05 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 20 Mar 2026 14:48:38 -0700 Subject: [PATCH 4/7] Fix "clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c" --- .../prebuilt-modules-in-stable-dirs.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c b/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c index 29fc9dda2953c..54b4b23771ca3 100644 --- a/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c +++ b/clang/test/ClangScanDeps/prebuilt-modules-in-stable-dirs.c @@ -15,11 +15,18 @@ // RUN: sed -e "s|DIR|%/t|g" %t/compile-pch.json.in > %t/compile-pch.json // RUN: clang-scan-deps -compilation-database %t/compile-pch.json \ // RUN: -j 1 -format experimental-full > %t/deps_pch.db -// FIXME: We can't just build the PCH implicitly and use it in the scan. -// RUN: %clang -x c-header -c %t/prebuild.h -isysroot %t/MacOSX.sdk \ -// RUN: -I%t/BuildDir -ivfsoverlay %t/overlay.json \ -// RUN: -I %t/MacOSX.sdk/usr/include -fmodules -fmodules-cache-path=%t/module-cache \ -// RUN: -fimplicit-module-maps -o %t/prebuild.pch + +// RUN: %deps-to-rsp %t/deps_pch.db --module-name=A > %t/A.rsp +// RUN: %deps-to-rsp %t/deps_pch.db --module-name=B > %t/B.rsp +// RUN: %deps-to-rsp %t/deps_pch.db --module-name=B_transitive > %t/B_transitive.rsp +// RUN: %deps-to-rsp %t/deps_pch.db --module-name=C > %t/C.rsp +// RUN: %deps-to-rsp %t/deps_pch.db --tu-index=0 > %t/pch.rsp +// RUN: %clang @%t/A.rsp +// RUN: %clang @%t/B.rsp +// RUN: %clang @%t/B_transitive.rsp +// RUN: %clang @%t/C.rsp +// RUN: %clang @%t/pch.rsp + // RUN: sed -e "s|DIR|%/t|g" %t/compile-commands.json.in > %t/compile-commands.json // RUN: clang-scan-deps -compilation-database %t/compile-commands.json \ // RUN: -j 1 -format experimental-full > %t/deps.db >From 3d2f13064ec41f65fa01208bfd9992826c866ef2 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 20 Mar 2026 20:54:01 -0700 Subject: [PATCH 5/7] Write to module cache --- .../include/clang/Frontend/CompilerInstance.h | 10 +++-- .../include/clang/Frontend/FrontendActions.h | 12 ++++++ .../include/clang/Serialization/ModuleCache.h | 7 +++- .../InProcessModuleCache.cpp | 6 +++ clang/lib/Frontend/CompilerInstance.cpp | 42 ++++++++++++++----- clang/lib/Frontend/FrontendActions.cpp | 3 +- clang/lib/Serialization/ModuleCache.cpp | 33 +++++++++++++++ 7 files changed, 96 insertions(+), 17 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 0d684d5c7f9fe..01f83498d8c8e 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -937,12 +937,14 @@ class CompilerInstance : public ModuleLoader { std::optional<ThreadSafeCloneConfig> ThreadSafeConfig = std::nullopt); /// Compile a module file for the given module, using the options - /// provided by the importing compiler instance. Returns true if the module - /// was built without errors. + /// provided by the importing compiler instance. Returns the PCM file in + /// a buffer. // FIXME: This should be private, but it's called from static non-member // functions in the implementation file. - bool compileModule(SourceLocation ImportLoc, StringRef ModuleName, - StringRef ModuleFileName, CompilerInstance &Instance); + std::unique_ptr<llvm::MemoryBuffer> compileModule(SourceLocation ImportLoc, + StringRef ModuleName, + StringRef ModuleFileName, + CompilerInstance &Instance); ModuleLoadResult loadModule(SourceLocation ImportLoc, ModuleIdPath Path, Module::NameVisibilityKind Visibility, diff --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h index 87a9f0d4cb06c..3d4da567e18dd 100644 --- a/clang/include/clang/Frontend/FrontendActions.h +++ b/clang/include/clang/Frontend/FrontendActions.h @@ -114,6 +114,13 @@ class GeneratePCHAction : public ASTFrontendAction { }; class GenerateModuleAction : public ASTFrontendAction { +public: + explicit GenerateModuleAction(std::unique_ptr<raw_pwrite_stream> OS = nullptr) + : OS(std::move(OS)) {} + +private: + std::unique_ptr<raw_pwrite_stream> OS; + virtual std::unique_ptr<raw_pwrite_stream> CreateOutputFile(CompilerInstance &CI, StringRef InFile) = 0; @@ -145,6 +152,11 @@ class GenerateInterfaceStubsAction : public ASTFrontendAction { }; class GenerateModuleFromModuleMapAction : public GenerateModuleAction { +public: + explicit GenerateModuleFromModuleMapAction( + std::unique_ptr<raw_pwrite_stream> OS = nullptr) + : GenerateModuleAction(std::move(OS)) {} + private: bool BeginSourceFileAction(CompilerInstance &CI) override; diff --git a/clang/include/clang/Serialization/ModuleCache.h b/clang/include/clang/Serialization/ModuleCache.h index c6795c5dc358a..febc642d205ab 100644 --- a/clang/include/clang/Serialization/ModuleCache.h +++ b/clang/include/clang/Serialization/ModuleCache.h @@ -14,6 +14,7 @@ #include <ctime> namespace llvm { +class MemoryBufferRef; class AdvisoryLock; } // namespace llvm @@ -52,7 +53,9 @@ class ModuleCache { virtual InMemoryModuleCache &getInMemoryModuleCache() = 0; virtual const InMemoryModuleCache &getInMemoryModuleCache() const = 0; - // TODO: Virtualize writing/reading PCM files, etc. + virtual time_t write(StringRef Path, llvm::MemoryBufferRef Buffer) = 0; + + // TODO: Virtualize reading PCM files, etc. virtual ~ModuleCache() = default; }; @@ -65,6 +68,8 @@ std::shared_ptr<ModuleCache> createCrossProcessModuleCache(); /// Shared implementation of `ModuleCache::maybePrune()`. void maybePruneImpl(StringRef Path, time_t PruneInterval, time_t PruneAfter); + +time_t writeImpl(StringRef Path, llvm::MemoryBufferRef Buffer); } // namespace clang #endif diff --git a/clang/lib/DependencyScanning/InProcessModuleCache.cpp b/clang/lib/DependencyScanning/InProcessModuleCache.cpp index cd7385c8f38c2..16d0893690c39 100644 --- a/clang/lib/DependencyScanning/InProcessModuleCache.cpp +++ b/clang/lib/DependencyScanning/InProcessModuleCache.cpp @@ -127,6 +127,12 @@ class InProcessModuleCache : public ModuleCache { maybePruneImpl(Path, PruneInterval, PruneAfter); } + time_t write(StringRef Path, llvm::MemoryBufferRef Buffer) override { + // FIXME: This could use an in-memory cache to avoid IO, and only write all + // PCMs at the end. + return writeImpl(Path, Buffer); + } + InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; } const InMemoryModuleCache &getInMemoryModuleCache() const override { return InMemory; diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 33919bc1c4634..7647f80c2f96a 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1236,7 +1236,7 @@ class PrettyStackTraceBuildModule : public llvm::PrettyStackTraceEntry { }; } // namespace -bool CompilerInstance::compileModule(SourceLocation ImportLoc, +std::unique_ptr<llvm::MemoryBuffer> CompilerInstance::compileModule(SourceLocation ImportLoc, StringRef ModuleName, StringRef ModuleFileName, CompilerInstance &Instance) { @@ -1248,18 +1248,22 @@ bool CompilerInstance::compileModule(SourceLocation ImportLoc, if (getModuleCache().getInMemoryModuleCache().isPCMFinal(ModuleFileName)) { getDiagnostics().Report(ImportLoc, diag::err_module_rebuild_finalized) << ModuleName; - return false; + return nullptr; } getDiagnostics().Report(ImportLoc, diag::remark_module_build) << ModuleName << ModuleFileName; + SmallString<0> Buffer; + // Execute the action to actually build the module in-place. Use a separate // thread so that we get a stack large enough. bool Crashed = !llvm::CrashRecoveryContext().RunSafelyOnNewStack( [&]() { + auto OS = std::make_unique<llvm::raw_svector_ostream>(Buffer); + std::unique_ptr<FrontendAction> Action = - std::make_unique<GenerateModuleFromModuleMapAction>(); + std::make_unique<GenerateModuleFromModuleMapAction>(std::move(OS)); if (auto WrapGenModuleAction = Instance.getGenModuleActionWrapper()) Action = WrapGenModuleAction(Instance.getFrontendOpts(), @@ -1297,8 +1301,11 @@ bool CompilerInstance::compileModule(SourceLocation ImportLoc, // If \p AllowPCMWithCompilerErrors is set return 'success' even if errors // occurred. - return !Instance.getDiagnostics().hasErrorOccurred() || - Instance.getFrontendOpts().AllowPCMWithCompilerErrors; + if (!Instance.getDiagnostics().hasErrorOccurred() || + Instance.getFrontendOpts().AllowPCMWithCompilerErrors) + return llvm::MemoryBuffer::getMemBufferCopy( + Buffer, Instance.getFrontendOpts().OutputFile); + return nullptr; } static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File, @@ -1440,13 +1447,17 @@ static bool compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, ModuleFileName ModuleFileName) { + std::unique_ptr<llvm::MemoryBuffer> Buffer; + { auto Instance = ImportingInstance.cloneForModuleCompile( ModuleNameLoc, Module, ModuleFileName); - if (!ImportingInstance.compileModule(ModuleNameLoc, + Buffer = ImportingInstance.compileModule(ModuleNameLoc, Module->getTopLevelModuleName(), - ModuleFileName, *Instance)) { + ModuleFileName, *Instance); + + if (!Buffer) { ImportingInstance.getDiagnostics().Report(ModuleNameLoc, diag::err_module_not_built) << Module->Name << SourceRange(ImportLoc, ModuleNameLoc); @@ -1454,6 +1465,12 @@ static bool compileModuleImpl(CompilerInstance &ImportingInstance, } } + time_t ModTime = + ImportingInstance.getModuleCache().write(ModuleFileName, *Buffer); + + // TODO: Write to in-memory module cache here. + (void)ModTime; + // The module is built successfully, we can update its timestamp now. if (ImportingInstance.getPreprocessor() .getHeaderSearchInfo() @@ -2195,8 +2212,9 @@ void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc, // output is nondeterministic (as .pcm files refer to each other by name). // Can this affect the output in any way? SmallString<128> ModuleFileName; + int FD; if (std::error_code EC = llvm::sys::fs::createTemporaryFile( - CleanModuleName, "pcm", ModuleFileName)) { + CleanModuleName, "pcm", FD, ModuleFileName)) { getDiagnostics().Report(ImportLoc, diag::err_fe_unable_to_open_output) << ModuleFileName << EC.message(); return; @@ -2224,12 +2242,14 @@ void CompilerInstance::createModuleFromSource(SourceLocation ImportLoc, Other->DeleteBuiltModules = false; // Build the module, inheriting any modules that we've built locally. - bool Success = compileModule(ImportLoc, ModuleName, ModuleFileName, *Other); - + std::unique_ptr<llvm::MemoryBuffer> Buffer = + compileModule(ImportLoc, ModuleName, ModuleFileName, *Other); + llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true); BuiltModules = std::move(Other->BuiltModules); - if (Success) { + if (Buffer) { BuiltModules[std::string(ModuleName)] = std::string(ModuleFileName); + OS.write(Buffer->getBufferStart(), Buffer->getBufferSize()); llvm::sys::RemoveFileOnSignal(ModuleFileName); } } diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index e5eaab0da7adb..42f1ae3d83ed3 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -188,7 +188,8 @@ bool GeneratePCHAction::BeginSourceFileAction(CompilerInstance &CI) { std::vector<std::unique_ptr<ASTConsumer>> GenerateModuleAction::CreateMultiplexConsumer(CompilerInstance &CI, StringRef InFile) { - std::unique_ptr<raw_pwrite_stream> OS = CreateOutputFile(CI, InFile); + if (!OS) + OS = CreateOutputFile(CI, InFile); if (!OS) return {}; diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 658da6e3b7145..718d6c99eff07 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -101,6 +101,32 @@ void clang::maybePruneImpl(StringRef Path, time_t PruneInterval, } } +time_t clang::writeImpl(StringRef Path, llvm::MemoryBufferRef Buffer) { + StringRef Extension = llvm::sys::path::extension(Path); + SmallString<128> ModelPath = + StringRef(Path).drop_back(Extension.size()); + ModelPath += "-%%%%%%%%"; + ModelPath += Extension; + ModelPath += ".tmp"; + + auto ModTime = std::chrono::system_clock::now(); + + SmallString<128> TmpPath; + int FD; + if (llvm::sys::fs::createUniqueFile(ModelPath, FD, TmpPath)) + return 0; + + { + llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true); + OS << Buffer.getBuffer(); + llvm::sys::fs::setLastAccessAndModificationTime(FD, ModTime); + } + + llvm::sys::fs::rename(TmpPath, Path); + + return std::chrono::system_clock::to_time_t(ModTime); +} + namespace { class CrossProcessModuleCache : public ModuleCache { InMemoryModuleCache InMemory; @@ -157,6 +183,13 @@ class CrossProcessModuleCache : public ModuleCache { maybePruneImpl(Path, PruneInterval, PruneAfter); } + time_t write(StringRef Path, llvm::MemoryBufferRef Buffer) override { + // This is a compiler-internal input/output, let's bypass the sandbox. + auto BypassSandbox = llvm::sys::sandbox::scopedDisable(); + + return writeImpl(Path, Buffer); + } + InMemoryModuleCache &getInMemoryModuleCache() override { return InMemory; } const InMemoryModuleCache &getInMemoryModuleCache() const override { return InMemory; >From b64dcd6706c2aaa70e8b562239dcdb167d44151b Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 20 Mar 2026 21:09:27 -0700 Subject: [PATCH 6/7] Write ModTime to in-memory module cache --- clang/include/clang/Serialization/ASTWriter.h | 5 +--- .../clang/Serialization/InMemoryModuleCache.h | 14 ++++++--- clang/lib/Frontend/CompilerInstance.cpp | 4 +-- clang/lib/Serialization/ASTWriter.cpp | 8 +---- clang/lib/Serialization/GeneratePCH.cpp | 12 +++----- .../lib/Serialization/InMemoryModuleCache.cpp | 18 ++++++++++-- clang/lib/Serialization/ModuleManager.cpp | 29 +++++-------------- .../Serialization/InMemoryModuleCacheTest.cpp | 20 ++++++------- 8 files changed, 51 insertions(+), 59 deletions(-) diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index be2fbdf1ade83..7b8376ada731e 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -723,8 +723,7 @@ class ASTWriter : public ASTDeserializationListener, /// the module but currently is merely a random 32-bit number. ASTFileSignature WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject, StringRef OutputFile, Module *WritingModule, - StringRef isysroot, - bool ShouldCacheASTInMemory = false); + StringRef isysroot); /// Emit a token. void AddToken(const Token &Tok, RecordDataImpl &Record); @@ -1002,7 +1001,6 @@ class PCHGenerator : public SemaConsumer { llvm::BitstreamWriter Stream; ASTWriter Writer; bool AllowASTWithErrors; - bool ShouldCacheASTInMemory; protected: ASTWriter &getWriter() { return Writer; } @@ -1024,7 +1022,6 @@ class PCHGenerator : public SemaConsumer { ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions, bool AllowASTWithErrors = false, bool IncludeTimestamps = true, bool BuildingImplicitModule = false, - bool ShouldCacheASTInMemory = false, bool GeneratingReducedBMI = false); ~PCHGenerator() override; diff --git a/clang/include/clang/Serialization/InMemoryModuleCache.h b/clang/include/clang/Serialization/InMemoryModuleCache.h index fc3ba334fc64d..170aafcc4741c 100644 --- a/clang/include/clang/Serialization/InMemoryModuleCache.h +++ b/clang/include/clang/Serialization/InMemoryModuleCache.h @@ -30,14 +30,16 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> { struct PCM { std::unique_ptr<llvm::MemoryBuffer> Buffer; + time_t ModTime; + /// Track whether this PCM is known to be good (either built or /// successfully imported by a CompilerInstance/ASTReader using this /// cache). bool IsFinal = false; PCM() = default; - PCM(std::unique_ptr<llvm::MemoryBuffer> Buffer) - : Buffer(std::move(Buffer)) {} + PCM(std::unique_ptr<llvm::MemoryBuffer> Buffer, time_t ModTime) + : Buffer(std::move(Buffer)), ModTime(ModTime) {} }; /// Cache of buffers. @@ -64,7 +66,8 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> { /// \post state is Tentative /// \return a reference to the buffer as a convenience. llvm::MemoryBuffer &addPCM(llvm::StringRef Filename, - std::unique_ptr<llvm::MemoryBuffer> Buffer); + std::unique_ptr<llvm::MemoryBuffer> Buffer, + time_t ModTime); /// Store a just-built PCM under the Filename. /// @@ -72,7 +75,8 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> { /// \pre state is not Tentative. /// \return a reference to the buffer as a convenience. llvm::MemoryBuffer &addBuiltPCM(llvm::StringRef Filename, - std::unique_ptr<llvm::MemoryBuffer> Buffer); + std::unique_ptr<llvm::MemoryBuffer> Buffer, + time_t ModTime); /// Try to remove a buffer from the cache. No effect if state is Final. /// @@ -90,6 +94,8 @@ class InMemoryModuleCache : public llvm::RefCountedBase<InMemoryModuleCache> { /// Get a pointer to the pCM if it exists; else nullptr. llvm::MemoryBuffer *lookupPCM(llvm::StringRef Filename) const; + const std::time_t *lookupPCMModTime(llvm::StringRef Filename) const; + /// Check whether the PCM is final and has been shown to work. /// /// \return true iff state is Final. diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 7647f80c2f96a..122df21338175 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1468,8 +1468,8 @@ static bool compileModuleImpl(CompilerInstance &ImportingInstance, time_t ModTime = ImportingInstance.getModuleCache().write(ModuleFileName, *Buffer); - // TODO: Write to in-memory module cache here. - (void)ModTime; + ImportingInstance.getModuleCache().getInMemoryModuleCache().addBuiltPCM( + ModuleFileName, std::move(Buffer), ModTime); // The module is built successfully, we can update its timestamp now. if (ImportingInstance.getPreprocessor() diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 2b0808ad5ad5e..e6f167ffd5564 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5495,7 +5495,7 @@ time_t ASTWriter::getTimestampForOutput(time_t ModTime) const { ASTFileSignature ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject, StringRef OutputFile, Module *WritingModule, - StringRef isysroot, bool ShouldCacheASTInMemory) { + StringRef isysroot) { llvm::TimeTraceScope scope("WriteAST", OutputFile); WritingAST = true; @@ -5522,12 +5522,6 @@ ASTWriter::WriteAST(llvm::PointerUnion<Sema *, Preprocessor *> Subject, WritingAST = false; - if (ShouldCacheASTInMemory) { - // Construct MemoryBuffer and update buffer manager. - ModCache.getInMemoryModuleCache().addBuiltPCM( - OutputFile, llvm::MemoryBuffer::getMemBufferCopy( - StringRef(Buffer.begin(), Buffer.size()))); - } return Signature; } diff --git a/clang/lib/Serialization/GeneratePCH.cpp b/clang/lib/Serialization/GeneratePCH.cpp index f8be0e45078db..7769d6170b845 100644 --- a/clang/lib/Serialization/GeneratePCH.cpp +++ b/clang/lib/Serialization/GeneratePCH.cpp @@ -28,14 +28,12 @@ PCHGenerator::PCHGenerator( const CodeGenOptions &CodeGenOpts, ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions, bool AllowASTWithErrors, bool IncludeTimestamps, - bool BuildingImplicitModule, bool ShouldCacheASTInMemory, - bool GeneratingReducedBMI) + bool BuildingImplicitModule,bool GeneratingReducedBMI) : PP(PP), Subject(&PP), OutputFile(OutputFile), isysroot(isysroot.str()), Buffer(std::move(Buffer)), Stream(this->Buffer->Data), Writer(Stream, this->Buffer->Data, ModCache, CodeGenOpts, Extensions, IncludeTimestamps, BuildingImplicitModule, GeneratingReducedBMI), - AllowASTWithErrors(AllowASTWithErrors), - ShouldCacheASTInMemory(ShouldCacheASTInMemory) { + AllowASTWithErrors(AllowASTWithErrors) { this->Buffer->IsComplete = false; } @@ -84,8 +82,7 @@ void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) { if (AllowASTWithErrors) PP.getDiagnostics().getClient()->clear(); - Buffer->Signature = Writer.WriteAST(Subject, OutputFile, Module, isysroot, - ShouldCacheASTInMemory); + Buffer->Signature = Writer.WriteAST(Subject, OutputFile, Module, isysroot); Buffer->IsComplete = true; } @@ -111,8 +108,7 @@ CXX20ModulesGenerator::CXX20ModulesGenerator(Preprocessor &PP, std::make_shared<PCHBuffer>(), CodeGenOpts, /*Extensions=*/ArrayRef<std::shared_ptr<ModuleFileExtension>>(), AllowASTWithErrors, /*IncludeTimestamps=*/false, - /*BuildingImplicitModule=*/false, /*ShouldCacheASTInMemory=*/false, - GeneratingReducedBMI) {} + /*BuildingImplicitModule=*/false, GeneratingReducedBMI) {} Module *CXX20ModulesGenerator::getEmittingModule(ASTContext &Ctx) { Module *M = Ctx.getCurrentNamedModule(); diff --git a/clang/lib/Serialization/InMemoryModuleCache.cpp b/clang/lib/Serialization/InMemoryModuleCache.cpp index d35fa2a807f4d..67e8411affe62 100644 --- a/clang/lib/Serialization/InMemoryModuleCache.cpp +++ b/clang/lib/Serialization/InMemoryModuleCache.cpp @@ -23,19 +23,23 @@ InMemoryModuleCache::getPCMState(llvm::StringRef Filename) const { llvm::MemoryBuffer & InMemoryModuleCache::addPCM(llvm::StringRef Filename, - std::unique_ptr<llvm::MemoryBuffer> Buffer) { - auto Insertion = PCMs.insert(std::make_pair(Filename, std::move(Buffer))); + std::unique_ptr<llvm::MemoryBuffer> Buffer, + time_t ModTime) { + auto Insertion = + PCMs.insert(std::make_pair(Filename, PCM(std::move(Buffer), ModTime))); assert(Insertion.second && "Already has a PCM"); return *Insertion.first->second.Buffer; } llvm::MemoryBuffer & InMemoryModuleCache::addBuiltPCM(llvm::StringRef Filename, - std::unique_ptr<llvm::MemoryBuffer> Buffer) { + std::unique_ptr<llvm::MemoryBuffer> Buffer, + time_t ModTime) { auto &PCM = PCMs[Filename]; assert(!PCM.IsFinal && "Trying to override finalized PCM?"); assert(!PCM.Buffer && "Trying to override tentative PCM?"); PCM.Buffer = std::move(Buffer); + PCM.ModTime = ModTime; PCM.IsFinal = true; return *PCM.Buffer; } @@ -48,6 +52,14 @@ InMemoryModuleCache::lookupPCM(llvm::StringRef Filename) const { return I->second.Buffer.get(); } +const std::time_t * +InMemoryModuleCache::lookupPCMModTime(llvm::StringRef Filename) const { + auto I = PCMs.find(Filename); + if (I == PCMs.end()) + return nullptr; + return &I->second.ModTime; +} + bool InMemoryModuleCache::isPCMFinal(llvm::StringRef Filename) const { return getPCMState(Filename) == Final; } diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index ed7b6cf67674e..7c7f2b0dc976d 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -157,31 +157,18 @@ ModuleManager::AddModuleResult ModuleManager::addModule( llvm::MemoryBuffer *ModuleBuffer = nullptr; std::unique_ptr<llvm::MemoryBuffer> NewFileBuffer = nullptr; if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { + auto Entry = FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/false, + /*CacheFailure=*/false); // The buffer was already provided for us. ModuleBuffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( - FileName, std::move(Buffer)); + FileName, std::move(Buffer), Entry->getModificationTime()); } else if (llvm::MemoryBuffer *Buffer = getModuleCache().getInMemoryModuleCache().lookupPCM( FileName)) { ModuleBuffer = Buffer; - if (!FileName.getImplicitModuleSuffixLength()) { - // Explicitly-built PCM files maintain consistency via mtime/size - // expectations on their imports. Even if we've previously successfully - // loaded a PCM file and stored it in the in-memory module cache, that - // does not mean its mtime/size matches current importer's expectations. - // Get that information so that it can be checked below. - // FIXME: Even though this FileManager access is likely already cached, we - // should store this directly in the in-memory module cache. - OptionalFileEntryRef Entry = - FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, - /*CacheFailure=*/false); - if (!Entry) { - ErrorStr = "module file not found"; - return Missing; - } - ModTime = Entry->getModificationTime(); - Size = Entry->getSize(); - } + Size = Buffer->getBufferSize(); + ModTime = + *getModuleCache().getInMemoryModuleCache().lookupPCMModTime(FileName); } else if (getModuleCache().getInMemoryModuleCache().shouldBuildPCM( FileName)) { // Report that the module is out of date, since we tried (and failed) to @@ -242,8 +229,8 @@ ModuleManager::AddModuleResult ModuleManager::addModule( return OutOfDate; if (NewFileBuffer) - getModuleCache().getInMemoryModuleCache().addPCM(FileName, - std::move(NewFileBuffer)); + getModuleCache().getInMemoryModuleCache().addPCM( + FileName, std::move(NewFileBuffer), ModTime); // We're keeping this module. Store it in the map. Module = Modules[*FileKey] = NewModule.get(); diff --git a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp index ed5e1538eba74..3cf63f6b77701 100644 --- a/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp +++ b/clang/unittests/Serialization/InMemoryModuleCacheTest.cpp @@ -39,15 +39,15 @@ TEST(InMemoryModuleCacheTest, addPCM) { auto *RawB = B.get(); InMemoryModuleCache Cache; - EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B))); + EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B), 0)); EXPECT_EQ(InMemoryModuleCache::Tentative, Cache.getPCMState("B")); EXPECT_EQ(RawB, Cache.lookupPCM("B")); EXPECT_FALSE(Cache.isPCMFinal("B")); EXPECT_FALSE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); - EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)), + EXPECT_DEATH(Cache.addPCM("B", getBuffer(2), 0), "Already has a PCM"); + EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2), 0), "Trying to override tentative PCM"); #endif } @@ -57,15 +57,15 @@ TEST(InMemoryModuleCacheTest, addBuiltPCM) { auto *RawB = B.get(); InMemoryModuleCache Cache; - EXPECT_EQ(RawB, &Cache.addBuiltPCM("B", std::move(B))); + EXPECT_EQ(RawB, &Cache.addBuiltPCM("B", std::move(B), 0)); EXPECT_EQ(InMemoryModuleCache::Final, Cache.getPCMState("B")); EXPECT_EQ(RawB, Cache.lookupPCM("B")); EXPECT_TRUE(Cache.isPCMFinal("B")); EXPECT_FALSE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); - EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2)), + EXPECT_DEATH(Cache.addPCM("B", getBuffer(2), 0), "Already has a PCM"); + EXPECT_DEATH(Cache.addBuiltPCM("B", getBuffer(2), 0), "Trying to override finalized PCM"); #endif } @@ -79,7 +79,7 @@ TEST(InMemoryModuleCacheTest, tryToDropPCM) { InMemoryModuleCache Cache; EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); - EXPECT_EQ(RawB1, &Cache.addPCM("B", std::move(B1))); + EXPECT_EQ(RawB1, &Cache.addPCM("B", std::move(B1), 0)); EXPECT_FALSE(Cache.tryToDropPCM("B")); EXPECT_EQ(nullptr, Cache.lookupPCM("B")); EXPECT_EQ(InMemoryModuleCache::ToBuild, Cache.getPCMState("B")); @@ -87,14 +87,14 @@ TEST(InMemoryModuleCacheTest, tryToDropPCM) { EXPECT_TRUE(Cache.shouldBuildPCM("B")); #if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST - EXPECT_DEATH(Cache.addPCM("B", getBuffer(2)), "Already has a PCM"); + EXPECT_DEATH(Cache.addPCM("B", getBuffer(2), 0), "Already has a PCM"); EXPECT_DEATH(Cache.tryToDropPCM("B"), "PCM to remove is scheduled to be built"); EXPECT_DEATH(Cache.finalizePCM("B"), "Trying to finalize a dropped PCM"); #endif // Add a new one. - EXPECT_EQ(RawB2, &Cache.addBuiltPCM("B", std::move(B2))); + EXPECT_EQ(RawB2, &Cache.addBuiltPCM("B", std::move(B2), 0)); EXPECT_TRUE(Cache.isPCMFinal("B")); // Can try to drop again, but this should error and do nothing. @@ -108,7 +108,7 @@ TEST(InMemoryModuleCacheTest, finalizePCM) { InMemoryModuleCache Cache; EXPECT_EQ(InMemoryModuleCache::Unknown, Cache.getPCMState("B")); - EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B))); + EXPECT_EQ(RawB, &Cache.addPCM("B", std::move(B), 0)); // Call finalize. Cache.finalizePCM("B"); >From 39dfef45b5c6b389e42f4a927235571c8f3d49d6 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Sat, 21 Mar 2026 10:54:10 -0700 Subject: [PATCH 7/7] Foo --- clang/lib/Frontend/CompilerInstance.cpp | 20 ++++++++++++++++---- clang/lib/Serialization/ModuleCache.cpp | 19 ++++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 122df21338175..8d6c4a17a1f06 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1465,11 +1465,23 @@ static bool compileModuleImpl(CompilerInstance &ImportingInstance, } } - time_t ModTime = - ImportingInstance.getModuleCache().write(ModuleFileName, *Buffer); + { + auto Out = ImportingInstance.getOutputManager().createFile(ModuleFileName.str()); + if (!Out) { + llvm::consumeError(Out.takeError()); + return false; + } + *Out << Buffer->getBuffer(); + if (llvm::Error Err = Out->keep()) { + llvm::consumeError(Out.takeError()); + return false; + } + } + // time_t ModTime = + // ImportingInstance.getModuleCache().write(ModuleFileName, *Buffer); - ImportingInstance.getModuleCache().getInMemoryModuleCache().addBuiltPCM( - ModuleFileName, std::move(Buffer), ModTime); + // ImportingInstance.getModuleCache().getInMemoryModuleCache().addBuiltPCM( + // ModuleFileName, std::move(Buffer), 0); // The module is built successfully, we can update its timestamp now. if (ImportingInstance.getPreprocessor() diff --git a/clang/lib/Serialization/ModuleCache.cpp b/clang/lib/Serialization/ModuleCache.cpp index 718d6c99eff07..3f5915a717d69 100644 --- a/clang/lib/Serialization/ModuleCache.cpp +++ b/clang/lib/Serialization/ModuleCache.cpp @@ -111,18 +111,31 @@ time_t clang::writeImpl(StringRef Path, llvm::MemoryBufferRef Buffer) { auto ModTime = std::chrono::system_clock::now(); + if (llvm::sys::fs::create_directories(llvm::sys::path::parent_path(ModelPath))) { + llvm::errs() << "failed to create_directories in clang::writeImpl\n"; + return 0; + } + SmallString<128> TmpPath; int FD; - if (llvm::sys::fs::createUniqueFile(ModelPath, FD, TmpPath)) + if (llvm::sys::fs::createUniqueFile(ModelPath, FD, TmpPath)) { + llvm::errs() << "failed to createUniqueFile in clang::writeImpl\n"; return 0; + } { llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true); OS << Buffer.getBuffer(); - llvm::sys::fs::setLastAccessAndModificationTime(FD, ModTime); + if (llvm::sys::fs::setLastAccessAndModificationTime(FD, ModTime)) { + llvm::errs() << "failed to setLastAccessAndModificationTime in clang::writeImpl\n"; + return 0; + } } - llvm::sys::fs::rename(TmpPath, Path); + if (llvm::sys::fs::rename(TmpPath, Path)) { + llvm::errs() << "failed to rename in clang::writeImpl\n"; + return 0; + } return std::chrono::system_clock::to_time_t(ModTime); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
