[clangd] Rename Module -> FeatureModule to avoid confusion. NFC
authorSam McCall <sam.mccall@gmail.com>
Thu, 4 Mar 2021 15:21:01 +0000 (16:21 +0100)
committerSam McCall <sam.mccall@gmail.com>
Fri, 5 Mar 2021 09:04:00 +0000 (10:04 +0100)
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

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/FeatureModule.cpp [moved from clang-tools-extra/clangd/Module.cpp with 65% similarity]
clang-tools-extra/clangd/FeatureModule.h [moved from clang-tools-extra/clangd/Module.h with 71% similarity]
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

index bf654a2..a6229a2 100644 (file)
@@ -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
index ba263d1..df5377d 100644 (file)
@@ -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);
   }
 
index d24271a..cbd9677 100644 (file)
@@ -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;
index fc710ae..5254bfe 100644 (file)
@@ -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 @@ public:
     /// Enable preview of FoldingRanges feature.
     bool FoldingRanges = false;
 
-    ModuleSet *Modules = nullptr;
+    FeatureModuleSet *FeatureModules = nullptr;
 
     explicit operator TUScheduler::Options() const;
   };
@@ -172,13 +172,13 @@ public:
                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 @@ public:
   void profile(MemoryTree &MT) const;
 
 private:
-  ModuleSet *Modules;
+  FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
   const ThreadsafeFS &TFS;
 
similarity index 65%
rename from clang-tools-extra/clangd/Module.cpp
rename to clang-tools-extra/clangd/FeatureModule.cpp
index 051f1b4..85977aa 100644 (file)
@@ -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));
similarity index 71%
rename from clang-tools-extra/clangd/Module.h
rename to clang-tools-extra/clangd/FeatureModule.h
index f660eff..337fa24 100644 (file)
@@ -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 @@ public:
   /// 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 @@ protected:
   /// 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 @@ private:
   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 @@ public:
     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
index 695ed89..c01a1d2 100644 (file)
@@ -42,7 +42,7 @@ protected:
     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 @@ protected:
 
   MockFS FS;
   ClangdLSPServer::Options Opts;
-  ModuleSet Modules;
+  FeatureModuleSet FeatureModules;
 
 private:
   // Color logs so we can distinguish them from test output.
@@ -227,7 +227,7 @@ CompileFlags:
 }
 
 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