https://github.com/fleeting-xx created https://github.com/llvm/llvm-project/pull/142162
Reverts llvm/llvm-project#142090 due to build failures on [arm64 windows](https://lab.llvm.org/buildbot/#/builders/161). I'll need someone with commit permission to apply this revert. >From 39fe26a200155bad4992bcc6f430ffa5b4cace77 Mon Sep 17 00:00:00 2001 From: fleeting-xx <bake...@gmail.com> Date: Fri, 30 May 2025 09:42:33 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"[clangd]=20[Modules]=20Fixes=20to=20c?= =?UTF-8?q?orrectly=20handle=20module=20dependencies=20(#14=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit d490526a81586c7b2fe674ce520276570c9881e2. --- clang-tools-extra/clangd/ModulesBuilder.cpp | 27 +++--- .../clangd/test/module_dependencies.test | 92 ------------------- clang-tools-extra/clangd/test/modules.test | 10 +- 3 files changed, 18 insertions(+), 111 deletions(-) delete mode 100644 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 different 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
