[clang-tools-extra] [clangd] Invalidate preamble when new module imports are added (PR #199460)
via cfe-commits
cfe-commits at lists.llvm.org
Sun May 24 16:36:56 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Berkay Sahin (berkaysahiin)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/199460.diff
4 Files Affected:
- (modified) clang-tools-extra/clangd/ModulesBuilder.cpp (+24)
- (modified) clang-tools-extra/clangd/ModulesBuilder.h (+7)
- (modified) clang-tools-extra/clangd/Preamble.cpp (+10-1)
- (modified) clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp (+56)
``````````diff
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 706fd459e15ec..6e47122bdd82b 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,20 @@ class ReusablePrerequisiteModules : public PrerequisiteModules {
RequiredModules.emplace_back(std::move(MF));
}
+ void setDirectModuleNames(std::vector<std::string> Names) {
+ DirectModuleNames.insert_range(Names);
+ }
+
+ 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 +1241,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 +1268,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..f8d05b3dc0ded 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -728,11 +728,20 @@ bool isPreambleCompatible(const PreambleData &Preamble,
auto Bounds = computePreambleBounds(CI.getLangOpts(), *ContentsBuffer,
Inputs.Opts.SkipPreambleBuild);
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
+ auto RequiredModulesMatch = [&]() -> bool {
+ if (!Inputs.ModulesManager || !Preamble.RequiredModules)
+ return true;
+ auto NewNames = Inputs.ModulesManager->getRequiredModuleNames(FileName);
+ llvm::StringSet<> NewNameSet;
+ NewNameSet.insert_range(NewNames);
+ 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)) &&
+ RequiredModulesMatch();
}
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/199460
More information about the cfe-commits
mailing list