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/4] [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/4] 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/4] 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/4] 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
