Reland "[clangd] Add feature modules registry"#154836
Merged
Merged
Conversation
This patch adds feature modules registry, which allows to discover registered feature modules and add them into the clangd's feature module set
Member
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: Aleksandr Platonov (ArcsinX) ChangesReland #153756 LLVM_INSTANTIATE_REGISTRY(..) and FeatureModuleRegistry::entries() were in different translation units, that rises a warning when compiling with clang and leads to compilation failure if -Werror flag is set. Full diff: https://github.com/llvm/llvm-project/pull/154836.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/FeatureModule.cpp b/clang-tools-extra/clangd/FeatureModule.cpp
index 872cea1443789..38461e3db4e6b 100644
--- a/clang-tools-extra/clangd/FeatureModule.cpp
+++ b/clang-tools-extra/clangd/FeatureModule.cpp
@@ -22,6 +22,10 @@ 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) {
@@ -33,5 +37,16 @@ bool FeatureModuleSet::addImpl(void *Key, std::unique_ptr<FeatureModule> M,
return true;
}
+FeatureModuleSet FeatureModuleSet::fromRegistry() {
+ FeatureModuleSet ModuleSet;
+ for (FeatureModuleRegistry::entry E : FeatureModuleRegistry::entries()) {
+ vlog("Adding feature module '{0}' ({1})", E.getName(), E.getDesc());
+ ModuleSet.add(E.instantiate());
+ }
+ return ModuleSet;
+}
+
} // 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 7b6883507be3f..ee65aa8a59ed2 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -15,6 +15,7 @@
#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>
@@ -143,9 +144,14 @@ class FeatureModule {
/// A FeatureModuleSet is a collection of feature modules installed in clangd.
///
-/// 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.
+/// 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.
///
/// The set owns the modules. It is itself owned by main, not ClangdServer.
class FeatureModuleSet {
@@ -164,6 +170,8 @@ class FeatureModuleSet {
public:
FeatureModuleSet() = default;
+ static FeatureModuleSet fromRegistry();
+
using iterator = llvm::pointee_iterator<decltype(Modules)::iterator>;
using const_iterator =
llvm::pointee_iterator<decltype(Modules)::const_iterator>;
@@ -172,6 +180,7 @@ 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);
}
@@ -185,6 +194,8 @@ 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 f287439f10cab..1856d4f47ffc5 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 "FeatureModule.h"
#include "IncludeCleaner.h"
#include "PathMapping.h"
#include "Protocol.h"
@@ -1017,6 +1018,10 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
: static_cast<int>(ErrorResultCode::CheckFailed);
}
+ FeatureModuleSet ModuleSet = FeatureModuleSet::fromRegistry();
+ 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
requested changes
Aug 26, 2025
Contributor
Author
|
@kadircet Sorry to bother you. Can you please take a look on this once again? |
kadircet
approved these changes
Sep 17, 2025
kadircet
left a comment
Member
There was a problem hiding this comment.
thanks a lot! sorry i got tangled up in couple of other things, hoping to catch up on rest of the reviews soon.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/16372 Here is the relevant piece of the build log for the reference |
ckoparkar
added a commit
to ckoparkar/llvm-project
that referenced
this pull request
Sep 23, 2025
* main: (1562 commits) Document Policy on supporting newer C++ standard in LLVM codebase (llvm#156823) [MLIR][Transform][SMT] Introduce transform.smt.constrain_params (llvm#159450) Reapply "[compiler-rt] Remove %T from shared object substitutions (llvm#155302)" [NFC] [IndVarSimplify] Add non-overflowing usub test (llvm#159683) [Github] Remove separate tools checkout from pr-code workflows (llvm#159967) [clang] fix using enum redecl in template regression (llvm#159996) [DAG] Skip `mstore` combine for `<1 x ty>` vectors (llvm#159915) [mlir] Expose optional `PatternBenefit` to `func` populate functions (NFC) (llvm#159986) [LV] Set correct costs for interleave group members. [clang] ast-dump: use template pattern for `instantiated_from` (llvm#159952) [ARM] ha-alignstack-call.ll - regenerate test checks (llvm#159988) [LLD][MachO] Silence warning when building with MSVC [llvm][Analysis] Silence warning when building with MSVC [LV] Skip select cost for invariant divisors in legacy cost model. [Clang] Fix an error-recovery crash after d1a80de (llvm#159976) [VPlanPatternMatch] Introduce m_ConstantInt (llvm#159558) [GlobalISel] Add G_ABS computeKnownBits (llvm#154413) [gn build] Port 4cabd1e Reland "[clangd] Add feature modules registry" (llvm#154836) [LV] Also handle non-uniform scalarized loads when processing AddrDefs. ...
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Reland #153756
LLVM_INSTANTIATE_REGISTRY(..) and FeatureModuleRegistry::entries() were in different translation units, that rises a warning when compiling with clang and leads to compilation failure if -Werror flag is set.
Instead of iterating directly in the main function, a static method FeatureModuleSet::fromRegistry() was added and it is called in the main function.