[clang-tools-extra] 278ef84 - Revert "[clangd] [Modules] Fixes to correctly handle module dependencies" (#142162)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 30 18:21:54 PDT 2025
Author: fleeting-xx
Date: 2025-05-31T09:21:50+08:00
New Revision: 278ef84d2019f8200cf0959a50e1c44819c8bfeb
URL: https://github.com/llvm/llvm-project/commit/278ef84d2019f8200cf0959a50e1c44819c8bfeb
DIFF: https://github.com/llvm/llvm-project/commit/278ef84d2019f8200cf0959a50e1c44819c8bfeb.diff
LOG: Revert "[clangd] [Modules] Fixes to correctly handle module dependencies" (#142162)
Added:
Modified:
clang-tools-extra/clangd/ModulesBuilder.cpp
clang-tools-extra/clangd/test/modules.test
Removed:
clang-tools-extra/clangd/test/module_dependencies.test
################################################################################
diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp
index 786fb88dbe318..bf77f43bd28bb 100644
--- a/clang-tools-extra/clangd/ModulesBuilder.cpp
+++ b/clang-tools-extra/clangd/ModulesBuilder.cpp
@@ -84,7 +84,8 @@ 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
@@ -429,21 +430,21 @@ 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<std::string> getAllRequiredModules(PathRef RequiredSource,
- CachingProjectModules &MDB,
- StringRef ModuleName) {
- llvm::SmallVector<std::string> ModuleNames;
+llvm::SmallVector<StringRef> getAllRequiredModules(PathRef RequiredSource,
+ CachingProjectModules &MDB,
+ StringRef ModuleName) {
+ llvm::SmallVector<llvm::StringRef> ModuleNames;
llvm::StringSet<> ModuleNamesSet;
auto VisitDeps = [&](StringRef ModuleName, auto Visitor) -> void {
ModuleNamesSet.insert(ModuleName);
- for (const std::string &RequiredModuleName : MDB.getRequiredModules(
+ for (StringRef RequiredModuleName : MDB.getRequiredModules(
MDB.getSourceForModuleName(ModuleName, RequiredSource)))
if (ModuleNamesSet.insert(RequiredModuleName).second)
Visitor(RequiredModuleName, Visitor);
- ModuleNames.push_back(ModuleName.str());
+ ModuleNames.push_back(ModuleName);
};
VisitDeps(ModuleName, VisitDeps);
@@ -493,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(ReqModuleName))
+ if (BuiltModuleFiles.isModuleUnitBuilt(ModuleName))
continue;
if (auto Cached = Cache.getModule(ReqModuleName)) {
if (IsModuleFileUpToDate(Cached->getModuleFilePath(), BuiltModuleFiles,
TFS.view(std::nullopt))) {
- log("Reusing module {0} from {1}", ReqModuleName,
+ log("Reusing module {0} from {1}", ModuleName,
Cached->getModuleFilePath());
BuiltModuleFiles.addModuleFile(std::move(Cached));
continue;
@@ -507,16 +508,14 @@ llvm::Error ModulesBuilder::ModulesBuilderImpl::getOrBuildModuleFile(
Cache.remove(ReqModuleName);
}
- std::string ReqFileName =
- MDB.getSourceForModuleName(ReqModuleName, RequiredSource);
llvm::Expected<ModuleFile> MF = buildModuleFile(
- ReqModuleName, ReqFileName, getCDB(), TFS, BuiltModuleFiles);
+ ModuleName, ModuleUnitFileName, getCDB(), TFS, BuiltModuleFiles);
if (llvm::Error Err = MF.takeError())
return Err;
- log("Built module {0} to {1}", ReqModuleName, MF->getModuleFilePath());
+ log("Built module {0} to {1}", ModuleName, MF->getModuleFilePath());
auto BuiltModuleFile = std::make_shared<const ModuleFile>(std::move(*MF));
- Cache.add(ReqModuleName, BuiltModuleFile);
+ Cache.add(ModuleName, BuiltModuleFile);
BuiltModuleFiles.addModuleFile(std::move(BuiltModuleFile));
}
diff --git a/clang-tools-extra/clangd/test/module_dependencies.test b/clang-tools-extra/clangd/test/module_dependencies.test
deleted file mode 100644
index ee1df7f8a35cc..0000000000000
--- a/clang-tools-extra/clangd/test/module_dependencies.test
+++ /dev/null
@@ -1,92 +0,0 @@
-# A smoke test to check that a simple dependency chain for modules can work.
-#
-# 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"}
diff --git a/clang-tools-extra/clangd/test/modules.test b/clang-tools-extra/clangd/test/modules.test
index 6792352bb8e56..74280811a6cff 100644
--- a/clang-tools-extra/clangd/test/modules.test
+++ b/clang-tools-extra/clangd/test/modules.test
@@ -1,16 +1,16 @@
# A smoke test to check the modules can work basically.
#
+# Windows have
diff erent escaping modes.
+# FIXME: We should add one for windows.
+# UNSUPPORTED: system-windows
+#
# 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: sed -e "s|DIR|%/t|g" %t/definition.jsonrpc.tmpl > %t/definition.jsonrpc
#
# RUN: clangd -experimental-modules-support -lit-test < %t/definition.jsonrpc \
# RUN: | FileCheck -strict-whitespace %t/definition.jsonrpc
More information about the cfe-commits
mailing list