[clang-tools-extra] [clangd] [Modules] Refactor cache to support duplicated module name (PR #193413)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 21 23:03:06 PDT 2026
https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/193413
Following of https://github.com/llvm/llvm-project/pull/193158
Although https://github.com/llvm/llvm-project/pull/193158 handles duplicated module name, the cache in ModulesBuilder still uses the old assumption.
This patch tries to resolve the problem while not affecting original use cases as much as possible. As the duplicated module name should be rare in real world case.
AI Assisted
>From 9d104d34d68242fe120bd1aeaa4033f15491283d Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Wed, 22 Apr 2026 13:58:05 +0800
Subject: [PATCH] [clangd] [Modules] Refactor cache to support duplicated
module name
Following of https://github.com/llvm/llvm-project/pull/193158
Although https://github.com/llvm/llvm-project/pull/193158 handles
duplicated module name, the cache in ModulesBuilder still uses the
old assumption.
This patch tries to resolve the problem while not affecting original
use cases as much as possible. As the duplicated module name should
be rare in real world case.
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 216 ++++++++++++++----
.../unittests/PrerequisiteModulesTest.cpp | 71 ++++++
2 files changed, 245 insertions(+), 42 deletions(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index c4a0c54a8fc2f..c5a1780784524 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -92,8 +92,7 @@ class FailedPrerequisiteModules : public PrerequisiteModules {
// We shouldn't adjust the compilation commands based on
// FailedPrerequisiteModules.
- void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {
- }
+ void adjustHeaderSearchOptions(HeaderSearchOptions &Options) const override {}
// FailedPrerequisiteModules can never be reused.
bool
@@ -395,73 +394,158 @@ bool ReusablePrerequisiteModules::canReuse(
return IsModuleFilesUpToDate(BMIPaths, *this, VFS);
}
+/// A cache for built module files. Currently this can handle cases for
+/// - A module name to the corresponding built module file.
+/// - A module name and the module unit path to the corresponding built
+/// module file. This is needed as there might be cases where we have
+/// multiple module units declaring the same module name.
+///
+/// FIXME: A missing point is, technically, we need to handle the cases
+/// we have multiple built module file from the same module units with
+/// different compilation options.
class ModuleFileCache {
public:
ModuleFileCache(const GlobalCompilationDatabase &CDB) : CDB(CDB) {}
const GlobalCompilationDatabase &getCDB() const { return CDB; }
- std::shared_ptr<const ModuleFile> getModule(StringRef ModuleName);
+ std::shared_ptr<const ModuleFile>
+ getUniqueModuleForModuleName(StringRef ModuleName) {
+ std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+
+ auto Iter = ModuleNameToUniqueModuleCache.find(ModuleName);
+ if (Iter == ModuleNameToUniqueModuleCache.end())
+ return nullptr;
- void add(StringRef ModuleName, std::shared_ptr<const ModuleFile> ModuleFile) {
+ if (auto Res = Iter->second.lock())
+ return Res;
+
+ ModuleNameToUniqueModuleCache.erase(Iter);
+ return nullptr;
+ }
+
+ void addUniqueEntry(StringRef ModuleName,
+ std::shared_ptr<const ModuleFile> ModuleFile) {
std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+ ModuleNameToUniqueModuleCache[ModuleName] = ModuleFile;
+ }
- ModuleFiles[ModuleName] = ModuleFile;
+ void eraseUniqueEntry(StringRef ModuleName) {
+ std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+ ModuleNameToUniqueModuleCache.erase(ModuleName);
}
- void remove(StringRef ModuleName);
+ std::shared_ptr<const ModuleFile>
+ getMultipleModuleForModuleName(StringRef ModuleName,
+ PathRef ModuleUnitSource) {
+ std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
-private:
- const GlobalCompilationDatabase &CDB;
+ auto Outer = ModuleNameToMultipleModuleCache.find(ModuleName);
+ if (Outer == ModuleNameToMultipleModuleCache.end())
+ return nullptr;
- llvm::StringMap<std::weak_ptr<const ModuleFile>> ModuleFiles;
- // Mutex to guard accesses to ModuleFiles.
- std::mutex ModuleFilesMutex;
-};
+ auto Inner = Outer->second.find(maybeCaseFoldPath(ModuleUnitSource));
+ if (Inner == Outer->second.end())
+ return nullptr;
-std::shared_ptr<const ModuleFile>
-ModuleFileCache::getModule(StringRef ModuleName) {
- std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+ if (auto Res = Inner->second.lock())
+ return Res;
- auto Iter = ModuleFiles.find(ModuleName);
- if (Iter == ModuleFiles.end())
+ Outer->second.erase(Inner);
+ if (Outer->second.empty())
+ ModuleNameToMultipleModuleCache.erase(Outer);
return nullptr;
+ }
- if (auto Res = Iter->second.lock())
- return Res;
+ void addMultipleEntry(StringRef ModuleName, PathRef ModuleUnitSource,
+ std::shared_ptr<const ModuleFile> ModuleFile) {
+ std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+ ModuleNameToMultipleModuleCache[ModuleName]
+ [maybeCaseFoldPath(ModuleUnitSource)] =
+ ModuleFile;
+ }
- ModuleFiles.erase(Iter);
- return nullptr;
-}
+ void eraseMultipleEntry(StringRef ModuleName, PathRef ModuleUnitSource) {
+ std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+ auto Outer = ModuleNameToMultipleModuleCache.find(ModuleName);
+ if (Outer == ModuleNameToMultipleModuleCache.end())
+ return;
+ Outer->second.erase(maybeCaseFoldPath(ModuleUnitSource));
+ if (Outer->second.empty())
+ ModuleNameToMultipleModuleCache.erase(Outer);
+ }
-void ModuleFileCache::remove(StringRef ModuleName) {
- std::lock_guard<std::mutex> Lock(ModuleFilesMutex);
+private:
+ const GlobalCompilationDatabase &CDB;
- ModuleFiles.erase(ModuleName);
-}
+ llvm::StringMap<std::weak_ptr<const ModuleFile>>
+ ModuleNameToUniqueModuleCache;
+ // Map from module name to a map from source to module unit to the built
+ // module files.
+ llvm::StringMap<llvm::StringMap<std::weak_ptr<const ModuleFile>>>
+ ModuleNameToMultipleModuleCache;
+ // Mutex to guard accesses to Caches.
+ std::mutex ModuleFilesMutex;
+};
class ModuleNameToSourceCache {
public:
- std::string getSourceForModuleName(llvm::StringRef ModuleName) {
+ std::string getUniqueSourceForModuleName(llvm::StringRef ModuleName) {
std::lock_guard<std::mutex> Lock(CacheMutex);
- auto Iter = ModuleNameToSourceCache.find(ModuleName);
- if (Iter != ModuleNameToSourceCache.end())
+ auto Iter = ModuleNameToUniqueSourceCache.find(ModuleName);
+ if (Iter != ModuleNameToUniqueSourceCache.end())
return Iter->second;
return "";
}
- void addEntry(llvm::StringRef ModuleName, PathRef Source) {
+ void addUniqueEntry(llvm::StringRef ModuleName, PathRef Source) {
+ std::lock_guard<std::mutex> Lock(CacheMutex);
+ ModuleNameToUniqueSourceCache[ModuleName] = Source.str();
+ }
+
+ void eraseUniqueEntry(llvm::StringRef ModuleName) {
+ std::lock_guard<std::mutex> Lock(CacheMutex);
+ ModuleNameToUniqueSourceCache.erase(ModuleName);
+ }
+
+ std::string getMultipleSourceForModuleName(llvm::StringRef ModuleName,
+ PathRef RequiredSrcFile) {
+ std::lock_guard<std::mutex> Lock(CacheMutex);
+ auto Outer = ModuleNameToMultipleSourceCache.find(ModuleName);
+ if (Outer == ModuleNameToMultipleSourceCache.end())
+ return "";
+ auto Inner = Outer->second.find(maybeCaseFoldPath(RequiredSrcFile));
+ if (Inner == Outer->second.end())
+ return "";
+ return Inner->second;
+ }
+
+ void addMultipleEntry(llvm::StringRef ModuleName, PathRef RequiredSrcFile,
+ PathRef Source) {
std::lock_guard<std::mutex> Lock(CacheMutex);
- ModuleNameToSourceCache[ModuleName] = Source.str();
+ ModuleNameToMultipleSourceCache[ModuleName]
+ [maybeCaseFoldPath(RequiredSrcFile)] =
+ Source.str();
}
- void eraseEntry(llvm::StringRef ModuleName) {
+ void eraseMultipleEntry(llvm::StringRef ModuleName, PathRef RequiredSrcFile) {
std::lock_guard<std::mutex> Lock(CacheMutex);
- ModuleNameToSourceCache.erase(ModuleName);
+ auto Outer = ModuleNameToMultipleSourceCache.find(ModuleName);
+ if (Outer == ModuleNameToMultipleSourceCache.end())
+ return;
+ Outer->second.erase(maybeCaseFoldPath(RequiredSrcFile));
+ if (Outer->second.empty())
+ ModuleNameToMultipleSourceCache.erase(Outer);
}
private:
std::mutex CacheMutex;
- llvm::StringMap<std::string> ModuleNameToSourceCache;
+ llvm::StringMap<std::string> ModuleNameToUniqueSourceCache;
+
+ // Map from module name to a map from required source to module unit source
+ // which declares the corresponding module name.
+ // This looks inefficiency. We can only assume there won't too many duplicated
+ // module names with different module units in a project.
+ llvm::StringMap<llvm::StringMap<std::string>> ModuleNameToMultipleSourceCache;
};
class CachingProjectModules : public ProjectModules {
@@ -481,9 +565,41 @@ class CachingProjectModules : public ProjectModules {
return MDB->getModuleNameForSource(File);
}
+ ModuleNameState getModuleNameState(llvm::StringRef ModuleName) override {
+ return MDB->getModuleNameState(ModuleName);
+ }
+
std::string getSourceForModuleName(llvm::StringRef ModuleName,
PathRef RequiredSrcFile) override {
- std::string CachedResult = Cache.getSourceForModuleName(ModuleName);
+ auto ModuleState = MDB->getModuleNameState(ModuleName);
+
+ if (ModuleState == ModuleNameState::Multiple) {
+ std::string CachedResult =
+ Cache.getMultipleSourceForModuleName(ModuleName, RequiredSrcFile);
+
+ // Verify Cached Result by seeing if the source declaring the same module
+ // as we query.
+ if (!CachedResult.empty()) {
+ std::string ModuleNameOfCachedSource =
+ MDB->getModuleNameForSource(CachedResult);
+ if (ModuleNameOfCachedSource == ModuleName)
+ return CachedResult;
+
+ // Cached Result is invalid. Clear it.
+ Cache.eraseMultipleEntry(ModuleName, RequiredSrcFile);
+ }
+
+ auto Result = MDB->getSourceForModuleName(ModuleName, RequiredSrcFile);
+ if (!Result.empty())
+ Cache.addMultipleEntry(ModuleName, RequiredSrcFile, Result);
+ return Result;
+ }
+
+ // For unknown module name state, assume it is unique. This may give user
+ // higher usability.
+ assert(ModuleState == ModuleNameState::Unique ||
+ ModuleState == ModuleNameState::Unknown);
+ std::string CachedResult = Cache.getUniqueSourceForModuleName(ModuleName);
// Verify Cached Result by seeing if the source declaring the same module
// as we query.
@@ -494,11 +610,12 @@ class CachingProjectModules : public ProjectModules {
return CachedResult;
// Cached Result is invalid. Clear it.
- Cache.eraseEntry(ModuleName);
+ Cache.eraseUniqueEntry(ModuleName);
}
auto Result = MDB->getSourceForModuleName(ModuleName, RequiredSrcFile);
- Cache.addEntry(ModuleName, Result);
+ if (!Result.empty())
+ Cache.addUniqueEntry(ModuleName, Result);
return Result;
}
@@ -632,7 +749,18 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
if (BuiltModuleFiles.isModuleUnitBuilt(ReqModuleName))
continue;
- if (auto Cached = Cache.getModule(ReqModuleName)) {
+ auto ModuleState = MDB.getModuleNameState(ReqModuleName);
+ std::string ReqFileName =
+ MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
+
+ std::shared_ptr<const ModuleFile> Cached;
+ if (ModuleState == ProjectModules::ModuleNameState::Multiple) {
+ Cached = Cache.getMultipleModuleForModuleName(ReqModuleName, ReqFileName);
+ } else {
+ Cached = Cache.getUniqueModuleForModuleName(ReqModuleName);
+ }
+
+ if (Cached) {
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
TFS.view(std::nullopt))) {
log("Reusing module {0} from {1}", ReqModuleName,
@@ -640,18 +768,22 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
BuiltModuleFiles.addModuleFile(std::move(Cached));
continue;
}
- Cache.remove(ReqModuleName);
+ if (ModuleState == ProjectModules::ModuleNameState::Multiple)
+ Cache.eraseMultipleEntry(ReqModuleName, ReqFileName);
+ else
+ Cache.eraseUniqueEntry(ReqModuleName);
}
- std::string ReqFileName =
- MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
llvm::Expected<std::shared_ptr<BuiltModuleFile>> MF = buildModuleFile(
ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles);
if (llvm::Error Err = MF.takeError())
return Err;
log("Built module {0} to {1}", ReqModuleName, (*MF)->getModuleFilePath());
- Cache.add(ReqModuleName, *MF);
+ if (ModuleState == ProjectModules::ModuleNameState::Multiple)
+ Cache.addMultipleEntry(ReqModuleName, ReqFileName, *MF);
+ else
+ Cache.addUniqueEntry(ReqModuleName, *MF);
BuiltModuleFiles.addModuleFile(std::move(*MF));
}
diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
index 28f9345d20e56..104ee8a876b65 100644
--- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
+++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp
@@ -1084,6 +1084,77 @@ int useB() { return onlyB; }
getFullPath("b/M.cppm"));
}
+TEST_F(PrerequisiteModulesTests, DuplicateModuleNamesKeepSeparateBMICache) {
+ PerFileModulesCompilationDatabase CDB(TestDir, FS);
+
+ SmallString<256> APcm(TestDir);
+ llvm::sys::path::append(APcm, "build", "a", "M.pcm");
+ SmallString<256> BPcm(TestDir);
+ llvm::sys::path::append(BPcm, "build", "b", "M.pcm");
+
+ CDB.addFile("a/M.cppm", R"cpp(
+export module M;
+export int onlyA = 1;
+ )cpp",
+ {"--precompile", "-o", std::string(APcm)});
+ CDB.addFile("b/M.cppm", R"cpp(
+export module M;
+export int onlyB = 2;
+ )cpp",
+ {"--precompile", "-o", std::string(BPcm)});
+ CDB.addFile("a/Use.cpp", R"cpp(
+import M;
+int useA() { return onlyA; }
+ )cpp",
+ {"-fmodule-file=M=" + std::string(APcm)});
+ CDB.addFile("b/Use.cpp", R"cpp(
+import M;
+int useB() { return onlyB; }
+ )cpp",
+ {"-fmodule-file=M=" + std::string(BPcm)});
+
+ ModulesBuilder Builder(CDB);
+
+ auto AInfo =
+ Builder.buildPrerequisiteModulesFor(getFullPath("a/Use.cpp"), FS);
+ auto BInfo =
+ Builder.buildPrerequisiteModulesFor(getFullPath("b/Use.cpp"), FS);
+ ASSERT_TRUE(AInfo);
+ ASSERT_TRUE(BInfo);
+
+ HeaderSearchOptions HSA(TestDir);
+ HeaderSearchOptions HSB(TestDir);
+ AInfo->adjustHeaderSearchOptions(HSA);
+ BInfo->adjustHeaderSearchOptions(HSB);
+ ASSERT_EQ(HSA.PrebuiltModuleFiles.count("M"), 1u);
+ ASSERT_EQ(HSB.PrebuiltModuleFiles.count("M"), 1u);
+ EXPECT_NE(HSA.PrebuiltModuleFiles["M"], HSB.PrebuiltModuleFiles["M"]);
+
+ auto UseA = getInputs("a/Use.cpp", CDB);
+ UseA.ModulesManager = &Builder;
+ auto CIA = buildCompilerInvocation(UseA, DiagConsumer);
+ ASSERT_TRUE(CIA);
+ auto PreambleA = buildPreamble(getFullPath("a/Use.cpp"), *CIA, UseA,
+ /*InMemory=*/true, /*Callback=*/nullptr);
+ ASSERT_TRUE(PreambleA);
+ auto ASTA = ParsedAST::build(getFullPath("a/Use.cpp"), UseA, std::move(CIA),
+ {}, PreambleA);
+ ASSERT_TRUE(ASTA);
+ EXPECT_TRUE(findDecl(*ASTA, "onlyA").isFromASTFile());
+
+ auto UseB = getInputs("b/Use.cpp", CDB);
+ UseB.ModulesManager = &Builder;
+ auto CIB = buildCompilerInvocation(UseB, DiagConsumer);
+ ASSERT_TRUE(CIB);
+ auto PreambleB = buildPreamble(getFullPath("b/Use.cpp"), *CIB, UseB,
+ /*InMemory=*/true, /*Callback=*/nullptr);
+ ASSERT_TRUE(PreambleB);
+ auto ASTB = ParsedAST::build(getFullPath("b/Use.cpp"), UseB, std::move(CIB),
+ {}, PreambleB);
+ ASSERT_TRUE(ASTB);
+ EXPECT_TRUE(findDecl(*ASTB, "onlyB").isFromASTFile());
+}
+
TEST_F(PrerequisiteModulesTests, ModuleImportThroughInclude) {
MockDirectoryCompilationDatabase CDB(TestDir, FS);
More information about the cfe-commits
mailing list