[PATCH] D98498: [clangd] Enable modules to contribute tweaks.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a project: clang-tools-extra.

Should we have a test somewhere that tweaks defined in modules actually work?



================
Comment at: clang-tools-extra/clangd/FeatureModule.h:97
+  virtual void
+  contributeTweaks(std::vector<std::unique_ptr<class Tweak>> &Out) {}
+
----------------
nit: can you move the forward declaration up the top? Makes the (sort-of) dep clearer, and is less fragile re scopes when moving code around.


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:55
+  if (!Modules)
+    return All;
+  for (auto &M : *Modules)
----------------
nit: the early return seems more confusing to me here than conditionally adding...


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
 std::vector<std::unique_ptr<Tweak>>
-prepareTweaks(const Tweak::Selection &S,
+prepareTweaks(const class FeatureModuleSet *Modules, const Tweak::Selection &S,
               llvm::function_ref<bool(const Tweak &)> Filter);
----------------
forward decl: and here


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
 std::vector<std::unique_ptr<Tweak>>
-prepareTweaks(const Tweak::Selection &S,
+prepareTweaks(const class FeatureModuleSet *Modules, const Tweak::Selection &S,
               llvm::function_ref<bool(const Tweak &)> Filter);
----------------
sammccall wrote:
> forward decl: and here
nit: move the modules to the end of the list?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98498/new/

https://reviews.llvm.org/D98498