[clang-tools-extra] [clangd] Invalidate preamble when new module imports are added (PR #199460)
Berkay Sahin via cfe-commits
cfe-commits at lists.llvm.org
Sun May 24 16:36:19 PDT 2026
https://github.com/berkaysahiin created https://github.com/llvm/llvm-project/pull/199460
When using `SkipPreambleBuild`, adding a new `import` statement to a file did
not invalidate the existing preamble because `isPreambleCompatible` only
checked whether existing prerequisite modules were up-to-date, not whether
the set of required modules itself had changed.
Fixes: #199389
Partially fixes: #126350
>From d9a83f3220811a4bade49807b83ddb3e8d69971d Mon Sep 17 00:00:00 2001
From: Berkay Sahin <berkaysahindev at gmail.com>
Date: Mon, 25 May 2026 02:18:13 +0300
Subject: [PATCH 1/2] [clangd] Invalidate preamble when new module imports are
added
When using SkipPreambleBuild, adding a new `import` statement to a file
did not invalidate the existing preamble because `isPreambleCompatible`
only checked whether existing prerequisite modules were up-to-date, not
whether the set of required modules itself had changed
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 25 +++++++++
clang-tools-extra/clangd/ModulesBuilder.h | 7 +++
clang-tools-extra/clangd/Preamble.cpp | 12 +++-
.../unittests/PrerequisiteModulesTest.cpp | 56 +++++++++++++++++++
4 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 706fd459e15ec..2fe4b3b278a8f 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -16,6 +16,7 @@
#include "clang/Serialization/ModuleCache.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/LockFileManager.h"
@@ -322,6 +323,8 @@ class FailedPrerequisiteModules : public PrerequisiteModules {
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>) const override {
return false;
}
+
+ llvm::StringSet<> getRequiredModuleNames() const override { return {}; }
};
/// Represents a reference to a module file (*.pcm).
@@ -502,10 +505,21 @@ class ReusablePrerequisiteModules : public PrerequisiteModules {
RequiredModules.emplace_back(std::move(MF));
}
+ void setDirectModuleNames(std::vector<std::string> Names) {
+ for (auto &Name : Names)
+ DirectModuleNames.insert(std::move(Name));
+ }
+
+ llvm::StringSet<> getRequiredModuleNames() const override {
+ return DirectModuleNames;
+ }
+
private:
llvm::SmallVector<std::shared_ptr<const ModuleFile>, 8> RequiredModules;
// A helper class to speedup the query if a module is built.
llvm::StringSet<> BuiltModuleNames;
+ // The directly required module names as scanned from the source file.
+ llvm::StringSet<> DirectModuleNames;
};
bool IsModuleFileUpToDate(PathRef ModuleFilePath,
@@ -1228,6 +1242,16 @@ bool ModulesBuilder::hasRequiredModules(PathRef File) {
return !CachedMDB.getRequiredModules(File).empty();
}
+std::vector<std::string> ModulesBuilder::getRequiredModuleNames(PathRef File) {
+ std::unique_ptr<ProjectModules> MDB = Impl->getCDB().getProjectModules(File);
+ if (!MDB)
+ return {};
+
+ CachingProjectModules CachedMDB(std::move(MDB),
+ Impl->getProjectModulesCache());
+ return CachedMDB.getRequiredModules(File);
+}
+
std::unique_ptr<PrerequisiteModules>
ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
const ThreadsafeFS &TFS) {
@@ -1245,6 +1269,7 @@ ModulesBuilder::buildPrerequisiteModulesFor(PathRef File,
return std::make_unique<ReusablePrerequisiteModules>();
auto RequiredModules = std::make_unique<ReusablePrerequisiteModules>();
+ RequiredModules->setDirectModuleNames(RequiredModuleNames);
for (llvm::StringRef RequiredModuleName : RequiredModuleNames) {
// Return early if there is any error.
if (llvm::Error Err = Impl->getOrBuildModuleFile(
diff --git a/clang-tools-extra/clangd/ModulesBuilder.h b/clang-tools-extra/clangd/ModulesBuilder.h
index b0e110b92b6a7..d8c8c446fd592 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.h
+++ b/clang-tools-extra/clangd/ModulesBuilder.h
@@ -26,6 +26,7 @@
#include "support/ThreadsafeFS.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringSet.h"
#include <memory>
namespace clang {
@@ -76,6 +77,9 @@ class PrerequisiteModules {
canReuse(const CompilerInvocation &CI,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>) const = 0;
+ /// Returns the set of directly required module names
+ virtual llvm::StringSet<> getRequiredModuleNames() const = 0;
+
virtual ~PrerequisiteModules() = default;
};
@@ -99,6 +103,9 @@ class ModulesBuilder {
bool hasRequiredModules(PathRef File);
+ /// Returns the list of directly required module names
+ std::vector<std::string> getRequiredModuleNames(PathRef File);
+
private:
class ModulesBuilderImpl;
std::unique_ptr<ModulesBuilderImpl> Impl;
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 58da7edcf3b93..10e0c5eaef47c 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -728,11 +728,21 @@ bool isPreambleCompatible(const PreambleData &Preamble,
auto Bounds = computePreambleBounds(CI.getLangOpts(), *ContentsBuffer,
Inputs.Opts.SkipPreambleBuild);
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
+ auto ModuleNamesAreEqual = [&]() -> bool {
+ if (!Inputs.ModulesManager || !Preamble.RequiredModules)
+ return true;
+ auto NewNames = Inputs.ModulesManager->getRequiredModuleNames(FileName);
+ llvm::StringSet<> NewNameSet;
+ for (const auto &Name : NewNames)
+ NewNameSet.insert(Name);
+ return NewNameSet == Preamble.RequiredModules->getRequiredModuleNames();
+ };
return compileCommandsAreEqual(Inputs.CompileCommand,
Preamble.CompileCommand) &&
Preamble.Preamble.CanReuse(CI, *ContentsBuffer, Bounds, *VFS) &&
(!Preamble.RequiredModules ||
- Preamble.RequiredModules->canReuse(CI, VFS));
+ Preamble.RequiredModules->canReuse(CI, VFS)) &&
+ ModuleNamesAreEqual();
}
void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) {
diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
index b88213decea44..f163e248c383a 100644
--- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
+++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
@@ -15,6 +15,7 @@
#include "CodeComplete.h"
#include "Compiler.h"
#include "ModulesBuilder.h"
+#include "Preamble.h"
#include "ProjectModules.h"
#include "TestTU.h"
#include "support/Path.h"
@@ -1613,6 +1614,61 @@ struct TypeFromHeader {};
named("TypeFromHeader")));
}
+TEST_F(PrerequisiteModulesTests,
+ SkipPreambleBuildInvalidatedByNewModuleImport) {
+ MockDirectoryCompilationDatabase CDB(TestDir, FS);
+
+ CDB.addFile("Dep.cppm", R"cpp(
+export module Dep;
+)cpp");
+
+ CDB.addFile("Consumer.cpp", R"cpp(
+import Dep;
+void use() {}
+)cpp");
+
+ ModulesBuilder Builder(CDB);
+
+ auto Inputs = getInputs("Consumer.cpp", CDB);
+ Inputs.ModulesManager = &Builder;
+ Inputs.Opts.SkipPreambleBuild = true;
+
+ auto CI = buildCompilerInvocation(Inputs, DiagConsumer);
+ ASSERT_TRUE(CI);
+
+ auto Preamble = buildPreamble(getFullPath("Consumer.cpp"), *CI, Inputs,
+ /*StoreInMemory=*/true,
+ /*PreambleCallback=*/nullptr);
+ ASSERT_TRUE(Preamble);
+ EXPECT_EQ(Preamble->Preamble.getBounds().Size, 0u);
+ ASSERT_TRUE(Preamble->RequiredModules);
+
+ CDB.addFile("NewDep.cppm", R"cpp(
+export module NewDep;
+)cpp");
+
+ // Add a new import.
+ Inputs.Contents = R"cpp(
+import Dep;
+import NewDep;
+void use() {}
+)cpp";
+
+ {
+ std::error_code EC;
+ llvm::raw_fd_ostream OS(getFullPath("Consumer.cpp"), EC,
+ llvm::sys::fs::OF_None);
+ ASSERT_FALSE(EC);
+ OS << Inputs.Contents;
+ }
+
+ auto NewCI = buildCompilerInvocation(Inputs, DiagConsumer);
+ ASSERT_TRUE(NewCI);
+
+ EXPECT_FALSE(isPreambleCompatible(*Preamble, Inputs,
+ getFullPath("Consumer.cpp"), *NewCI));
+}
+
} // namespace
} // namespace clang::clangd
>From 510ca026f4486f0d6a0bfc5f91e7941b91275d20 Mon Sep 17 00:00:00 2001
From: Berkay Sahin <berkaysahindev at gmail.com>
Date: Mon, 25 May 2026 02:30:04 +0300
Subject: [PATCH 2/2] [clangd] Prefer insert_range, rename local lambda
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 3 +--
clang-tools-extra/clangd/Preamble.cpp | 7 +++----
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 2fe4b3b278a8f..6e47122bdd82b 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -506,8 +506,7 @@ class ReusablePrerequisiteModules : public PrerequisiteModules {
}
void setDirectModuleNames(std::vector<std::string> Names) {
- for (auto &Name : Names)
- DirectModuleNames.insert(std::move(Name));
+ DirectModuleNames.insert_range(Names);
}
llvm::StringSet<> getRequiredModuleNames() const override {
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 10e0c5eaef47c..f8d05b3dc0ded 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -728,13 +728,12 @@ bool isPreambleCompatible(const PreambleData &Preamble,
auto Bounds = computePreambleBounds(CI.getLangOpts(), *ContentsBuffer,
Inputs.Opts.SkipPreambleBuild);
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
- auto ModuleNamesAreEqual = [&]() -> bool {
+ auto RequiredModulesMatch = [&]() -> bool {
if (!Inputs.ModulesManager || !Preamble.RequiredModules)
return true;
auto NewNames = Inputs.ModulesManager->getRequiredModuleNames(FileName);
llvm::StringSet<> NewNameSet;
- for (const auto &Name : NewNames)
- NewNameSet.insert(Name);
+ NewNameSet.insert_range(NewNames);
return NewNameSet == Preamble.RequiredModules->getRequiredModuleNames();
};
return compileCommandsAreEqual(Inputs.CompileCommand,
@@ -742,7 +741,7 @@ bool isPreambleCompatible(const PreambleData &Preamble,
Preamble.Preamble.CanReuse(CI, *ContentsBuffer, Bounds, *VFS) &&
(!Preamble.RequiredModules ||
Preamble.RequiredModules->canReuse(CI, VFS)) &&
- ModuleNamesAreEqual();
+ RequiredModulesMatch();
}
void escapeBackslashAndQuotes(llvm::StringRef Text, llvm::raw_ostream &OS) {
More information about the cfe-commits
mailing list