https://github.com/petr-polezhaev updated https://github.com/llvm/llvm-project/pull/122606
>From 0813476d626e21828f73e9f9a3a3561becd37277 Mon Sep 17 00:00:00 2001 From: Petr Polezhaev <petr.polezh...@ratigorsk-12.ru> Date: Sat, 11 Jan 2025 21:21:16 +0300 Subject: [PATCH 1/3] [clangd] Support .clangd command line modifications in ScanningAllProjectModules --- .../clangd/GlobalCompilationDatabase.cpp | 11 ++++- .../clangd/GlobalCompilationDatabase.h | 3 ++ clang-tools-extra/clangd/ProjectModules.h | 7 +++ .../clangd/ScanningProjectModules.cpp | 39 +++++++++------ .../unittests/PrerequisiteModulesTest.cpp | 49 +++++++++++++++++-- 5 files changed, 90 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index 71e97ac4efd673..ea2ff3d604efbc 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -830,6 +830,16 @@ bool OverlayCDB::setCompileCommand(PathRef File, return true; } +std::unique_ptr<ProjectModules> +OverlayCDB::getProjectModules(PathRef File) const { + auto MDB = DelegatingCDB::getProjectModules(File); + MDB->setCommandProvider([&Mangler = Mangler](tooling::CompileCommand &Command, + PathRef CommandPath) { + Mangler(Command, CommandPath); + }); + return std::move(MDB); +} + DelegatingCDB::DelegatingCDB(const GlobalCompilationDatabase *Base) : Base(Base) { if (Base) @@ -874,6 +884,5 @@ bool DelegatingCDB::blockUntilIdle(Deadline D) const { return true; return Base->blockUntilIdle(D); } - } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.h b/clang-tools-extra/clangd/GlobalCompilationDatabase.h index f8349c6efecb01..1d636d73664bee 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.h +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.h @@ -209,6 +209,9 @@ class OverlayCDB : public DelegatingCDB { setCompileCommand(PathRef File, std::optional<tooling::CompileCommand> CompilationCommand); + std::unique_ptr<ProjectModules> + getProjectModules(PathRef File) const override; + private: mutable std::mutex Mutex; llvm::StringMap<tooling::CompileCommand> Commands; /* GUARDED_BY(Mut) */ diff --git a/clang-tools-extra/clangd/ProjectModules.h b/clang-tools-extra/clangd/ProjectModules.h index 3b9b564a87da01..6830d44919fa81 100644 --- a/clang-tools-extra/clangd/ProjectModules.h +++ b/clang-tools-extra/clangd/ProjectModules.h @@ -9,8 +9,10 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROJECTMODULES_H +#include "support/Function.h" #include "support/Path.h" #include "support/ThreadsafeFS.h" +#include "clang/Tooling/CompilationDatabase.h" #include <memory> @@ -36,11 +38,16 @@ namespace clangd { /// `<primary-module-name>[:partition-name]`. So module names covers partitions. class ProjectModules { public: + using CommandProvider = + llvm::unique_function<void(tooling::CompileCommand &, PathRef) const>; + virtual std::vector<std::string> getRequiredModules(PathRef File) = 0; virtual PathRef getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSrcFile = PathRef()) = 0; + virtual void setCommandProvider(CommandProvider Provider) {} + virtual ~ProjectModules() = default; }; diff --git a/clang-tools-extra/clangd/ScanningProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp index 92f75ef7d5c25a..6f098c18cd80a0 100644 --- a/clang-tools-extra/clangd/ScanningProjectModules.cpp +++ b/clang-tools-extra/clangd/ScanningProjectModules.cpp @@ -32,6 +32,9 @@ namespace { /// interfere with each other. class ModuleDependencyScanner { public: + using CommandProvider = + llvm::unique_function<void(tooling::CompileCommand &, PathRef) const>; + ModuleDependencyScanner( std::shared_ptr<const clang::tooling::CompilationDatabase> CDB, const ThreadsafeFS &TFS) @@ -48,7 +51,8 @@ class ModuleDependencyScanner { }; /// Scanning the single file specified by \param FilePath. - std::optional<ModuleDependencyInfo> scan(PathRef FilePath); + std::optional<ModuleDependencyInfo> scan(PathRef FilePath, + CommandProvider const &Provider); /// Scanning every source file in the current project to get the /// <module-name> to <module-unit-source> map. @@ -57,7 +61,7 @@ class ModuleDependencyScanner { /// a global module dependency scanner to monitor every file. Or we /// can simply require the build systems (or even the end users) /// to provide the map. - void globalScan(); + void globalScan(CommandProvider const &Provider); /// Get the source file from the module name. Note that the language /// guarantees all the module names are unique in a valid program. @@ -69,7 +73,8 @@ class ModuleDependencyScanner { /// Return the direct required modules. Indirect required modules are not /// included. - std::vector<std::string> getRequiredModules(PathRef File); + std::vector<std::string> getRequiredModules(PathRef File, + CommandProvider const &Provider); private: std::shared_ptr<const clang::tooling::CompilationDatabase> CDB; @@ -87,7 +92,8 @@ class ModuleDependencyScanner { }; std::optional<ModuleDependencyScanner::ModuleDependencyInfo> -ModuleDependencyScanner::scan(PathRef FilePath) { +ModuleDependencyScanner::scan(PathRef FilePath, + CommandProvider const &Provider) { auto Candidates = CDB->getCompileCommands(FilePath); if (Candidates.empty()) return std::nullopt; @@ -97,10 +103,8 @@ ModuleDependencyScanner::scan(PathRef FilePath) { // DirectoryBasedGlobalCompilationDatabase::getCompileCommand. tooling::CompileCommand Cmd = std::move(Candidates.front()); - static int StaticForMainAddr; // Just an address in this process. - Cmd.CommandLine.push_back("-resource-dir=" + - CompilerInvocation::GetResourcesPath( - "clangd", (void *)&StaticForMainAddr)); + if (Provider) + Provider(Cmd, FilePath); using namespace clang::tooling::dependencies; @@ -130,9 +134,9 @@ ModuleDependencyScanner::scan(PathRef FilePath) { return Result; } -void ModuleDependencyScanner::globalScan() { +void ModuleDependencyScanner::globalScan(CommandProvider const &Provider) { for (auto &File : CDB->getAllFiles()) - scan(File); + scan(File, Provider); GlobalScanned = true; } @@ -150,9 +154,9 @@ PathRef ModuleDependencyScanner::getSourceForModuleName( return {}; } -std::vector<std::string> -ModuleDependencyScanner::getRequiredModules(PathRef File) { - auto ScanningResult = scan(File); +std::vector<std::string> ModuleDependencyScanner::getRequiredModules( + PathRef File, CommandProvider const &CmdProvider) { + auto ScanningResult = scan(File, CmdProvider); if (!ScanningResult) return {}; @@ -177,7 +181,11 @@ class ScanningAllProjectModules : public ProjectModules { ~ScanningAllProjectModules() override = default; std::vector<std::string> getRequiredModules(PathRef File) override { - return Scanner.getRequiredModules(File); + return Scanner.getRequiredModules(File, Provider); + } + + void setCommandProvider(CommandProvider Provider) override { + this->Provider = std::move(Provider); } /// RequiredSourceFile is not used intentionally. See the comments of @@ -185,12 +193,13 @@ class ScanningAllProjectModules : public ProjectModules { PathRef getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSourceFile = PathRef()) override { - Scanner.globalScan(); + Scanner.globalScan(Provider); return Scanner.getSourceForModuleName(ModuleName); } private: ModuleDependencyScanner Scanner; + CommandProvider Provider; }; std::unique_ptr<ProjectModules> scanningProjectModules( diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 1bb8e19cce23e0..b48ab0bd6ff6f9 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -9,6 +9,7 @@ /// FIXME: Skip testing on windows temporarily due to the different escaping /// code mode. +#include "llvm/ADT/StringRef.h" #ifndef _WIN32 #include "ModulesBuilder.h" @@ -39,7 +40,25 @@ class MockDirectoryCompilationDatabase : public MockCompilationDatabase { void addFile(llvm::StringRef Path, llvm::StringRef Contents); std::unique_ptr<ProjectModules> getProjectModules(PathRef) const override { - return scanningProjectModules(MockedCDBPtr, TFS); + auto Modules = scanningProjectModules(MockedCDBPtr, TFS); + Modules->setCommandProvider( + [this](tooling::CompileCommand &Command, PathRef) { + for (auto &Flag : ExcludeFlags) { + auto const It = std::find(Command.CommandLine.begin(), + Command.CommandLine.end(), Flag); + if (It != Command.CommandLine.end()) + Command.CommandLine.erase(It); + } + }); + return Modules; + } + + void addExtraClangFlag(std::string Flag) { + this->ExtraClangFlags.push_back(std::move(Flag)); + } + + void addExcludedFlag(std::string Flag) { + this->ExcludeFlags.push_back(std::move(Flag)); } private: @@ -66,6 +85,7 @@ class MockDirectoryCompilationDatabase : public MockCompilationDatabase { }; std::shared_ptr<MockClangCompilationDatabase> MockedCDBPtr; + std::vector<std::string> ExcludeFlags; const ThreadsafeFS &TFS; }; @@ -191,6 +211,29 @@ export module M; EXPECT_TRUE(MInfo->canReuse(*Invocation, FS.view(TestDir))); } +TEST_F(PrerequisiteModulesTests, ModuleWithArgumentPatch) { + MockDirectoryCompilationDatabase CDB(TestDir, FS); + + CDB.addExtraClangFlag("-invalid-unknown-flag"); + + CDB.addFile("Dep.cppm", R"cpp( +export module Dep; + )cpp"); + + CDB.addFile("M.cppm", R"cpp( +export module M; +import Dep; + )cpp"); + + auto ProjectModules = CDB.getProjectModules(getFullPath("M.cppm")); + EXPECT_TRUE( + ProjectModules->getRequiredModules(getFullPath("M.cppm")).empty()); + + CDB.addExcludedFlag("-invalid-unknown-flag"); + EXPECT_FALSE( + ProjectModules->getRequiredModules(getFullPath("M.cppm")).empty()); +} + TEST_F(PrerequisiteModulesTests, ModuleWithDepTest) { MockDirectoryCompilationDatabase CDB(TestDir, FS); @@ -435,7 +478,7 @@ void func() { /*Callback=*/nullptr); EXPECT_TRUE(Preamble); EXPECT_TRUE(Preamble->RequiredModules); - + auto Result = codeComplete(getFullPath("Use.cpp"), Test.point(), Preamble.get(), Use, {}); EXPECT_FALSE(Result.Completions.empty()); @@ -474,7 +517,7 @@ void func() { /*Callback=*/nullptr); EXPECT_TRUE(Preamble); EXPECT_TRUE(Preamble->RequiredModules); - + auto Result = signatureHelp(getFullPath("Use.cpp"), Test.point(), *Preamble.get(), Use, MarkupKind::PlainText); EXPECT_FALSE(Result.signatures.empty()); >From 5c89b9dbf97b57d232817a4716433239e84365a1 Mon Sep 17 00:00:00 2001 From: Petr Polezhaev <petr.polezh...@ratigorsk-12.ru> Date: Tue, 14 Jan 2025 18:36:28 +0300 Subject: [PATCH 2/3] fixup! [clangd] Support .clangd command line modifications in ScanningAllProjectModules --- clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index b48ab0bd6ff6f9..2888d570af5ad3 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -9,7 +9,6 @@ /// FIXME: Skip testing on windows temporarily due to the different escaping /// code mode. -#include "llvm/ADT/StringRef.h" #ifndef _WIN32 #include "ModulesBuilder.h" @@ -19,6 +18,7 @@ #include "Compiler.h" #include "TestTU.h" #include "support/ThreadsafeFS.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/raw_ostream.h" #include "gmock/gmock.h" >From 3f139acecfcb80ae82e674b58aa7767cdbe77a14 Mon Sep 17 00:00:00 2001 From: Petr Polezhaev <petr.polezh...@ratigorsk-12.ru> Date: Sat, 18 Jan 2025 14:52:56 +0300 Subject: [PATCH 3/3] fixup! [clangd] Support .clangd command line modifications in ScanningAllProjectModules --- .../clangd/GlobalCompilationDatabase.cpp | 5 ++- clang-tools-extra/clangd/ProjectModules.h | 9 +++-- .../clangd/ScanningProjectModules.cpp | 39 +++++++++---------- .../unittests/PrerequisiteModulesTest.cpp | 39 +++++++------------ 4 files changed, 42 insertions(+), 50 deletions(-) diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp index ea2ff3d604efbc..2b1da4be5c1bd6 100644 --- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp +++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp @@ -833,8 +833,8 @@ bool OverlayCDB::setCompileCommand(PathRef File, std::unique_ptr<ProjectModules> OverlayCDB::getProjectModules(PathRef File) const { auto MDB = DelegatingCDB::getProjectModules(File); - MDB->setCommandProvider([&Mangler = Mangler](tooling::CompileCommand &Command, - PathRef CommandPath) { + MDB->setCommandMangler([&Mangler = Mangler](tooling::CompileCommand &Command, + PathRef CommandPath) { Mangler(Command, CommandPath); }); return std::move(MDB); @@ -884,5 +884,6 @@ bool DelegatingCDB::blockUntilIdle(Deadline D) const { return true; return Base->blockUntilIdle(D); } + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ProjectModules.h b/clang-tools-extra/clangd/ProjectModules.h index 6830d44919fa81..cabc06996c80c3 100644 --- a/clang-tools-extra/clangd/ProjectModules.h +++ b/clang-tools-extra/clangd/ProjectModules.h @@ -12,11 +12,14 @@ #include "support/Function.h" #include "support/Path.h" #include "support/ThreadsafeFS.h" -#include "clang/Tooling/CompilationDatabase.h" #include <memory> namespace clang { +namespace tooling { +class CompileCommand; +} // namespace tooling + namespace clangd { /// An interface to query the modules information in the project. @@ -38,7 +41,7 @@ namespace clangd { /// `<primary-module-name>[:partition-name]`. So module names covers partitions. class ProjectModules { public: - using CommandProvider = + using CommandMangler = llvm::unique_function<void(tooling::CompileCommand &, PathRef) const>; virtual std::vector<std::string> getRequiredModules(PathRef File) = 0; @@ -46,7 +49,7 @@ class ProjectModules { getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSrcFile = PathRef()) = 0; - virtual void setCommandProvider(CommandProvider Provider) {} + virtual void setCommandMangler(CommandMangler Mangler) {} virtual ~ProjectModules() = default; }; diff --git a/clang-tools-extra/clangd/ScanningProjectModules.cpp b/clang-tools-extra/clangd/ScanningProjectModules.cpp index 6f098c18cd80a0..e4dc11c1c28958 100644 --- a/clang-tools-extra/clangd/ScanningProjectModules.cpp +++ b/clang-tools-extra/clangd/ScanningProjectModules.cpp @@ -32,9 +32,6 @@ namespace { /// interfere with each other. class ModuleDependencyScanner { public: - using CommandProvider = - llvm::unique_function<void(tooling::CompileCommand &, PathRef) const>; - ModuleDependencyScanner( std::shared_ptr<const clang::tooling::CompilationDatabase> CDB, const ThreadsafeFS &TFS) @@ -51,8 +48,8 @@ class ModuleDependencyScanner { }; /// Scanning the single file specified by \param FilePath. - std::optional<ModuleDependencyInfo> scan(PathRef FilePath, - CommandProvider const &Provider); + std::optional<ModuleDependencyInfo> + scan(PathRef FilePath, const ProjectModules::CommandMangler &Mangler); /// Scanning every source file in the current project to get the /// <module-name> to <module-unit-source> map. @@ -61,7 +58,7 @@ class ModuleDependencyScanner { /// a global module dependency scanner to monitor every file. Or we /// can simply require the build systems (or even the end users) /// to provide the map. - void globalScan(CommandProvider const &Provider); + void globalScan(const ProjectModules::CommandMangler &Mangler); /// Get the source file from the module name. Note that the language /// guarantees all the module names are unique in a valid program. @@ -73,8 +70,9 @@ class ModuleDependencyScanner { /// Return the direct required modules. Indirect required modules are not /// included. - std::vector<std::string> getRequiredModules(PathRef File, - CommandProvider const &Provider); + std::vector<std::string> + getRequiredModules(PathRef File, + const ProjectModules::CommandMangler &Mangler); private: std::shared_ptr<const clang::tooling::CompilationDatabase> CDB; @@ -93,7 +91,7 @@ class ModuleDependencyScanner { std::optional<ModuleDependencyScanner::ModuleDependencyInfo> ModuleDependencyScanner::scan(PathRef FilePath, - CommandProvider const &Provider) { + const ProjectModules::CommandMangler &Mangler) { auto Candidates = CDB->getCompileCommands(FilePath); if (Candidates.empty()) return std::nullopt; @@ -103,8 +101,8 @@ ModuleDependencyScanner::scan(PathRef FilePath, // DirectoryBasedGlobalCompilationDatabase::getCompileCommand. tooling::CompileCommand Cmd = std::move(Candidates.front()); - if (Provider) - Provider(Cmd, FilePath); + if (Mangler) + Mangler(Cmd, FilePath); using namespace clang::tooling::dependencies; @@ -134,9 +132,10 @@ ModuleDependencyScanner::scan(PathRef FilePath, return Result; } -void ModuleDependencyScanner::globalScan(CommandProvider const &Provider) { +void ModuleDependencyScanner::globalScan( + const ProjectModules::CommandMangler &Mangler) { for (auto &File : CDB->getAllFiles()) - scan(File, Provider); + scan(File, Mangler); GlobalScanned = true; } @@ -155,8 +154,8 @@ PathRef ModuleDependencyScanner::getSourceForModuleName( } std::vector<std::string> ModuleDependencyScanner::getRequiredModules( - PathRef File, CommandProvider const &CmdProvider) { - auto ScanningResult = scan(File, CmdProvider); + PathRef File, const ProjectModules::CommandMangler &Mangler) { + auto ScanningResult = scan(File, Mangler); if (!ScanningResult) return {}; @@ -181,11 +180,11 @@ class ScanningAllProjectModules : public ProjectModules { ~ScanningAllProjectModules() override = default; std::vector<std::string> getRequiredModules(PathRef File) override { - return Scanner.getRequiredModules(File, Provider); + return Scanner.getRequiredModules(File, Mangler); } - void setCommandProvider(CommandProvider Provider) override { - this->Provider = std::move(Provider); + void setCommandMangler(CommandMangler Mangler) override { + this->Mangler = std::move(Mangler); } /// RequiredSourceFile is not used intentionally. See the comments of @@ -193,13 +192,13 @@ class ScanningAllProjectModules : public ProjectModules { PathRef getSourceForModuleName(llvm::StringRef ModuleName, PathRef RequiredSourceFile = PathRef()) override { - Scanner.globalScan(Provider); + Scanner.globalScan(Mangler); return Scanner.getSourceForModuleName(ModuleName); } private: ModuleDependencyScanner Scanner; - CommandProvider Provider; + CommandMangler Mangler; }; std::unique_ptr<ProjectModules> scanningProjectModules( diff --git a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp index 2888d570af5ad3..972f1ba4b458a3 100644 --- a/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp +++ b/clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp @@ -39,28 +39,6 @@ class MockDirectoryCompilationDatabase : public MockCompilationDatabase { void addFile(llvm::StringRef Path, llvm::StringRef Contents); - std::unique_ptr<ProjectModules> getProjectModules(PathRef) const override { - auto Modules = scanningProjectModules(MockedCDBPtr, TFS); - Modules->setCommandProvider( - [this](tooling::CompileCommand &Command, PathRef) { - for (auto &Flag : ExcludeFlags) { - auto const It = std::find(Command.CommandLine.begin(), - Command.CommandLine.end(), Flag); - if (It != Command.CommandLine.end()) - Command.CommandLine.erase(It); - } - }); - return Modules; - } - - void addExtraClangFlag(std::string Flag) { - this->ExtraClangFlags.push_back(std::move(Flag)); - } - - void addExcludedFlag(std::string Flag) { - this->ExcludeFlags.push_back(std::move(Flag)); - } - private: class MockClangCompilationDatabase : public tooling::CompilationDatabase { public: @@ -85,7 +63,6 @@ class MockDirectoryCompilationDatabase : public MockCompilationDatabase { }; std::shared_ptr<MockClangCompilationDatabase> MockedCDBPtr; - std::vector<std::string> ExcludeFlags; const ThreadsafeFS &TFS; }; @@ -214,7 +191,7 @@ export module M; TEST_F(PrerequisiteModulesTests, ModuleWithArgumentPatch) { MockDirectoryCompilationDatabase CDB(TestDir, FS); - CDB.addExtraClangFlag("-invalid-unknown-flag"); + CDB.ExtraClangFlags.push_back("-invalid-unknown-flag"); CDB.addFile("Dep.cppm", R"cpp( export module Dep; @@ -225,11 +202,23 @@ export module M; import Dep; )cpp"); + // An invalid flag will break the module compilation and the + // getRequiredModules would return an empty array auto ProjectModules = CDB.getProjectModules(getFullPath("M.cppm")); EXPECT_TRUE( ProjectModules->getRequiredModules(getFullPath("M.cppm")).empty()); - CDB.addExcludedFlag("-invalid-unknown-flag"); + // Set the mangler to filter out the invalid flag + ProjectModules->setCommandMangler( + [](tooling::CompileCommand &Command, PathRef) { + auto const It = + std::find(Command.CommandLine.begin(), Command.CommandLine.end(), + "-invalid-unknown-flag"); + Command.CommandLine.erase(It); + }); + + // And now it returns a non-empty list of required modules since the + // compilation succeeded EXPECT_FALSE( ProjectModules->getRequiredModules(getFullPath("M.cppm")).empty()); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits