https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/185994
>From d175c7157a0b6f74471d9711d216c7c6c5f3e6b0 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Thu, 12 Mar 2026 15:33:01 -0700 Subject: [PATCH 1/2] [clang][modules] Remove `Module::ASTFile` --- .../ExpandModularHeadersPPCallbacks.cpp | 2 +- clang/include/clang/Basic/Module.h | 29 +++++++++++++------ .../DependencyScanning/ModuleDepCollector.h | 2 +- clang/lib/Basic/ASTSourceDescriptor.cpp | 4 +-- .../DependencyScanning/ModuleDepCollector.cpp | 6 ++-- clang/lib/Frontend/CompilerInstance.cpp | 10 +++---- clang/lib/Index/IndexingAction.cpp | 3 +- clang/lib/Sema/SemaModule.cpp | 4 +-- clang/lib/Serialization/ASTReader.cpp | 17 ++++++----- clang/lib/Serialization/ModuleManager.cpp | 8 ++--- clang/test/Modules/DebugInfoSubmoduleImport.c | 2 +- .../test/Modules/DebugInfoTransitiveImport.m | 2 +- clang/tools/libclang/CIndex.cpp | 7 +---- clang/tools/libclang/CXIndexDataConsumer.cpp | 6 +++- 14 files changed, 55 insertions(+), 47 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 d2aea2f09353b..a524443c73996 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -365,9 +365,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 and key 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; @@ -840,15 +841,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; } - /// 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; + /// 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 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/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 ca0bcd2dc2ee3..33919bc1c4634 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/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 e9c4ff3a75801..03b1b02859b81 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -4652,10 +4652,10 @@ 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. @@ -6339,15 +6339,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); @@ -6361,7 +6362,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/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index d0bf4bc74e8ab..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; } @@ -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 93b085d1616bda15ea262eef683cdc023971ce71 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Fri, 13 Mar 2026 13:51:00 -0700 Subject: [PATCH 2/2] 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 e1adaba4be7fc..129518ab2d018 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -499,6 +499,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
