Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clangd] Support .clangd command line modifications for C++ modules #122606

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petr-polezhaev
Copy link

Tunnels Manger object into the ScanningAllProjectModules so it can be used to perform necessary command-line modifications (which also adds --resources path previously added there explicitly). This allows using the experimental C++ modules support with gcc.

This was discussed in the issue with @ChuanqiXu9 and @kadircet

Closes #112635

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Jan 11, 2025

@llvm/pr-subscribers-clangd

@llvm/pr-subscribers-clang-tools-extra

Author: Petr Polezhaev (petr-polezhaev)

Changes

Tunnels Manger object into the ScanningAllProjectModules so it can be used to perform necessary command-line modifications (which also adds --resources path previously added there explicitly). This allows using the experimental C++ modules support with gcc.

This was discussed in the issue with @ChuanqiXu9 and @kadircet

Closes #112635


Full diff: https://github.com/llvm/llvm-project/pull/122606.diff

5 Files Affected:

  • (modified) clang-tools-extra/clangd/GlobalCompilationDatabase.cpp (+10-1)
  • (modified) clang-tools-extra/clangd/GlobalCompilationDatabase.h (+3)
  • (modified) clang-tools-extra/clangd/ProjectModules.h (+7)
  • (modified) clang-tools-extra/clangd/ScanningProjectModules.cpp (+24-15)
  • (modified) clang-tools-extra/clangd/unittests/PrerequisiteModulesTest.cpp (+46-3)
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());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clangd] C++20 modules support ignores CompileFlags.Add/Remove compiling a module preamble
2 participants