Skip to content

Conversation

mpark
Copy link
Member

@mpark mpark commented Jul 1, 2024

This diff adds an overload of clangdMain that makes a FeatureModuleSet. clangdMain was initially added in 56ac9d4 to allow custom builds of clangd outside of the LLVM repo that link against clangd. Currently, there's no way for such custom builds to use their own FeatureModules.

This adds a new overload of clangdMain rather than adding a new default parameter since that would break existing code that links against the library.

It's also possible that there is a use that takes the address of clangdMain which this change would break. That seems very unlikely, and is not addressed in this diff. If this turns out to be a problem, we can add a clangdMainWithFeatures instead of adding a new overload.

This diff adds an overload of `clangdMain` that makes a `FeatureModuleSet`.
`clangdMain` was initially added in 56ac9d4 to allow custom builds
of clangd outside of the LLVM repo that link against clangd. Currently,
there's no way for such custom builds to use their own `FeatureModule`s.

This adds a new overload of `clangdMain` rather than adding a new default
parameter since that would break existing code that links against the library.

It's also possible that there is a use that takes the address of `clangdMain`
which this change would break. That seems very unlikely, and is not addressed
in this diff. If this turns out to be a problem, we can add a
`clangdMainWithFeatures` instead of adding a new overload.
@llvmbot
Copy link
Member

llvmbot commented Jul 1, 2024

@llvm/pr-subscribers-clangd

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

Author: Michael Park (mpark)

Changes

This diff adds an overload of clangdMain that makes a FeatureModuleSet. clangdMain was initially added in 56ac9d4 to allow custom builds of clangd outside of the LLVM repo that link against clangd. Currently, there's no way for such custom builds to use their own FeatureModules.

This adds a new overload of clangdMain rather than adding a new default parameter since that would break existing code that links against the library.

It's also possible that there is a use that takes the address of clangdMain which this change would break. That seems very unlikely, and is not addressed in this diff. If this turns out to be a problem, we can add a clangdMainWithFeatures instead of adding a new overload.


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

2 Files Affected:

  • (modified) clang-tools-extra/clangd/tool/ClangdMain.cpp (+6)
  • (modified) clang-tools-extra/clangd/tool/ClangdMain.h (+2)
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index c3ba655ee2dc6..4caa1425c5ab6 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -13,6 +13,7 @@
 #include "Config.h"
 #include "ConfigProvider.h"
 #include "Feature.h"
+#include "FeatureModuleSet.h"
 #include "IncludeCleaner.h"
 #include "PathMapping.h"
 #include "Protocol.h"
@@ -712,6 +713,10 @@ enum class ErrorResultCode : int {
 };
 
 int clangdMain(int argc, char *argv[]) {
+  return clangdMain(arc, argv, nullptr);
+}
+
+int clangdMain(int argc, char *argv[], FeatureModuleSet* FeatureModules) {
   // Clang could run on the main thread. e.g., when the flag '-check' or '-sync'
   // is enabled.
   clang::noteBottomOfStack();
@@ -898,6 +903,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
   Opts.StaticIndex = PAI.get();
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.MemoryCleanup = getMemoryCleanupFunction();
+  Opts.FeatureModules = FeatureModules;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.h b/clang-tools-extra/clangd/tool/ClangdMain.h
index bd0f51aac83bb..88b590be58d5e 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.h
+++ b/clang-tools-extra/clangd/tool/ClangdMain.h
@@ -11,8 +11,10 @@
 
 namespace clang {
 namespace clangd {
+class FeatureModuleSet;
 // clangd main function (clangd server loop)
 int clangdMain(int argc, char *argv[]);
+int clangdMain(int argc, char *argv[], FeatureModuleSet *FeatureModules);
 } // namespace clangd
 } // namespace clang
 

Copy link

github-actions bot commented Jul 1, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 8598bcb9934dca16ea16d87304e00defc85d986c 01ae6c9aee33d3b2b0a00484bf7c041f6b90e710 -- clang-tools-extra/clangd/tool/ClangdMain.cpp clang-tools-extra/clangd/tool/ClangdMain.h
View the diff from clang-format here.
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 4caa1425c5..4e4c0d69a1 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -716,7 +716,7 @@ int clangdMain(int argc, char *argv[]) {
   return clangdMain(arc, argv, nullptr);
 }
 
-int clangdMain(int argc, char *argv[], FeatureModuleSet* FeatureModules) {
+int clangdMain(int argc, char *argv[], FeatureModuleSet *FeatureModules) {
   // Clang could run on the main thread. e.g., when the flag '-check' or '-sync'
   // is enabled.
   clang::noteBottomOfStack();

@mpark mpark closed this Jul 1, 2024
@mpark mpark deleted the clangdMain branch July 1, 2024 05:14
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.

2 participants