[PATCH] D56267: [clangd] Interfaces for writing code actions

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 14 07:35:47 PST 2019


sammccall added a comment.

To be a bit more concrete...

One approach which won't win any OO design points but might be quite easy to navigate: combine the ActionProvider and PreparedAction into one class.

  class SwapIfBranches : public MiniRefactoringThing {
    SwapIfBranches(); // must have a default constructor.
    const char* id() const override { return "swapIfBranches"; } // must be constant for the subclass.
    bool prepare(const Inputs&) override; // may only be called once. Returns false if this operation doesn't apply.
    std::string title() override; // REQUIRES: prepare() returned true
    Expected<tooling::Replacements> apply() override; // REQUIRES: prepare() returned true
  }

note that the "subclasses of a common interface, have a default constructor" fits exactly with the LLVM Registry pattern, if we want to move to that.
Otherwise, a `StringMap<unique_ptr<MiniRefactoringThing>>`

A more... complicated version could use something like CRTP to give a generic external interface while keeping the internal dataflow/signatures clean:

  class SwapIfBranches : public MiniRefactoringThing<SwapIfBranches> {
    struct State { ... }; // completing forward declaration from CRTP base
    Optional<State> prepare(const Inputs&) const override;
    std::string getTitle(const State&) const;
    Expected<tooling::Replacements> apply(const State&) const override;
  }
  const char MiniRefactoringThing<SwapIfBranches>::ID[] = "swapIfBranches";

Similarly we could use Registry or a `StringMap<unique_ptr<MiniRefactoringThingIface>>` where the CRTP base implements the interface.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267





More information about the cfe-commits mailing list