diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h --- a/clang-tools-extra/clangd/AST.h +++ b/clang-tools-extra/clangd/AST.h @@ -95,7 +95,8 @@ /// 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/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -349,7 +349,8 @@ 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 @@ 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/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -21,6 +21,7 @@ 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 --- /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 y; }; +// the tweak inserts the constructor: +// S(int x, unique_ptr 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(); + 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(" difficult 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 apply(const Selection &Inputs) override { + std::string Code = buildCode(); + // Prefer to place the new constructor... + std::vector Anchors = { + // Below special constructors. + {[](const Decl *D) { + if (const auto *CCD = llvm::dyn_cast(D)) + return CCD->isDefaultConstructor(); + return false; + }, + Anchor::Below}, + // Above other constructors + {[](const Decl *D) { return llvm::isa(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 { + 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 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 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 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -117,6 +117,7 @@ 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 --- /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 + 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 " + With("T")), Move); + EXPECT_THAT(apply("template " + With("typename Traits::Type")), + Move); + EXPECT_THAT(apply("template " + With("T*")), Copy); +} + +} // namespace +} // namespace clangd +} // namespace clang