[clang-tools-extra] [clangd] Fix prerequisite module stale dependency detection (PR #234181)
SLIMANI Lukas via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 06:51:01 PDT 2026
https://github.com/lslimani updated https://github.com/llvm/llvm-project/pull/234181
>From a43f9dcbc5248d7d6892ee1645cdc24425123ab9 Mon Sep 17 00:00:00 2001
From: lslimani <lukas.slimani at gmail.com>
Date: Wed, 21 Oct 2026 16:01:30 +0200
Subject: [PATCH] [clangd] Fix prerequisite module stale dependency detection
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 55 ++++-
.../unittests/PrerequisiteModulesTest.cpp | 192 ++++++++++++++++++
2 files changed, 244 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 72bb5b836d2012..801f4a8624bba0 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -1157,6 +1157,14 @@ class ModulesBuilder::ModulesBuilderImpl {
/// Runs GC once for the cache root owning a project root.
void garbageCollectModuleCacheForProjectRoot(PathRef ProjectRoot);
+ /// Returns true if the published module file of a direct dependency of
+ /// `ModuleName` is newer than the published module file of `ModuleName`
+ /// itself. C++20 named modules don't record the signatures of the modules
+ /// they import, so the AST reader can't detect this kind of staleness.
+ bool hasNewerDependency(StringRef ModuleName, PathRef PublishedModuleFilePath,
+ PathRef RequiredSource, PathRef ModuleUnitFileName,
+ CachingProjectModules &MDB);
+
ModuleFileCache Cache;
ObservedModuleFiles ObservedFiles;
ModuleNameToSourceCache ProjectModulesCache;
@@ -1235,6 +1243,39 @@ void ModulesBuilder::ModulesBuilderImpl::getPrebuiltModuleFile(
}
}
+bool ModulesBuilder::ModulesBuilderImpl::hasNewerDependency(
+ StringRef ModuleName, PathRef PublishedModuleFilePath,
+ PathRef RequiredSource, PathRef ModuleUnitFileName,
+ CachingProjectModules &MDB) {
+ std::vector<std::string> Deps = MDB.getRequiredModules(ModuleUnitFileName);
+ if (Deps.empty())
+ return false;
+
+ llvm::sys::fs::file_status Status;
+ // Without a published module file there is nothing to compare against, so
+ // treat any cached copy as stale.
+ if (llvm::sys::fs::status(PublishedModuleFilePath, Status))
+ return true;
+ const auto ModuleTime = Status.getLastModificationTime();
+
+ return llvm::any_of(Deps, [&](const std::string &Dep) {
+ std::string DepSource = MDB.getSourceForModuleName(Dep, RequiredSource);
+ if (DepSource.empty())
+ return false;
+ auto DepCmd = getCDB().getCompileCommand(DepSource);
+ if (!DepCmd)
+ return false;
+ const std::string DepPublishedPath = getPublishedModuleFilePath(
+ Dep, getModuleFilesDirectory(DepSource, *DepCmd, getCDB()));
+ llvm::sys::fs::file_status DepStatus;
+ // Dependencies that clangd didn't publish, like module files from the
+ // build system, are validated by the AST reader instead.
+ if (llvm::sys::fs::status(DepPublishedPath, DepStatus))
+ return false;
+ return DepStatus.getLastModificationTime() > ModuleTime;
+ });
+}
+
llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
PathRef RequiredSource, StringRef ModuleName, const ThreadsafeFS &TFS,
CachingProjectModules &MDB, ReusablePrerequisiteModules &BuiltModuleFiles) {
@@ -1267,6 +1308,7 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
std::string ReqFileName =
MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
+
auto Cmd = getCDB().getCompileCommand(ReqFileName);
if (!Cmd)
return llvm::createStringError(
@@ -1284,11 +1326,15 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
auto SourceLock = ScopedModuleSourceLock::acquire(ReqFileName, getCDB());
if (!SourceLock)
return SourceLock.takeError();
+
+ const bool DependencyRebuilt =
+ hasNewerDependency(ReqModuleName, PublishedModuleFilePath,
+ RequiredSource, ReqFileName, MDB);
std::shared_ptr<const ModuleFile> Cached =
Cache.getModule(ReqModuleName, ReqFileName, CommandHash);
- if (Cached) {
+ if (Cached && !DependencyRebuilt) {
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
TFS.view(std::nullopt))) {
log("Reusing module {0} from {1}", ReqModuleName,
@@ -1296,11 +1342,14 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
BuiltModuleFiles.addModuleFile(std::move(Cached));
continue;
}
- Cache.remove(ReqModuleName, ReqFileName, CommandHash);
}
+ if (Cached)
+ Cache.remove(ReqModuleName, ReqFileName, CommandHash);
+
if (llvm::sys::fs::exists(PublishedModuleFilePath)) {
- if (IsModuleFileUpToDate(PublishedModuleFilePath, BuiltModuleFiles,
+ if (!DependencyRebuilt &&
+ IsModuleFileUpToDate(PublishedModuleFilePath, BuiltModuleFiles,
TFS.view(std::nullopt))) {
log("Reusing persistent module {0} from {1}", ReqModuleName,
PublishedModuleFilePath);
diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
index e878e2e0fa637b..9d2e0da4d0dd95 100644
--- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
+++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
@@ -1802,6 +1802,198 @@ export struct TypeFromModule {};
EXPECT_TRUE(HasToken("export", HighlightingKind::Modifier));
}
+TEST_F(PrerequisiteModulesTests, DetectsStaleTransitivePrerequisite) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline constexpr int a = 1;
+)cpp");
+
+ CDB.addFile("M.cppm", R"cpp(
+export module M;
+import A;
+export inline constexpr int m = a + 1;
+)cpp");
+
+ CDB.addFile("Use.cpp", R"cpp(
+import M;
+int use() { return m; }
+)cpp");
+
+ ModulesBuilder Builder(CDB);
+
+ auto Info = Builder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS);
+ ASSERT_TRUE(Info);
+
+ auto Inputs = getInputs("Use.cpp", CDB);
+ auto Invocation = buildCompilerInvocation(Inputs, DiagConsumer);
+ ASSERT_TRUE(Invocation);
+
+ auto VFS = FS.view(TestDir);
+
+ // Initially, all prerequisite modules are valid.
+ EXPECT_TRUE(Info->canReuse(*Invocation, VFS));
+
+ // Repeated checks should not change the result.
+ EXPECT_TRUE(Info->canReuse(*Invocation, VFS));
+ EXPECT_TRUE(Info->canReuse(*Invocation, VFS));
+
+ // Change the transitive prerequisite without changing M or Use.cpp.
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline constexpr int a = 42;
+)cpp");
+
+ // The existing prerequisite information must not be reused if
+ // A's module file is now stale, even if M's own AST is readable.
+ EXPECT_FALSE(Info->canReuse(*Invocation, VFS));
+
+ // Rebuild the prerequisite information and check that it is reusable
+ // again after the module dependency graph has been refreshed.
+ auto RefreshedInfo =
+ Builder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS);
+ ASSERT_TRUE(RefreshedInfo);
+ EXPECT_TRUE(RefreshedInfo->canReuse(*Invocation, VFS));
+
+ // M depends on A, which was rebuilt, so M must be rebuilt too rather than
+ // reusing its stale cached module file.
+ HeaderSearchOptions OldHSOpts, NewHSOpts;
+ Info->adjustHeaderSearchOptions(OldHSOpts);
+ RefreshedInfo->adjustHeaderSearchOptions(NewHSOpts);
+ ASSERT_EQ(OldHSOpts.PrebuiltModuleFiles.count("M"), 1u);
+ ASSERT_EQ(NewHSOpts.PrebuiltModuleFiles.count("M"), 1u);
+ EXPECT_NE(OldHSOpts.PrebuiltModuleFiles["M"],
+ NewHSOpts.PrebuiltModuleFiles["M"]);
+
+ // An unrelated module must not invalidate Use.cpp's prerequisites.
+ CDB.addFile("Unrelated.cppm", R"cpp(
+export module Unrelated;
+export inline constexpr int unrelated = 100;
+)cpp");
+
+ EXPECT_TRUE(RefreshedInfo->canReuse(*Invocation, VFS));
+}
+
+TEST_F(PrerequisiteModulesTests,
+ RebuiltDependencyInvalidatesDependentAcrossFiles) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline constexpr int a = 1;
+)cpp");
+
+ CDB.addFile("M.cppm", R"cpp(
+export module M;
+import A;
+export inline constexpr int m = a + 1;
+)cpp");
+
+ CDB.addFile("UseA.cpp", R"cpp(
+import A;
+int useA() { return a; }
+)cpp");
+
+ CDB.addFile("UseM.cpp", R"cpp(
+import M;
+int useM() { return m; }
+)cpp");
+
+ ModulesBuilder Builder(CDB);
+
+ auto InfoM = Builder.buildPrerequisiteModulesFor(getFullPath("UseM.cpp"), FS);
+ ASSERT_TRUE(InfoM);
+ HeaderSearchOptions OldHSOpts;
+ InfoM->adjustHeaderSearchOptions(OldHSOpts);
+ ASSERT_EQ(OldHSOpts.PrebuiltModuleFiles.count("M"), 1u);
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline constexpr int a = 42;
+)cpp");
+
+ // Opening a file that only imports A rebuilds A, but not M.
+ auto InfoA = Builder.buildPrerequisiteModulesFor(getFullPath("UseA.cpp"), FS);
+ ASSERT_TRUE(InfoA);
+
+ // M's cached file was built against the old A, so it must be rebuilt.
+ auto RefreshedInfoM =
+ Builder.buildPrerequisiteModulesFor(getFullPath("UseM.cpp"), FS);
+ ASSERT_TRUE(RefreshedInfoM);
+ HeaderSearchOptions NewHSOpts;
+ RefreshedInfoM->adjustHeaderSearchOptions(NewHSOpts);
+ ASSERT_EQ(NewHSOpts.PrebuiltModuleFiles.count("M"), 1u);
+ EXPECT_NE(OldHSOpts.PrebuiltModuleFiles["M"],
+ NewHSOpts.PrebuiltModuleFiles["M"]);
+}
+
+// C++20 named modules don't record the signatures of the modules they import,
+// so a persistent BMI of M built against an old A must be detected as stale by
+// clangd itself, even from a fresh builder (e.g. a new clangd process).
+TEST_F(PrerequisiteModulesTests,
+ PersistentModuleCacheRebuildsDependentOfRebuiltModuleAcrossBuilders) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline int a() { return 1; }
+)cpp");
+
+ CDB.addFile("M.cppm", R"cpp(
+export module M;
+import A;
+export inline int m() { return a() + 1; }
+)cpp");
+
+ CDB.addFile("Use.cpp", R"cpp(
+import M;
+int use() { return m(); }
+)cpp");
+
+ auto ReadModuleFile = [](const PrerequisiteModules &Info,
+ llvm::StringRef ModuleName) -> std::string {
+ HeaderSearchOptions HS;
+ Info.adjustHeaderSearchOptions(HS);
+ auto It = HS.PrebuiltModuleFiles.find(ModuleName.str());
+ if (It == HS.PrebuiltModuleFiles.end())
+ return "";
+ auto Buf = llvm::MemoryBuffer::getFile(It->second);
+ return Buf ? (*Buf)->getBuffer().str() : "";
+ };
+
+ // Keep the first builder's module files alive so their contents can be
+ // compared with the second builder's.
+ ModulesBuilder FirstBuilder(CDB);
+ auto FirstInfo =
+ FirstBuilder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS);
+ ASSERT_TRUE(FirstInfo);
+ std::string FirstM = ReadModuleFile(*FirstInfo, "M");
+ ASSERT_FALSE(FirstM.empty());
+
+ CDB.addFile("A.cppm", R"cpp(
+export module A;
+export inline int a() { return 42; }
+)cpp");
+
+ ModulesBuilder SecondBuilder(CDB);
+ auto SecondInfo =
+ SecondBuilder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS);
+ ASSERT_TRUE(SecondInfo);
+ std::string SecondM = ReadModuleFile(*SecondInfo, "M");
+ ASSERT_FALSE(SecondM.empty());
+
+ // A reused M would be a byte-for-byte copy of the first published BMI.
+ EXPECT_NE(FirstM, SecondM);
+
+ // With nothing changed, a third builder reuses the persistent M.
+ ModulesBuilder ThirdBuilder(CDB);
+ auto ThirdInfo =
+ ThirdBuilder.buildPrerequisiteModulesFor(getFullPath("Use.cpp"), FS);
+ ASSERT_TRUE(ThirdInfo);
+ EXPECT_EQ(SecondM, ReadModuleFile(*ThirdInfo, "M"));
+}
+
} // namespace
} // namespace clang::clangd
More information about the cfe-commits
mailing list