[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