https://github.com/fleeting-xx updated
https://github.com/llvm/llvm-project/pull/142828
>From 63c7b6ea7927750c0559098ac9d74f34d03b4863 Mon Sep 17 00:00:00 2001
From: fleeting-xx <bake...@gmail.com>
Date: Wed, 4 Jun 2025 12:57:48 -0500
Subject: [PATCH 1/2] [clangd] [Modules] Fix to correctly handle module
dependencies
- Fix dangling string references in the return value of
getAllRequiredModules()
- Change a couple of calls in getOrBuildModuleFile() to use the loop
variable instead of the ModuleName parameter.
---
clang-tools-extra/clangd/ModulesBuilder.cpp | 22 +++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp
b/clang-tools-extra/clangd/ModulesBuilder.cpp
index bf77f43bd28bb..d88aa01aad05d 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -430,10 +430,10 @@ class CachingProjectModules : public ProjectModules {
/// Collect the directly and indirectly required module names for \param
/// ModuleName in topological order. The \param ModuleName is guaranteed to
/// be the last element in \param ModuleNames.
-llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
- CachingProjectModules &MDB,
- StringRef ModuleName) {
- llvm::SmallVector<llvm::StringRef> ModuleNames;
+llvm::SmallVector<std::string> getAllRequiredModules(PathRef RequiredSource,
+ CachingProjectModules
&MDB,
+ StringRef ModuleName) {
+ llvm::SmallVector<std::string> ModuleNames;
llvm::StringSet<> ModuleNamesSet;
auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
@@ -444,7 +444,7 @@ llvm::SmallVector<StringRef> getAllRequiredModules(PathRef
RequiredSource,
if (ModuleNamesSet.insert(RequiredModuleName).second)
Visitor(RequiredModuleName, Visitor);
- ModuleNames.push_back(ModuleName);
+ ModuleNames.push_back(ModuleName.str());
};
VisitDeps(ModuleName, VisitDeps);
@@ -494,13 +494,13 @@ llvm::Error
ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
// Get Required modules in topological order.
auto ReqModuleNames = getAllRequiredModules(RequiredSource, MDB, ModuleName);
for (llvm::StringRef ReqModuleName : ReqModuleNames) {
- if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
+ if (BuiltModuleFiles.isModuleUnitBuilt(ReqModuleName))
continue;
if (auto Cached = Cache.getModule(ReqModuleName)) {
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
TFS.view(std::nullopt))) {
- log("Reusing module {0} from {1}", ModuleName,
+ log("Reusing module {0} from {1}", ReqModuleName,
Cached->getModuleFilePath());
BuiltModuleFiles.addModuleFile(std::move(Cached));
continue;
@@ -508,14 +508,16 @@ llvm::Error
ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
Cache.remove(ReqModuleName);
}
+ std::string ReqFileName =
+ MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
llvm::Expected<ModuleFile> MF = buildModuleFile(
- ModuleName, ModuleUnitFileName, getCDB(), TFS, BuiltModuleFiles);
+ ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles);
if (llvm::Error Err = MF.takeError())
return Err;
- log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
+ log("Built module {0} to {1}", ReqModuleName, MF->getModuleFilePath());
auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
- Cache.add(ModuleName, BuiltModuleFile);
+ Cache.add(ReqModuleName, BuiltModuleFile);
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
}
>From 593eb155ca8202e2d52e69800114918369ab642e Mon Sep 17 00:00:00 2001
From: fleeting-xx <bake...@gmail.com>
Date: Thu, 5 Jun 2025 10:24:34 -0500
Subject: [PATCH 2/2] Added a clangd unit test for resolving module dependency
chains
---
.../clangd/test/module_dependencies.test | 96 +++++++++++++++++++
1 file changed, 96 insertions(+)
create mode 100644 clang-tools-extra/clangd/test/module_dependencies.test
diff --git a/clang-tools-extra/clangd/test/module_dependencies.test
b/clang-tools-extra/clangd/test/module_dependencies.test
new file mode 100644
index 0000000000000..1023b2363c9fa
--- /dev/null
+++ b/clang-tools-extra/clangd/test/module_dependencies.test
@@ -0,0 +1,96 @@
+# A smoke test to check that a simple dependency chain for modules can work.
+#
+# FIXME: This fails on the Windows ARM64 build server. Not entirely sure why
as it has been tested on
+# an ARM64 Windows VM and appears to work there.
+# UNSUPPORTED: host=aarch64-pc-windows-msvc
+#
+# RUN: rm -fr %t
+# RUN: mkdir -p %t
+# RUN: split-file %s %t
+#
+# RUN: sed -e "s|DIR|%/t|g" %t/compile_commands.json.tmpl >
%t/compile_commands.json.tmp
+# RUN: sed -e "s|CLANG_CC|%clang|g" %t/compile_commands.json.tmp >
%t/compile_commands.json
+# RUN: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl >
%t/definition.jsonrpc.tmp
+#
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g'
%/t/definition.jsonrpc.tmp > %/t/definition.jsonrpc
+#
+# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc \
+# RUN: | FileCheck -strict-whitespace %t/definition.jsonrpc
+
+#--- A-frag.cppm
+export module A:frag;
+export void printA() {}
+
+#--- A.cppm
+export module A;
+export import :frag;
+
+#--- Use.cpp
+import A;
+void foo() {
+ print
+}
+
+#--- compile_commands.json.tmpl
+[
+ {
+ "directory": "DIR",
+ "command": "CLANG_CC -fprebuilt-module-path=DIR -std=c++20 -o
DIR/main.cpp.o -c DIR/Use.cpp",
+ "file": "DIR/Use.cpp"
+ },
+ {
+ "directory": "DIR",
+ "command": "CLANG_CC -std=c++20 DIR/A.cppm --precompile -o DIR/A.pcm",
+ "file": "DIR/A.cppm"
+ },
+ {
+ "directory": "DIR",
+ "command": "CLANG_CC -std=c++20 DIR/A-frag.cppm --precompile -o
DIR/A-frag.pcm",
+ "file": "DIR/A-frag.cppm"
+ }
+]
+
+#--- definition.jsonrpc.tmpl
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+ "processId": 123,
+ "rootPath": "clangd",
+ "capabilities": {
+ "textDocument": {
+ "completion": {
+ "completionItem": {
+ "snippetSupport": true
+ }
+ }
+ }
+ },
+ "trace": "off"
+ }
+}
+---
+{
+ "jsonrpc": "2.0",
+ "method": "textDocument/didOpen",
+ "params": {
+ "textDocument": {
+ "uri": "file://DIR/Use.cpp",
+ "languageId": "cpp",
+ "version": 1,
+ "text": "import A;\nvoid foo() {\n print\n}\n"
+ }
+ }
+}
+
+# CHECK: "message"{{.*}}printA{{.*}}(fix available)
+
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file://DIR/Use.cpp"},"context":{"triggerKind":1},"position":{"line":2,"character":6}}}
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
