[PATCH] D122102: [clangd] Introduce "add subclass" tweak

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

Thanks, this looks really good! I always wanted a code action like this.

I always envisioned this as a code action on a class to implement missing virtual methods though. The idea would be:

  class Foo : public B^ar { };

expands into:
class Foo : public Bar {

  int baz() overrides { return Bar::baz(); }

};

This has several advantages:

- no need to rename (the user already provided the name)
- no need to move the code (it adds the code in appropriate place)
- in situations like this change, where you implement a subclass of Tweak, there's no need to navigate to the Tweak.h header file (which could be outside of your project, maybe even read-only, or could just #include[_next] something else) and you don't need to wait for the AST build for the header file when you're just going to move it to another file anyway.

Have you considered an approach like this? Obviously it's a bit more complicated, but I think it would make this tweak even better. The way it's triggered now has it's use cases too, of course. I wonder if we could share the code between two use cases like this.

Ideally I'd like to see this code action show up as code completion option as well. Something like:

  class Foo : public Bar {^

could result in "add virtual methods" as #1 option, greatly increasing discoverability, but that's a whole different problem and not specific to this tweak (PopulateSwitch would be great candidate for this too).



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddSubclass.cpp:1
+//===--- ExpandAutoType.cpp --------------------------------------*- C++-*-===//
+//
----------------
Wrong file name in the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122102