Skip to content

Commit a60d06d

Browse files
committed
[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
1 parent 0766981 commit a60d06d

File tree

7 files changed

+69
-69
lines changed

7 files changed

+69
-69
lines changed

Diff for: clang-tools-extra/clangd/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ add_clang_library(clangDaemon
6262
DraftStore.cpp
6363
DumpAST.cpp
6464
ExpectedTypes.cpp
65+
FeatureModule.cpp
6566
FindSymbols.cpp
6667
FindTarget.cpp
6768
FileDistance.cpp
@@ -75,7 +76,6 @@ add_clang_library(clangDaemon
7576
Hover.cpp
7677
IncludeFixer.cpp
7778
JSONTransport.cpp
78-
Module.cpp
7979
PathMapping.cpp
8080
Protocol.cpp
8181
Quality.cpp

Diff for: clang-tools-extra/clangd/ClangdLSPServer.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -580,8 +580,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
580580
{
581581
LSPBinder Binder(Handlers, *this);
582582
bindMethods(Binder, Params.capabilities);
583-
if (Opts.Modules)
584-
for (auto &Mod : *Opts.Modules)
583+
if (Opts.FeatureModules)
584+
for (auto &Mod : *Opts.FeatureModules)
585585
Mod.initializeLSP(Binder, Params.rawCapabilities, ServerCaps);
586586
}
587587

Diff for: clang-tools-extra/clangd/ClangdServer.cpp

+9-9
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ ClangdServer::Options::operator TUScheduler::Options() const {
129129
ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
130130
const ThreadsafeFS &TFS, const Options &Opts,
131131
Callbacks *Callbacks)
132-
: Modules(Opts.Modules), CDB(CDB), TFS(TFS),
132+
: FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
133133
DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
134134
ClangTidyProvider(Opts.ClangTidyProvider),
135135
WorkspaceRoot(Opts.WorkspaceRoot) {
@@ -168,13 +168,13 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
168168
if (DynamicIdx)
169169
AddIndex(DynamicIdx.get());
170170

171-
if (Opts.Modules) {
172-
Module::Facilities F{
171+
if (Opts.FeatureModules) {
172+
FeatureModule::Facilities F{
173173
*this->WorkScheduler,
174174
this->Index,
175175
this->TFS,
176176
};
177-
for (auto &Mod : *Opts.Modules)
177+
for (auto &Mod : *Opts.FeatureModules)
178178
Mod.initialize(F);
179179
}
180180
}
@@ -184,11 +184,11 @@ ClangdServer::~ClangdServer() {
184184
// otherwise access members concurrently.
185185
// (Nobody can be using TUScheduler because we're on the main thread).
186186
WorkScheduler.reset();
187-
// Now requests have stopped, we can shut down modules.
188-
if (Modules) {
189-
for (auto &Mod : *Modules)
187+
// Now requests have stopped, we can shut down feature modules.
188+
if (FeatureModules) {
189+
for (auto &Mod : *FeatureModules)
190190
Mod.stop();
191-
for (auto &Mod : *Modules)
191+
for (auto &Mod : *FeatureModules)
192192
Mod.blockUntilIdle(Deadline::infinity());
193193
}
194194
}
@@ -920,7 +920,7 @@ ClangdServer::blockUntilIdleForTest(llvm::Optional<double> TimeoutSeconds) {
920920
return false;
921921
if (BackgroundIdx && !BackgroundIdx->blockUntilIdleForTest(Timeout))
922922
return false;
923-
if (Modules && llvm::any_of(*Modules, [&](Module &M) {
923+
if (FeatureModules && llvm::any_of(*FeatureModules, [&](FeatureModule &M) {
924924
return !M.blockUntilIdle(timeoutSeconds(Timeout));
925925
}))
926926
return false;

Diff for: clang-tools-extra/clangd/ClangdServer.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@
1313
#include "CodeComplete.h"
1414
#include "ConfigProvider.h"
1515
#include "DraftStore.h"
16+
#include "FeatureModule.h"
1617
#include "GlobalCompilationDatabase.h"
1718
#include "Hover.h"
18-
#include "Module.h"
1919
#include "Protocol.h"
2020
#include "SemanticHighlighting.h"
2121
#include "TUScheduler.h"
@@ -153,7 +153,7 @@ class ClangdServer {
153153
/// Enable preview of FoldingRanges feature.
154154
bool FoldingRanges = false;
155155

156-
ModuleSet *Modules = nullptr;
156+
FeatureModuleSet *FeatureModules = nullptr;
157157

158158
explicit operator TUScheduler::Options() const;
159159
};
@@ -172,13 +172,13 @@ class ClangdServer {
172172
const Options &Opts, Callbacks *Callbacks = nullptr);
173173
~ClangdServer();
174174

175-
/// Gets the installed module of a given type, if any.
176-
/// This exposes access the public interface of modules that have one.
177-
template <typename Mod> Mod *getModule() {
178-
return Modules ? Modules->get<Mod>() : nullptr;
175+
/// Gets the installed feature module of a given type, if any.
176+
/// This exposes access the public interface of feature modules that have one.
177+
template <typename Mod> Mod *featureModule() {
178+
return FeatureModules ? FeatureModules->get<Mod>() : nullptr;
179179
}
180-
template <typename Mod> const Mod *getModule() const {
181-
return Modules ? Modules->get<Mod>() : nullptr;
180+
template <typename Mod> const Mod *featureModule() const {
181+
return FeatureModules ? FeatureModules->get<Mod>() : nullptr;
182182
}
183183

184184
/// Add a \p File to the list of tracked C++ files or update the contents if
@@ -361,7 +361,7 @@ class ClangdServer {
361361
void profile(MemoryTree &MT) const;
362362

363363
private:
364-
ModuleSet *Modules;
364+
FeatureModuleSet *FeatureModules;
365365
const GlobalCompilationDatabase &CDB;
366366
const ThreadsafeFS &TFS;
367367

Diff for: clang-tools-extra/clangd/Module.cpp renamed to clang-tools-extra/clangd/FeatureModule.cpp

+7-7
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,32 @@
1-
//===--- Module.cpp - Plugging features into clangd -----------------------===//
1+
//===--- FeatureModule.cpp - Plugging features into clangd ----------------===//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include "Module.h"
9+
#include "FeatureModule.h"
1010
#include "support/Logger.h"
1111

1212
namespace clang {
1313
namespace clangd {
1414

15-
void Module::initialize(const Facilities &F) {
15+
void FeatureModule::initialize(const Facilities &F) {
1616
assert(!Fac.hasValue() && "Initialized twice");
1717
Fac.emplace(F);
1818
}
1919

20-
Module::Facilities &Module::facilities() {
20+
FeatureModule::Facilities &FeatureModule::facilities() {
2121
assert(Fac.hasValue() && "Not initialized yet");
2222
return *Fac;
2323
}
2424

25-
bool ModuleSet::addImpl(void *Key, std::unique_ptr<Module> M,
26-
const char *Source) {
25+
bool FeatureModuleSet::addImpl(void *Key, std::unique_ptr<FeatureModule> M,
26+
const char *Source) {
2727
if (!Map.try_emplace(Key, M.get()).second) {
2828
// Source should (usually) include the name of the concrete module type.
29-
elog("Tried to register duplicate modules via {0}", Source);
29+
elog("Tried to register duplicate feature modules via {0}", Source);
3030
return false;
3131
}
3232
Modules.push_back(std::move(M));

Diff for: clang-tools-extra/clangd/Module.h renamed to clang-tools-extra/clangd/FeatureModule.h

+30-30
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
//===--- Module.h - Plugging features into clangd -----------------*-C++-*-===//
1+
//===--- FeatureModule.h - Plugging features into clangd ----------*-C++-*-===//
22
//
33
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
44
// See https://llvm.org/LICENSE.txt for license information.
55
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
10-
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_MODULE_H
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FEATUREMODULE_H
1111

1212
#include "support/Function.h"
1313
#include "support/Threading.h"
@@ -26,11 +26,11 @@ class SymbolIndex;
2626
class ThreadsafeFS;
2727
class TUScheduler;
2828

29-
/// A Module contributes a vertical feature to clangd.
29+
/// A FeatureModule contributes a vertical feature to clangd.
3030
///
3131
/// The lifetime of a module is roughly:
32-
/// - modules are created before the LSP server, in ClangdMain.cpp
33-
/// - these modules are then passed to ClangdLSPServer in a ModuleSet
32+
/// - feature modules are created before the LSP server, in ClangdMain.cpp
33+
/// - these modules are then passed to ClangdLSPServer in a FeatureModuleSet
3434
/// - initializeLSP() is called when the editor calls initialize.
3535
// - initialize() is then called by ClangdServer as it is constructed.
3636
/// - module hooks can be called by the server at this point.
@@ -39,31 +39,31 @@ class TUScheduler;
3939
/// FIXME: Block server shutdown until all the modules are idle.
4040
/// - When shutting down, ClangdServer will wait for all requests to
4141
/// finish, call stop(), and then blockUntilIdle().
42-
/// - modules will be destroyed after ClangdLSPServer is destroyed.
42+
/// - feature modules will be destroyed after ClangdLSPServer is destroyed.
4343
///
44-
/// Modules are not threadsafe in general. A module's entrypoints are:
44+
/// FeatureModules are not threadsafe in general. A module's entrypoints are:
4545
/// - method handlers registered in initializeLSP()
46-
/// - public methods called directly via ClangdServer.getModule<T>()->...
47-
/// - specific overridable "hook" methods inherited from Module
46+
/// - public methods called directly via ClangdServer.featureModule<T>()->...
47+
/// - specific overridable "hook" methods inherited from FeatureModule
4848
/// Unless otherwise specified, these are only called on the main thread.
4949
///
50-
/// Conventionally, standard modules live in the `clangd` namespace, and other
51-
/// exposed details live in a sub-namespace.
52-
class Module {
50+
/// Conventionally, standard feature modules live in the `clangd` namespace,
51+
/// and other exposed details live in a sub-namespace.
52+
class FeatureModule {
5353
public:
54-
virtual ~Module() {
54+
virtual ~FeatureModule() {
5555
/// Perform shutdown sequence on destruction in case the ClangdServer was
5656
/// never initialized. Usually redundant, but shutdown is idempotent.
5757
stop();
5858
blockUntilIdle(Deadline::infinity());
5959
}
6060

61-
/// Called by the server to connect this module to LSP.
61+
/// Called by the server to connect this feature module to LSP.
6262
/// The module should register the methods/notifications/commands it handles,
6363
/// and update the server capabilities to advertise them.
6464
///
6565
/// This is only called if the module is running in ClangdLSPServer!
66-
/// Modules with a public interface should satisfy it without LSP bindings.
66+
/// FeatureModules with a public interface should work without LSP bindings.
6767
virtual void initializeLSP(LSPBinder &Bind,
6868
const llvm::json::Object &ClientCaps,
6969
llvm::json::Object &ServerCaps) {}
@@ -87,7 +87,7 @@ class Module {
8787
/// Waits until the module is idle (no background work) or a deadline expires.
8888
/// In general all modules should eventually go idle, though it may take a
8989
/// long time (e.g. background indexing).
90-
/// Modules should go idle quickly if stop() has been called.
90+
/// FeatureModules should go idle quickly if stop() has been called.
9191
/// Called by the server when shutting down, and also by tests.
9292
virtual bool blockUntilIdle(Deadline) { return true; }
9393

@@ -101,7 +101,7 @@ class Module {
101101
/// The filesystem is used to read source files on disk.
102102
const ThreadsafeFS &fs() { return facilities().FS; }
103103

104-
/// Types of function objects that modules use for outgoing calls.
104+
/// Types of function objects that feature modules use for outgoing calls.
105105
/// (Bound throuh LSPBinder, made available here for convenience).
106106
template <typename P>
107107
using OutgoingNotification = llvm::unique_function<void(const P &)>;
@@ -112,28 +112,28 @@ class Module {
112112
llvm::Optional<Facilities> Fac;
113113
};
114114

115-
/// A ModuleSet is a collection of modules installed in clangd.
115+
/// A FeatureModuleSet is a collection of feature modules installed in clangd.
116116
///
117-
/// Modules can be looked up by type, or used through the Module interface.
117+
/// Modules can be looked up by type, or used via the FeatureModule interface.
118118
/// This allows individual modules to expose a public API.
119-
/// For this reason, there can be only one module of each type.
119+
/// For this reason, there can be only one feature module of each type.
120120
///
121-
/// ModuleSet owns the modules. It is itself owned by main, not ClangdServer.
122-
class ModuleSet {
123-
std::vector<std::unique_ptr<Module>> Modules;
124-
llvm::DenseMap<void *, Module *> Map;
121+
/// The set owns the modules. It is itself owned by main, not ClangdServer.
122+
class FeatureModuleSet {
123+
std::vector<std::unique_ptr<FeatureModule>> Modules;
124+
llvm::DenseMap<void *, FeatureModule *> Map;
125125

126126
template <typename Mod> struct ID {
127-
static_assert(std::is_base_of<Module, Mod>::value &&
127+
static_assert(std::is_base_of<FeatureModule, Mod>::value &&
128128
std::is_final<Mod>::value,
129129
"Modules must be final classes derived from clangd::Module");
130130
static int Key;
131131
};
132132

133-
bool addImpl(void *Key, std::unique_ptr<Module>, const char *Source);
133+
bool addImpl(void *Key, std::unique_ptr<FeatureModule>, const char *Source);
134134

135135
public:
136-
ModuleSet() = default;
136+
FeatureModuleSet() = default;
137137

138138
using iterator = llvm::pointee_iterator<decltype(Modules)::iterator>;
139139
using const_iterator =
@@ -150,11 +150,11 @@ class ModuleSet {
150150
return static_cast<Mod *>(Map.lookup(&ID<Mod>::Key));
151151
}
152152
template <typename Mod> const Mod *get() const {
153-
return const_cast<ModuleSet *>(this)->get<Mod>();
153+
return const_cast<FeatureModuleSet *>(this)->get<Mod>();
154154
}
155155
};
156156

157-
template <typename Mod> int ModuleSet::ID<Mod>::Key;
157+
template <typename Mod> int FeatureModuleSet::ID<Mod>::Key;
158158

159159
} // namespace clangd
160160
} // namespace clang

Diff for: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

+11-11
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class LSPTest : public ::testing::Test, private clangd::Logger {
4242
Base = ClangdServer::optsForTest();
4343
// This is needed to we can test index-based operations like call hierarchy.
4444
Base.BuildDynamicSymbolIndex = true;
45-
Base.Modules = &Modules;
45+
Base.FeatureModules = &FeatureModules;
4646
}
4747

4848
LSPClient &start() {
@@ -70,7 +70,7 @@ class LSPTest : public ::testing::Test, private clangd::Logger {
7070

7171
MockFS FS;
7272
ClangdLSPServer::Options Opts;
73-
ModuleSet Modules;
73+
FeatureModuleSet FeatureModules;
7474

7575
private:
7676
// Color logs so we can distinguish them from test output.
@@ -227,7 +227,7 @@ TEST_F(LSPTest, CDBConfigIntegration) {
227227
}
228228

229229
TEST_F(LSPTest, ModulesTest) {
230-
class MathModule final : public Module {
230+
class MathModule final : public FeatureModule {
231231
OutgoingNotification<int> Changed;
232232
void initializeLSP(LSPBinder &Bind, const llvm::json::Object &ClientCaps,
233233
llvm::json::Object &ServerCaps) override {
@@ -248,7 +248,7 @@ TEST_F(LSPTest, ModulesTest) {
248248
[Reply(std::move(Reply)), Value(Value)]() mutable { Reply(Value); });
249249
}
250250
};
251-
Modules.add(std::make_unique<MathModule>());
251+
FeatureModules.add(std::make_unique<MathModule>());
252252

253253
auto &Client = start();
254254
Client.notify("add", 2);
@@ -266,10 +266,10 @@ capture(llvm::Optional<llvm::Expected<T>> &Out) {
266266
return [&Out](llvm::Expected<T> V) { Out.emplace(std::move(V)); };
267267
}
268268

269-
TEST_F(LSPTest, ModulesThreadingTest) {
270-
// A module that does its work on a background thread, and so exercises the
271-
// block/shutdown protocol.
272-
class AsyncCounter final : public Module {
269+
TEST_F(LSPTest, FeatureModulesThreadingTest) {
270+
// A feature module that does its work on a background thread, and so
271+
// exercises the block/shutdown protocol.
272+
class AsyncCounter final : public FeatureModule {
273273
bool ShouldStop = false;
274274
int State = 0;
275275
std::deque<Callback<int>> Queue; // null = increment, non-null = read.
@@ -347,19 +347,19 @@ TEST_F(LSPTest, ModulesThreadingTest) {
347347
}
348348
};
349349

350-
Modules.add(std::make_unique<AsyncCounter>());
350+
FeatureModules.add(std::make_unique<AsyncCounter>());
351351
auto &Client = start();
352352

353353
Client.notify("increment", nullptr);
354354
Client.notify("increment", nullptr);
355355
Client.notify("increment", nullptr);
356356
EXPECT_THAT_EXPECTED(Client.call("sync", nullptr).take(), Succeeded());
357-
EXPECT_EQ(3, Modules.get<AsyncCounter>()->getSync());
357+
EXPECT_EQ(3, FeatureModules.get<AsyncCounter>()->getSync());
358358
// Throw some work on the queue to make sure shutdown blocks on it.
359359
Client.notify("increment", nullptr);
360360
Client.notify("increment", nullptr);
361361
Client.notify("increment", nullptr);
362-
// And immediately shut down. Module destructor verifies that we blocked.
362+
// And immediately shut down. FeatureModule destructor verifies we blocked.
363363
}
364364

365365
} // namespace

0 commit comments

Comments
 (0)