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

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang 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)) {}
+//   };
----------------
```
// 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


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:49
+      Class = N->ASTNode.get<CXXRecordDecl>();
+    if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion())
+      return false;
----------------
do we also need to exclude anonymous class declarations here? (Not sure if those are also modelled as `CXXRecordDecl` in the clang AST...)


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp:133
+    // Decide what to do based on the field type.
+    class Visitor : public TypeVisitor<Visitor, FieldAction> {
+    public:
----------------
Is this visitor able to look through `using MyType = int;` and `typedef`?
I think we should also add a test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116514