[clang-tools-extra] a60d06d - [clangd] Rename Module -> FeatureModule to avoid confusion. NFC
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 5 01:15:04 PST 2021
Author: Sam McCall
Date: 2021-03-05T10:04:00+01:00
New Revision: a60d06d8b757b656726b70e83e76417297c7057b
URL: https://github.com/llvm/llvm-project/commit/a60d06d8b757b656726b70e83e76417297c7057b
DIFF: https://github.com/llvm/llvm-project/commit/a60d06d8b757b656726b70e83e76417297c7057b.diff
LOG: [clangd] Rename Module -> FeatureModule to avoid confusion. NFC
As pointed out in D96244, "Module" is already pretty overloaded to refer
to clang and llvm modules. (And clangd deals directly with the former).
FeatureModule is a bit of a mouthful but it's pretty self-descriptive.
I think it might be better than "Component" which doesn't really capture
the "common interface" aspect - it's IMO confusing to refer to
"components" but exclude CDB for example.
Differential Revision: https://reviews.llvm.org/D97950
Added:
clang-tools-extra/clangd/FeatureModule.cpp
clang-tools-extra/clangd/FeatureModule.h
Modified:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
Removed:
clang-tools-extra/clangd/Module.cpp
clang-tools-extra/clangd/Module.h
################################################################################
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt
index bf654a23e00c..a6229a229002 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -62,6 +62,7 @@ add_clang_library(clangDaemon
DraftStore.cpp
DumpAST.cpp
ExpectedTypes.cpp
+ FeatureModule.cpp
FindSymbols.cpp
FindTarget.cpp
FileDistance.cpp
@@ -75,7 +76,6 @@ add_clang_library(clangDaemon
Hover.cpp
IncludeFixer.cpp
JSONTransport.cpp
- Module.cpp
PathMapping.cpp
Protocol.cpp
Quality.cpp
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index ba263d123f1d..df5377d77f2b 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -580,8 +580,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
{
LSPBinder Binder(Handlers, *this);
bindMethods(Binder, Params.capabilities);
- if (Opts.Modules)
- for (auto &Mod : *Opts.Modules)
+ if (Opts.FeatureModules)
+ for (auto &Mod : *Opts.FeatureModules)
Mod.initializeLSP(Binder, Params.rawCapabilities, ServerCaps);
}
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index d24271aafeae..cbd9677f340c 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -129,7 +129,7 @@ ClangdServer::Options::operator TUScheduler::Options() const {
ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
const ThreadsafeFS &TFS, const Options &Opts,
Callbacks *Callbacks)
- : Modules(Opts.Modules), CDB(CDB), TFS(TFS),
+ : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
ClangTidyProvider(Opts.ClangTidyProvider),
WorkspaceRoot(Opts.WorkspaceRoot) {
@@ -168,13 +168,13 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
if (DynamicIdx)
AddIndex(DynamicIdx.get());
- if (Opts.Modules) {
- Module::Facilities F{
+ if (Opts.FeatureModules) {
+ FeatureModule::Facilities F{
*this->WorkScheduler,
this->Index,
this->TFS,
};
- for (auto &Mod : *Opts.Modules)
+ for (auto &Mod : *Opts.FeatureModules)
Mod.initialize(F);
}
}
@@ -184,11 +184,11 @@ ClangdServer::~ClangdServer() {
// otherwise access members concurrently.
// (Nobody can be using TUScheduler because we're on the main thread).
WorkScheduler.reset();
- // Now requests have stopped, we can shut down modules.
- if (Modules) {
- for (auto &Mod : *Modules)
+ // Now requests have stopped, we can shut down feature modules.
+ if (FeatureModules) {
+ for (auto &Mod : *FeatureModules)
Mod.stop();
- for (auto &Mod : *Modules)
+ for (auto &Mod : *FeatureModules)
Mod.blockUntilIdle(Deadline::infinity());
}
}
@@ -920,7 +920,7 @@ ClangdServer::blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds) {
return false;
if (BackgroundIdx && !BackgroundIdx->blockUntilIdleForTest(Timeout))
return false;
- if (Modules && llvm::any_of(*Modules, [&](Module &M) {
+ if (FeatureModules && llvm::any_of(*FeatureModules, [&](FeatureModule &M) {
return !M.blockUntilIdle(timeoutSeconds(Timeout));
}))
return false;
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index fc710ae4724d..5254bfe78b71 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -13,9 +13,9 @@
#include "CodeComplete.h"
#include "ConfigProvider.h"
#include "DraftStore.h"
+#include "FeatureModule.h"
#include "GlobalCompilationDatabase.h"
#include "Hover.h"
-#include "Module.h"
#include "Protocol.h"
#include "SemanticHighlighting.h"
#include "TUScheduler.h"
@@ -153,7 +153,7 @@ class ClangdServer {
/// Enable preview of FoldingRanges feature.
bool FoldingRanges = false;
- ModuleSet *Modules = nullptr;
+ FeatureModuleSet *FeatureModules = nullptr;
explicit operator TUScheduler::Options() const;
};
@@ -172,13 +172,13 @@ class ClangdServer {
const Options &Opts, Callbacks *Callbacks = nullptr);
~ClangdServer();
- /// Gets the installed module of a given type, if any.
- /// This exposes access the public interface of modules that have one.
- template <typename Mod> Mod *getModule() {
- return Modules ? Modules->get<Mod>() : nullptr;
+ /// Gets the installed feature module of a given type, if any.
+ /// This exposes access the public interface of feature modules that have one.
+ template <typename Mod> Mod *featureModule() {
+ return FeatureModules ? FeatureModules->get<Mod>() : nullptr;
}
- template <typename Mod> const Mod *getModule() const {
- return Modules ? Modules->get<Mod>() : nullptr;
+ template <typename Mod> const Mod *featureModule() const {
+ return FeatureModules ? FeatureModules->get<Mod>() : nullptr;
}
/// Add a \p File to the list of tracked C++ files or update the contents if
@@ -361,7 +361,7 @@ class ClangdServer {
void profile(MemoryTree &MT) const;
private:
- ModuleSet *Modules;
+ FeatureModuleSet *FeatureModules;
const GlobalCompilationDatabase &CDB;
const ThreadsafeFS &TFS;
diff --git a/clang-tools-extra/clangd/Module.cpp b/clang-tools-extra/clangd/FeatureModule.cpp
similarity index 65%
rename from clang-tools-extra/clangd/Module.cpp
rename to clang-tools-extra/clangd/FeatureModule.cpp
index 051f1b45c837..85977aadd6e3 100644
--- a/clang-tools-extra/clangd/Module.cpp
+++ b/clang-tools-extra/clangd/FeatureModule.cpp
@@ -1,4 +1,4 @@
-//===--- Module.cpp - Plugging features into clangd -----------------------===//
+//===--- FeatureModule.cpp - Plugging features into clangd ----------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,27 +6,27 @@
//
//===----------------------------------------------------------------------===//
-#include "Module.h"
+#include "FeatureModule.h"
#include "support/Logger.h"
namespace clang {
namespace clangd {
-void Module::initialize(const Facilities &F) {
+void FeatureModule::initialize(const Facilities &F) {
assert(!Fac.hasValue() && "Initialized twice");
Fac.emplace(F);
}
-Module::Facilities &Module::facilities() {
+FeatureModule::Facilities &FeatureModule::facilities() {
assert(Fac.hasValue() && "Not initialized yet");
return *Fac;
}
-bool ModuleSet::addImpl(void *Key, std::unique_ptr<Module> M,
- const char *Source) {
+bool FeatureModuleSet::addImpl(void *Key, std::unique_ptr<FeatureModule> M,
+ const char *Source) {
if (!Map.try_emplace(Key, M.get()).second) {
// Source should (usually) include the name of the concrete module type.
- elog("Tried to register duplicate modules via {0}", Source);
+ elog("Tried to register duplicate feature modules via {0}", Source);
return false;
}
Modules.push_back(std::move(M));
diff --git a/clang-tools-extra/clangd/Module.h b/clang-tools-extra/clangd/FeatureModule.h
similarity index 71%
rename from clang-tools-extra/clangd/Module.h
rename to clang-tools-extra/clangd/FeatureModule.h
index f660eff85653..337fa24e9454 100644
--- a/clang-tools-extra/clangd/Module.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -1,4 +1,4 @@
-//===--- Module.h - Plugging features into clangd -----------------*-C++-*-===//
+//===--- FeatureModule.h - Plugging features into clangd ----------*-C++-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
#include "support/Function.h"
#include "support/Threading.h"
@@ -26,11 +26,11 @@ class SymbolIndex;
class ThreadsafeFS;
class TUScheduler;
-/// A Module contributes a vertical feature to clangd.
+/// A FeatureModule contributes a vertical feature to clangd.
///
/// The lifetime of a module is roughly:
-/// - modules are created before the LSP server, in ClangdMain.cpp
-/// - these modules are then passed to ClangdLSPServer in a ModuleSet
+/// - feature modules are created before the LSP server, in ClangdMain.cpp
+/// - these modules are then passed to ClangdLSPServer in a FeatureModuleSet
/// - initializeLSP() is called when the editor calls initialize.
// - initialize() is then called by ClangdServer as it is constructed.
/// - module hooks can be called by the server at this point.
@@ -39,31 +39,31 @@ class TUScheduler;
/// FIXME: Block server shutdown until all the modules are idle.
/// - When shutting down, ClangdServer will wait for all requests to
/// finish, call stop(), and then blockUntilIdle().
-/// - modules will be destroyed after ClangdLSPServer is destroyed.
+/// - feature modules will be destroyed after ClangdLSPServer is destroyed.
///
-/// Modules are not threadsafe in general. A module's entrypoints are:
+/// FeatureModules are not threadsafe in general. A module's entrypoints are:
/// - method handlers registered in initializeLSP()
-/// - public methods called directly via ClangdServer.getModule<T>()->...
-/// - specific overridable "hook" methods inherited from Module
+/// - public methods called directly via ClangdServer.featureModule<T>()->...
+/// - specific overridable "hook" methods inherited from FeatureModule
/// Unless otherwise specified, these are only called on the main thread.
///
-/// Conventionally, standard modules live in the `clangd` namespace, and other
-/// exposed details live in a sub-namespace.
-class Module {
+/// Conventionally, standard feature modules live in the `clangd` namespace,
+/// and other exposed details live in a sub-namespace.
+class FeatureModule {
public:
- virtual ~Module() {
+ virtual ~FeatureModule() {
/// Perform shutdown sequence on destruction in case the ClangdServer was
/// never initialized. Usually redundant, but shutdown is idempotent.
stop();
blockUntilIdle(Deadline::infinity());
}
- /// Called by the server to connect this module to LSP.
+ /// Called by the server to connect this feature module to LSP.
/// The module should register the methods/notifications/commands it handles,
/// and update the server capabilities to advertise them.
///
/// This is only called if the module is running in ClangdLSPServer!
- /// Modules with a public interface should satisfy it without LSP bindings.
+ /// FeatureModules with a public interface should work without LSP bindings.
virtual void initializeLSP(LSPBinder &Bind,
const llvm::json::Object &ClientCaps,
llvm::json::Object &ServerCaps) {}
@@ -87,7 +87,7 @@ class Module {
/// Waits until the module is idle (no background work) or a deadline expires.
/// In general all modules should eventually go idle, though it may take a
/// long time (e.g. background indexing).
- /// Modules should go idle quickly if stop() has been called.
+ /// FeatureModules should go idle quickly if stop() has been called.
/// Called by the server when shutting down, and also by tests.
virtual bool blockUntilIdle(Deadline) { return true; }
@@ -101,7 +101,7 @@ class Module {
/// The filesystem is used to read source files on disk.
const ThreadsafeFS &fs() { return facilities().FS; }
- /// Types of function objects that modules use for outgoing calls.
+ /// Types of function objects that feature modules use for outgoing calls.
/// (Bound throuh LSPBinder, made available here for convenience).
template <typename P>
using OutgoingNotification = llvm::unique_function<void(const P &)>;
@@ -112,28 +112,28 @@ class Module {
llvm::Optional<Facilities> Fac;
};
-/// A ModuleSet is a collection of modules installed in clangd.
+/// A FeatureModuleSet is a collection of feature modules installed in clangd.
///
-/// Modules can be looked up by type, or used through the Module interface.
+/// 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 module of each type.
+/// For this reason, there can be only one feature module of each type.
///
-/// ModuleSet owns the modules. It is itself owned by main, not ClangdServer.
-class ModuleSet {
- std::vector<std::unique_ptr<Module>> Modules;
- llvm::DenseMap<void *, Module *> Map;
+/// The set owns the modules. It is itself owned by main, not ClangdServer.
+class FeatureModuleSet {
+ std::vector<std::unique_ptr<FeatureModule>> Modules;
+ llvm::DenseMap<void *, FeatureModule *> Map;
template <typename Mod> struct ID {
- static_assert(std::is_base_of<Module, Mod>::value &&
+ static_assert(std::is_base_of<FeatureModule, Mod>::value &&
std::is_final<Mod>::value,
"Modules must be final classes derived from clangd::Module");
static int Key;
};
- bool addImpl(void *Key, std::unique_ptr<Module>, const char *Source);
+ bool addImpl(void *Key, std::unique_ptr<FeatureModule>, const char *Source);
public:
- ModuleSet() = default;
+ FeatureModuleSet() = default;
using iterator = llvm::pointee_iterator<decltype(Modules)::iterator>;
using const_iterator =
@@ -150,11 +150,11 @@ class ModuleSet {
return static_cast<Mod *>(Map.lookup(&ID<Mod>::Key));
}
template <typename Mod> const Mod *get() const {
- return const_cast<ModuleSet *>(this)->get<Mod>();
+ return const_cast<FeatureModuleSet *>(this)->get<Mod>();
}
};
-template <typename Mod> int ModuleSet::ID<Mod>::Key;
+template <typename Mod> int FeatureModuleSet::ID<Mod>::Key;
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 695ed89ae7f4..c01a1d2ebc88 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -42,7 +42,7 @@ class LSPTest : public ::testing::Test, private clangd::Logger {
Base = ClangdServer::optsForTest();
// This is needed to we can test index-based operations like call hierarchy.
Base.BuildDynamicSymbolIndex = true;
- Base.Modules = &Modules;
+ Base.FeatureModules = &FeatureModules;
}
LSPClient &start() {
@@ -70,7 +70,7 @@ class LSPTest : public ::testing::Test, private clangd::Logger {
MockFS FS;
ClangdLSPServer::Options Opts;
- ModuleSet Modules;
+ FeatureModuleSet FeatureModules;
private:
// Color logs so we can distinguish them from test output.
@@ -227,7 +227,7 @@ TEST_F(LSPTest, CDBConfigIntegration) {
}
TEST_F(LSPTest, ModulesTest) {
- class MathModule final : public Module {
+ class MathModule final : public FeatureModule {
OutgoingNotification<int> Changed;
void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps,
llvm::json::Object &ServerCaps) override {
@@ -248,7 +248,7 @@ TEST_F(LSPTest, ModulesTest) {
[Reply(std::move(Reply)), Value(Value)]() mutable { Reply(Value); });
}
};
- Modules.add(std::make_unique<MathModule>());
+ FeatureModules.add(std::make_unique<MathModule>());
auto &Client = start();
Client.notify("add", 2);
@@ -266,10 +266,10 @@ capture(llvm::Optional<llvm::Expected<T>> &Out) {
return [&Out](llvm::Expected<T> V) { Out.emplace(std::move(V)); };
}
-TEST_F(LSPTest, ModulesThreadingTest) {
- // A module that does its work on a background thread, and so exercises the
- // block/shutdown protocol.
- class AsyncCounter final : public Module {
+TEST_F(LSPTest, FeatureModulesThreadingTest) {
+ // A feature module that does its work on a background thread, and so
+ // exercises the block/shutdown protocol.
+ class AsyncCounter final : public FeatureModule {
bool ShouldStop = false;
int State = 0;
std::deque<Callback<int>> Queue; // null = increment, non-null = read.
@@ -347,19 +347,19 @@ TEST_F(LSPTest, ModulesThreadingTest) {
}
};
- Modules.add(std::make_unique<AsyncCounter>());
+ FeatureModules.add(std::make_unique<AsyncCounter>());
auto &Client = start();
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
- EXPECT_EQ(3, Modules.get<AsyncCounter>()->getSync());
+ EXPECT_EQ(3, FeatureModules.get<AsyncCounter>()->getSync());
// Throw some work on the queue to make sure shutdown blocks on it.
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
Client.notify("increment", nullptr);
- // And immediately shut down. Module destructor verifies that we blocked.
+ // And immediately shut down. FeatureModule destructor verifies we blocked.
}
} // namespace
More information about the cfe-commits
mailing list