[PATCH] D116514: [clangd] Add code action to generate a constructor for a C++ class

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 18 06:35:12 PST 2022


njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:26-28
+//   struct S {
+//     S(int x, unique_ptr<double> y) : x(x), y(std::move(y)) {}
+//   };
----------------
avogelsgesang wrote:
> ```
> // e.g. given `struct S{ int x; unique_ptr<double> y; };`, produces:
> //   struct S {
> //     int x; unique_ptr<double> y;
> //     S(int x, unique_ptr<double> y) : x(x), y(std::move(y)) {}
> //   };
> ```
> or just
> ```
> // e.g. given `struct S{ int x; unique_ptr<double> y; };`, produces:
> //     S(int x, unique_ptr<double> y) : x(x), y(std::move(y)) {}
> ```
> 
> The tweak does not remove the members, as currently suggested by the comment
That's just a bad comment, the tweak won't remove the members


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+      Class = N->ASTNode.get<CXXRecordDecl>();
+    if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+      return false;
----------------
avogelsgesang wrote:
> do we also need to exclude anonymous class declarations here? (Not sure if those are also modelled as `CXXRecordDecl` in the clang AST...)
Good point, should also ensure there is a test case for this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514



More information about the cfe-commits mailing list