Revert "[clangd] Add feature modules registry" #154711
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit ff5767a.
Member
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Aleksandr Platonov (ArcsinX) ChangesReverts llvm/llvm-project#153756 It leads to new build bot failure. I need some time to fix this in a correct way Full diff: https://github.com/llvm/llvm-project/pull/154711.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/FeatureModule.cpp b/clang-tools-extra/clangd/FeatureModule.cpp
index b6d700134919d..872cea1443789 100644
--- a/clang-tools-extra/clangd/FeatureModule.cpp
+++ b/clang-tools-extra/clangd/FeatureModule.cpp
@@ -22,10 +22,6 @@ FeatureModule::Facilities &FeatureModule::facilities() {
return *Fac;
}
-void FeatureModuleSet::add(std::unique_ptr<FeatureModule> M) {
- Modules.push_back(std::move(M));
-}
-
bool FeatureModuleSet::addImpl(void *Key, std::unique_ptr<FeatureModule> M,
const char *Source) {
if (!Map.try_emplace(Key, M.get()).second) {
@@ -39,5 +35,3 @@ bool FeatureModuleSet::addImpl(void *Key, std::unique_ptr<FeatureModule> M,
} // namespace clangd
} // namespace clang
-
-LLVM_INSTANTIATE_REGISTRY(clang::clangd::FeatureModuleRegistry)
diff --git a/clang-tools-extra/clangd/FeatureModule.h b/clang-tools-extra/clangd/FeatureModule.h
index 075db954a606a..7b6883507be3f 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -15,7 +15,6 @@
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/JSON.h"
-#include "llvm/Support/Registry.h"
#include <memory>
#include <optional>
#include <type_traits>
@@ -144,14 +143,9 @@ class FeatureModule {
/// A FeatureModuleSet is a collection of feature modules installed in clangd.
///
-/// Modules added with explicit type specification can be looked up by type, or
-/// used via the FeatureModule interface. This allows individual modules to
-/// expose a public API. For this reason, there can be only one feature module
-/// of each type.
-///
-/// Modules added using a base class pointer can be used only via the
-/// FeatureModule interface and can't be looked up by type, thus custom public
-/// API (if provided by the module) can't be used.
+/// Modules can be looked up by type, or used via the FeatureModule interface.
+/// This allows individual modules to expose a public API.
+/// For this reason, there can be only one feature module of each type.
///
/// The set owns the modules. It is itself owned by main, not ClangdServer.
class FeatureModuleSet {
@@ -178,7 +172,6 @@ class FeatureModuleSet {
const_iterator begin() const { return const_iterator(Modules.begin()); }
const_iterator end() const { return const_iterator(Modules.end()); }
- void add(std::unique_ptr<FeatureModule> M);
template <typename Mod> bool add(std::unique_ptr<Mod> M) {
return addImpl(&ID<Mod>::Key, std::move(M), LLVM_PRETTY_FUNCTION);
}
@@ -192,8 +185,6 @@ class FeatureModuleSet {
template <typename Mod> int FeatureModuleSet::ID<Mod>::Key;
-using FeatureModuleRegistry = llvm::Registry<FeatureModule>;
-
} // namespace clangd
} // namespace clang
#endif
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 827233dd6486c..f287439f10cab 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -1017,14 +1017,6 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
: static_cast<int>(ErrorResultCode::CheckFailed);
}
- FeatureModuleSet ModuleSet;
- for (FeatureModuleRegistry::entry E : FeatureModuleRegistry::entries()) {
- vlog("Adding feature module '{0}' ({1})", E.getName(), E.getDesc());
- ModuleSet.add(E.instantiate());
- }
- if (ModuleSet.begin() != ModuleSet.end())
- Opts.FeatureModules = &ModuleSet;
-
// Initialize and run ClangdLSPServer.
// Change stdin to binary to not lose \r\n on windows.
llvm::sys::ChangeStdinToBinary();
|
kadircet
approved these changes
Aug 21, 2025
Member
kadircet
left a comment
kadircet
left a comment
There was a problem hiding this comment.
thanks, for the future, feel free to land reverts (espeically of your own commits) without review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #153756
It leads to new build bot failure.
https://lab.llvm.org/buildbot/#/builders/145/builds/9200
I need some time to fix this in a correct way