From 68eac9a6e7a10eba8081bab340fda8be13a7840e Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Sun, 2 Jan 2022 05:01:43 +0100 Subject: [PATCH] [clangd] Code action to declare missing move/copy constructor/assignment Fixes https://github.com/clangd/clangd/issues/973 Differential Revision: https://reviews.llvm.org/D116490 --- clang-tools-extra/clangd/ParsedAST.cpp | 2 + clang-tools-extra/clangd/ParsedAST.h | 3 + .../clangd/refactor/tweaks/CMakeLists.txt | 1 + .../clangd/refactor/tweaks/SpecialMembers.cpp | 153 +++++++++++++++++++++ .../unittests/tweaks/SpecialMembersTests.cpp | 47 +++++++ 5 files changed, 206 insertions(+) create mode 100644 clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp create mode 100644 clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 295418c..77cc456 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -629,6 +629,8 @@ const ASTContext &ParsedAST::getASTContext() const { return Clang->getASTContext(); } +Sema &ParsedAST::getSema() { return Clang->getSema(); } + Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); } std::shared_ptr ParsedAST::getPreprocessorPtr() { diff --git a/clang-tools-extra/clangd/ParsedAST.h b/clang-tools-extra/clangd/ParsedAST.h index 72ba2a9..425a411 100644 --- a/clang-tools-extra/clangd/ParsedAST.h +++ b/clang-tools-extra/clangd/ParsedAST.h @@ -37,6 +37,7 @@ #include namespace clang { +class Sema; namespace clangd { class HeuristicResolver; @@ -63,6 +64,8 @@ public: ASTContext &getASTContext(); const ASTContext &getASTContext() const; + Sema &getSema(); + Preprocessor &getPreprocessor(); std::shared_ptr getPreprocessorPtr(); const Preprocessor &getPreprocessor() const; diff --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt index ae27978..0b968ee 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_library(clangDaemonTweaks OBJECT PopulateSwitch.cpp RawStringLiteral.cpp RemoveUsingNamespace.cpp + SpecialMembers.cpp SwapIfBranches.cpp LINK_LIBS diff --git a/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp b/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp new file mode 100644 index 0000000..70604c0 --- /dev/null +++ b/clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp @@ -0,0 +1,153 @@ +//===--- SpecialMembers.cpp - Generate C++ special member functions -------===// +// +// 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 "ParsedAST.h" +#include "refactor/InsertionPoint.h" +#include "refactor/Tweak.h" +#include "support/Logger.h" +#include "clang/AST/DeclCXX.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Sema/Sema.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 { + +// Returns code to declare missing copy/move constructors/assignment operators. +// They will be deleted or defaulted to match the class's current state. +std::string buildSpecialMemberDeclarations(const CXXRecordDecl &Class) { + struct Members { + const CXXMethodDecl *Copy = nullptr; + const CXXMethodDecl *Move = nullptr; + } Ctor, Assign; + + for (const auto &M : Class.methods()) { + if (M->isCopyAssignmentOperator()) + Assign.Copy = M; + else if (M->isMoveAssignmentOperator()) + Assign.Move = M; + if (const auto *C = llvm::dyn_cast(M)) { + if (C->isCopyConstructor()) + Ctor.Copy = C; + else if (C->isMoveConstructor()) + Ctor.Move = C; + } + } + + std::string S; + llvm::raw_string_ostream OS(S); + + auto PrintMember = [&](const CXXMethodDecl *D, const char *MemberPattern, + const char *ParmPattern) { + if (D && !D->isImplicit()) + return; + bool Delete = !D || D->isDeleted(); + OS << llvm::formatv( + "{0} = {1};\n", + llvm::formatv(MemberPattern, Class.getName(), + llvm::formatv(ParmPattern, Class.getName())), + Delete ? "delete" : "default"); + }; + auto PrintMembers = [&](const Members &M, const char *MemberPattern) { + PrintMember(M.Copy, MemberPattern, /*ParmPattern=*/"const {0}&"); + PrintMember(M.Move, MemberPattern, /*ParmPattern=*/"{0}&&"); + }; + PrintMembers(Ctor, /*MemberPattern=*/"{0}({1})"); + PrintMembers(Assign, /*MemberPattern=*/"{0} &operator=({1})"); + + return S; +} + +// A tweak that adds missing declarations of copy & move constructors. +// +// e.g. given `struct ^S{};`, produces: +// struct S { +// S(const S&) = default; +// S(S&&) = default; +// S &operator=(const S&) = default; +// S &operator=(S&&) = default; +// }; +// +// Added members are defaulted or deleted to approximately preserve semantics. +// (May not be a strict no-op when they were not implicitly declared). +// +// Having these spelled out is useful: +// - to understand the implicit behavior +// - to avoid relying on the implicit behavior +// - as a baseline for explicit modification +class DeclareCopyMove : 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("declare implicit {0} members", + NeedCopy ? NeedMove ? "copy/move" : "copy" : "move"); + } + + bool prepare(const Selection &Inputs) override { + // This tweak relies on =default and =delete. + if (!Inputs.AST->getLangOpts().CPlusPlus11) + return false; + + // Trigger only on class definitions. + if (auto *N = Inputs.ASTSelection.commonAncestor()) + Class = const_cast(N->ASTNode.get()); + if (!Class || !Class->isThisDeclarationADefinition()) + return false; + + // Tweak is only available if some members are missing. + NeedCopy = !Class->hasUserDeclaredCopyConstructor() || + !Class->hasUserDeclaredCopyAssignment(); + NeedMove = !Class->hasUserDeclaredMoveAssignment() || + !Class->hasUserDeclaredMoveConstructor(); + return NeedCopy || NeedMove; + } + + Expected apply(const Selection &Inputs) override { + // Implicit special members are created lazily by clang. + // We need them so we can tell whether they should be =default or =delete. + Inputs.AST->getSema().ForceDeclarationOfImplicitMembers(Class); + std::string Code = buildSpecialMemberDeclarations(*Class); + + // Prefer to place the new members... + std::vector Anchors = { + // Below the default constructor + {[](const Decl *D) { + if (const auto *CCD = llvm::dyn_cast(D)) + return CCD->isDefaultConstructor(); + return false; + }, + Anchor::Below}, + // Above existing 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: + bool NeedCopy = false, NeedMove = false; + CXXRecordDecl *Class = nullptr; +}; +REGISTER_TWEAK(DeclareCopyMove) + +} // namespace +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp new file mode 100644 index 0000000..7fcaf41 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp @@ -0,0 +1,47 @@ +//===-- SpecialMembersTests.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 { + +TWEAK_TEST(SpecialMembers); + +TEST_F(SpecialMembersTest, Test) { + EXPECT_AVAILABLE("struct ^S {};"); + EXPECT_UNAVAILABLE("struct S { ^ };"); + EXPECT_UNAVAILABLE("union ^U {};"); + EXPECT_AVAILABLE("struct ^S { S(const S&); S(S&&); };"); + EXPECT_UNAVAILABLE("struct ^S {" + "S(const S&); S(S&&);" + "S &operator=(S&&); S &operator=(const S&);" + "};"); + + const char *Output = R"cpp(struct S{S(const S &) = default; + S(S &&) = default; + S &operator=(const S &) = default; + S &operator=(S &&) = default; +};)cpp"; + EXPECT_EQ(apply("struct ^S{};"), Output); + + Output = R"cpp(struct S{S(const S &) = default; +S(S &&) = default; +S &operator=(const S &) = delete; +S &operator=(S &&) = delete; +int& ref;};)cpp"; + EXPECT_EQ(apply("struct ^S{int& ref;};"), Output); +} + +} // namespace +} // namespace clangd +} // namespace clang -- 2.7.4