[clang-tools-extra] afa9430 - [clangd] Add code action to generate a constructor for a C++ class

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 6 07:24:25 PDT 2022


Author: Sam McCall
Date: 2022-04-06T16:23:50+02:00
New Revision: afa94306a8c197e346d3234e5ac5292ab90eae73

URL: https://github.com/llvm/llvm-project/commit/afa94306a8c197e346d3234e5ac5292ab90eae73
DIFF: https://github.com/llvm/llvm-project/commit/afa94306a8c197e346d3234e5ac5292ab90eae73.diff

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

Differential Revision: https://reviews.llvm.org/D116514

Added: 
    clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
    clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp

Modified: 
    clang-tools-extra/clangd/AST.cpp
    clang-tools-extra/clangd/AST.h
    clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
    clang-tools-extra/clangd/unittests/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 660c7187c2268..70d98d0e0bb40 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -349,7 +349,8 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
   return SymbolID(USR);
 }
 
-std::string printType(const QualType QT, const DeclContext &CurContext) {
+std::string printType(const QualType QT, const DeclContext &CurContext,
+                      const llvm::StringRef Placeholder) {
   std::string Result;
   llvm::raw_string_ostream OS(Result);
   PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
@@ -370,7 +371,7 @@ std::string printType(const QualType QT, const DeclContext &CurContext) {
   PrintCB PCB(&CurContext);
   PP.Callbacks = &PCB;
 
-  QT.print(OS, PP);
+  QT.print(OS, PP, Placeholder);
   return OS.str();
 }
 

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 523e1d9a9a5f4..c903a34e728fa 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -95,7 +95,8 @@ SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like anonymous/inline namespace.
-std::string printType(const QualType QT, const DeclContext &CurContext);
+std::string printType(const QualType QT, const DeclContext &CurContext,
+                      llvm::StringRef Placeholder = "");
 
 /// Indicates if \p D is a template instantiation implicitly generated by the
 /// compiler, e.g.

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index 0b968ee2a66c1..a68e7157a5af0 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -21,6 +21,7 @@ add_clang_library(clangDaemonTweaks OBJECT
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
+  MemberwiseConstructor.cpp
   ObjCLocalizeStringLiteral.cpp
   ObjCMemberwiseInitializer.cpp
   PopulateSwitch.cpp

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp b/clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
new file mode 100644
index 0000000000000..a00b778e7dd79
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/MemberwiseConstructor.cpp
@@ -0,0 +1,267 @@
+//===--- MemberwiseConstructor.cpp - Generate C++ constructor -------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+#include "AST.h"
+#include "ParsedAST.h"
+#include "refactor/InsertionPoint.h"
+#include "refactor/Tweak.h"
+#include "support/Logger.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/TypeVisitor.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// A tweak that adds a C++ constructor which initializes each member.
+//
+// Given:
+//   struct S{ int x; unique_ptr<double> y; };
+// the tweak inserts the constructor:
+//   S(int x, unique_ptr<double> y) : x(x), y(std::move(y)) {}
+//
+// We place the constructor inline, other tweaks are available to outline it.
+class MemberwiseConstructor : public Tweak {
+public:
+  const char *id() const override final;
+  llvm::StringLiteral kind() const override {
+    return CodeAction::REFACTOR_KIND;
+  }
+  std::string title() const override {
+    return llvm::formatv("Define constructor");
+  }
+
+  bool prepare(const Selection &Inputs) override {
+    // This tweak assumes move semantics.
+    if (!Inputs.AST->getLangOpts().CPlusPlus11)
+      return false;
+
+    // Trigger only on class definitions.
+    if (auto *N = Inputs.ASTSelection.commonAncestor())
+      Class = N->ASTNode.get<CXXRecordDecl>();
+    if (!Class || !Class->isThisDeclarationADefinition() || Class->isUnion() ||
+        Class->getDeclName().isEmpty())
+      return false;
+
+    dlog("MemberwiseConstructor for {0}?", Class->getName());
+    // For now, don't support nontrivial initialization of bases.
+    for (const CXXBaseSpecifier &Base : Class->bases()) {
+      const auto *BaseClass = Base.getType()->getAsCXXRecordDecl();
+      if (!BaseClass || !BaseClass->hasDefaultConstructor()) {
+        dlog("  can't construct base {0}", Base.getType().getAsString());
+        return false;
+      }
+    }
+
+    // We don't want to offer the tweak if there's a similar constructor.
+    // For now, only offer it if all constructors are special members.
+    for (const CXXConstructorDecl *CCD : Class->ctors()) {
+      if (!CCD->isDefaultConstructor() && !CCD->isCopyOrMoveConstructor()) {
+        dlog("  conflicting constructor");
+        return false;
+      }
+    }
+
+    // Examine the fields to see which ones we should initialize.
+    for (const FieldDecl *D : Class->fields()) {
+      switch (FieldAction A = considerField(D)) {
+      case Fail:
+        dlog("  
diff icult field {0}", D->getName());
+        return false;
+      case Skip:
+        dlog("  (skipping field {0})", D->getName());
+        break;
+      default:
+        Fields.push_back({D, A});
+        break;
+      }
+    }
+    // Only offer the tweak if we have some fields to initialize.
+    if (Fields.empty()) {
+      dlog("  no fields to initialize");
+      return false;
+    }
+
+    return true;
+  }
+
+  Expected<Effect> apply(const Selection &Inputs) override {
+    std::string Code = buildCode();
+    // Prefer to place the new constructor...
+    std::vector<Anchor> Anchors = {
+        // Below special constructors.
+        {[](const Decl *D) {
+           if (const auto *CCD = llvm::dyn_cast<CXXConstructorDecl>(D))
+             return CCD->isDefaultConstructor();
+           return false;
+         },
+         Anchor::Below},
+        // Above other constructors
+        {[](const Decl *D) { return llvm::isa<CXXConstructorDecl>(D); },
+         Anchor::Above},
+        // At the top of the public section
+        {[](const Decl *D) { return true; }, Anchor::Above},
+    };
+    auto Edit = insertDecl(Code, *Class, std::move(Anchors), AS_public);
+    if (!Edit)
+      return Edit.takeError();
+    return Effect::mainFileEdit(Inputs.AST->getSourceManager(),
+                                tooling::Replacements{std::move(*Edit)});
+  }
+
+private:
+  enum FieldAction {
+    Fail,    // Disallow the tweak, we can't handle this field.
+    Skip,    // Do not initialize this field, but allow the tweak anyway.
+    Move,    // Pass by value and std::move into place
+    Copy,    // Pass by value and copy into place
+    CopyRef, // Pass by const ref and copy into place
+  };
+  FieldAction considerField(const FieldDecl *Field) const {
+    if (Field->hasInClassInitializer())
+      return Skip;
+    if (!Field->getIdentifier())
+      return Fail;
+
+    // Decide what to do based on the field type.
+    class Visitor : public TypeVisitor<Visitor, FieldAction> {
+    public:
+      Visitor(const ASTContext &Ctx) : Ctx(Ctx) {}
+      const ASTContext &Ctx;
+
+      // If we don't understand the type, assume we can't handle it.
+      FieldAction VisitType(const Type *T) { return Fail; }
+      FieldAction VisitRecordType(const RecordType *T) {
+        if (const auto *D = T->getAsCXXRecordDecl())
+          return considerClassValue(*D);
+        return Fail;
+      }
+      FieldAction VisitBuiltinType(const BuiltinType *T) {
+        if (T->isInteger() || T->isFloatingPoint() || T->isNullPtrType())
+          return Copy;
+        return Fail;
+      }
+      FieldAction VisitObjCObjectPointerType(const ObjCObjectPointerType *) {
+        return Ctx.getLangOpts().ObjCAutoRefCount ? Copy : Fail;
+      }
+      FieldAction VisitAttributedType(const AttributedType *T) {
+        return Visit(T->getModifiedType().getCanonicalType().getTypePtr());
+      }
+#define ALWAYS(T, Action)                                                      \
+  FieldAction Visit##T##Type(const T##Type *) { return Action; }
+      // Trivially copyable types (pointers and numbers).
+      ALWAYS(Pointer, Copy);
+      ALWAYS(MemberPointer, Copy);
+      ALWAYS(Reference, Copy);
+      ALWAYS(Complex, Copy);
+      ALWAYS(Enum, Copy);
+      // These types are dependent (when canonical) and likely to be classes.
+      // Move is a reasonable generic option.
+      ALWAYS(DependentName, Move);
+      ALWAYS(UnresolvedUsing, Move);
+      ALWAYS(TemplateTypeParm, Move);
+      ALWAYS(TemplateSpecialization, Move);
+    };
+#undef ALWAYS
+    return Visitor(Class->getASTContext())
+        .Visit(Field->getType().getCanonicalType().getTypePtr());
+  }
+
+  // Decide what to do with a field of type C.
+  static FieldAction considerClassValue(const CXXRecordDecl &C) {
+    // We can't always tell if C is copyable/movable without doing Sema work.
+    // We assume operations are possible unless we can prove not.
+    bool CanCopy = C.hasUserDeclaredCopyConstructor() ||
+                   C.needsOverloadResolutionForCopyConstructor() ||
+                   !C.defaultedCopyConstructorIsDeleted();
+    bool CanMove = C.hasUserDeclaredMoveConstructor() ||
+                   (C.needsOverloadResolutionForMoveConstructor() ||
+                    !C.defaultedMoveConstructorIsDeleted());
+    bool CanDefaultConstruct = C.hasDefaultConstructor();
+    if (C.hasUserDeclaredCopyConstructor() ||
+        C.hasUserDeclaredMoveConstructor()) {
+      for (const CXXConstructorDecl *CCD : C.ctors()) {
+        bool IsUsable = !CCD->isDeleted() && CCD->getAccess() == AS_public;
+        if (CCD->isCopyConstructor())
+          CanCopy = CanCopy && IsUsable;
+        if (CCD->isMoveConstructor())
+          CanMove = CanMove && IsUsable;
+        if (CCD->isDefaultConstructor())
+          CanDefaultConstruct = IsUsable;
+      }
+    }
+    dlog("  {0} CanCopy={1} CanMove={2} TriviallyCopyable={3}", C.getName(),
+         CanCopy, CanMove, C.isTriviallyCopyable());
+    if (CanCopy && C.isTriviallyCopyable())
+      return Copy;
+    if (CanMove)
+      return Move;
+    if (CanCopy)
+      return CopyRef;
+    // If it's neither copyable nor movable, then default construction is
+    // likely to make sense (example: std::mutex).
+    if (CanDefaultConstruct)
+      return Skip;
+    return Fail;
+  }
+
+  std::string buildCode() const {
+    std::string S;
+    llvm::raw_string_ostream OS(S);
+
+    if (Fields.size() == 1)
+      OS << "explicit ";
+    OS << Class->getName() << "(";
+    const char *Sep = "";
+    for (const FieldInfo &Info : Fields) {
+      OS << Sep;
+      QualType ParamType = Info.Field->getType().getLocalUnqualifiedType();
+      if (Info.Action == CopyRef)
+        ParamType = Class->getASTContext().getLValueReferenceType(
+            ParamType.withConst());
+      OS << printType(ParamType, *Class,
+                      /*Placeholder=*/paramName(Info.Field));
+      Sep = ", ";
+    }
+    OS << ")";
+    Sep = " : ";
+    for (const FieldInfo &Info : Fields) {
+      OS << Sep << Info.Field->getName() << "(";
+      if (Info.Action == Move)
+        OS << "std::move("; // FIXME: #include <utility> too
+      OS << paramName(Info.Field);
+      if (Info.Action == Move)
+        OS << ")";
+      OS << ")";
+      Sep = ", ";
+    }
+    OS << " {}\n";
+
+    return S;
+  }
+
+  llvm::StringRef paramName(const FieldDecl *Field) const {
+    return Field->getName().trim("_");
+  }
+
+  const CXXRecordDecl *Class = nullptr;
+  struct FieldInfo {
+    const FieldDecl *Field;
+    FieldAction Action;
+  };
+  std::vector<FieldInfo> Fields;
+};
+REGISTER_TWEAK(MemberwiseConstructor)
+
+} // namespace
+} // namespace clangd
+} // namespace clang

diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index 8d8efb9ecc3c7..8309be64ef238 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -117,6 +117,7 @@ add_unittest(ClangdUnitTests ClangdTests
   tweaks/ExpandMacroTests.cpp
   tweaks/ExtractFunctionTests.cpp
   tweaks/ExtractVariableTests.cpp
+  tweaks/MemberwiseConstructorTests.cpp
   tweaks/ObjCLocalizeStringLiteralTests.cpp
   tweaks/ObjCMemberwiseInitializerTests.cpp
   tweaks/PopulateSwitchTests.cpp

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
new file mode 100644
index 0000000000000..cc2750d557c8d
--- /dev/null
+++ b/clang-tools-extra/clangd/unittests/tweaks/MemberwiseConstructorTests.cpp
@@ -0,0 +1,99 @@
+//===-- MemberwiseConstructorTests.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TweakTesting.h"
+#include "gmock/gmock-matchers.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using testing::AllOf;
+using testing::Eq;
+using testing::HasSubstr;
+using testing::Not;
+
+TWEAK_TEST(MemberwiseConstructor);
+
+TEST_F(MemberwiseConstructorTest, Availability) {
+  EXPECT_AVAILABLE("^struct ^S ^{ int x, y; };");
+  EXPECT_UNAVAILABLE("struct S { ^int ^x, y; }; struct ^S;");
+  EXPECT_UNAVAILABLE("struct ^S {};");
+  EXPECT_UNAVAILABLE("union ^S { int x; };");
+  EXPECT_UNAVAILABLE("struct ^S { int x = 0; };");
+  EXPECT_UNAVAILABLE("struct ^S { struct { int x; }; };");
+  EXPECT_UNAVAILABLE("struct ^{ int x; } e;");
+}
+
+TEST_F(MemberwiseConstructorTest, Edits) {
+  Header = R"cpp(
+    struct Move {
+      Move(Move&&) = default;
+      Move(const Move&) = delete;
+    };
+    struct Copy {
+      Copy(Copy&&) = delete;
+      Copy(const Copy&);
+    };
+  )cpp";
+  EXPECT_EQ(apply("struct ^S{Move M; Copy C; int I; int J=4;};"),
+            "struct S{"
+            "S(Move M, const Copy &C, int I) : M(std::move(M)), C(C), I(I) {}\n"
+            "Move M; Copy C; int I; int J=4;};");
+}
+
+TEST_F(MemberwiseConstructorTest, FieldTreatment) {
+  Header = R"cpp(
+    struct MoveOnly {
+      MoveOnly(MoveOnly&&) = default;
+      MoveOnly(const MoveOnly&) = delete;
+    };
+    struct CopyOnly {
+      CopyOnly(CopyOnly&&) = delete;
+      CopyOnly(const CopyOnly&);
+    };
+    struct CopyTrivial {
+      CopyTrivial(CopyTrivial&&) = default;
+      CopyTrivial(const CopyTrivial&) = default;
+    };
+    struct Immovable {
+      Immovable(Immovable&&) = delete;
+      Immovable(const Immovable&) = delete;
+    };
+    template <typename T>
+    struct Traits { using Type = typename T::Type; };
+    using IntAlias = int;
+  )cpp";
+
+  auto Fail = Eq("unavailable");
+  auto Move = HasSubstr(": Member(std::move(Member))");
+  auto CopyRef = AllOf(HasSubstr("S(const "), HasSubstr(": Member(Member)"));
+  auto Copy = AllOf(Not(HasSubstr("S(const ")), HasSubstr(": Member(Member)"));
+  auto With = [](llvm::StringRef Type) {
+    return ("struct ^S { " + Type + " Member; };").str();
+  };
+
+  EXPECT_THAT(apply(With("Immovable")), Fail);
+  EXPECT_THAT(apply(With("MoveOnly")), Move);
+  EXPECT_THAT(apply(With("CopyOnly")), CopyRef);
+  EXPECT_THAT(apply(With("CopyTrivial")), Copy);
+  EXPECT_THAT(apply(With("int")), Copy);
+  EXPECT_THAT(apply(With("IntAlias")), Copy);
+  EXPECT_THAT(apply(With("Immovable*")), Copy);
+  EXPECT_THAT(apply(With("Immovable&")), Copy);
+
+  EXPECT_THAT(apply("template <typename T>" + With("T")), Move);
+  EXPECT_THAT(apply("template <typename T>" + With("typename Traits<T>::Type")),
+              Move);
+  EXPECT_THAT(apply("template <typename T>" + With("T*")), Copy);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang


        


More information about the cfe-commits mailing list