sammccall updated this revision to Diff 396950. sammccall added a comment. oops, and the test
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116490/new/ https://reviews.llvm.org/D116490 Files: clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/ParsedAST.h clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp
Index: clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp =================================================================== --- /dev/null +++ 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 Index: clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp @@ -0,0 +1,196 @@ +//===--- 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/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<CXXConstructorDecl>(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; +} + +// Chooses where in the class definition to insert constructors/operators. +// If there is any public constructor, we insert after the last one. +// Otherwise we insert at the top of the first specified public section. +// If no public section is specified in a struct, insert at the top. +// If no public section is specified in a class, set NeedsPublic. +SourceLocation chooseSpecialMemberInsertionPoint(const CXXRecordDecl &Class, + bool &NeedsPublic) { + SourceLocation InFirstPublic; + SourceLocation AfterLastPublicCtor; + SourceLocation FirstDecl; + SourceLocation RBrace = Class.getBraceRange().getEnd(); + + // We want a slightly odd loop here, pairing each declaration with + // the location *after* it. + auto SawDecl = [&](const Decl *D, SourceLocation NextLoc) { + if (FirstDecl.isInvalid()) + FirstDecl = D->getSourceRange().getBegin(); + if (InFirstPublic.isInvalid()) { + if (auto *ASD = llvm::dyn_cast<AccessSpecDecl>(D)) { + if (ASD->getAccess() == AS_public) + InFirstPublic = NextLoc; + } + } + if (llvm::isa<CXXConstructorDecl>(D) && D->getAccess() == AS_public) + AfterLastPublicCtor = NextLoc; + }; + + const Decl *Prev = nullptr; + for (const auto *D : Class.decls()) { + if (D->isImplicit()) + continue; + if (Prev) + SawDecl(Prev, D->getSourceRange().getBegin()); + Prev = D; + } + if (Prev) + SawDecl(Prev, RBrace); + + // We've gathered all the locations, pick the best insertion point. + if (AfterLastPublicCtor.isValid()) + return AfterLastPublicCtor; + if (InFirstPublic.isValid()) + return InFirstPublic; + if (Class.getTagKind() == TTK_Class) { + NeedsPublic = true; + // Insert at the end, to avoid inserting a second access specifier. + return RBrace; + } + return FirstDecl.isValid() ? FirstDecl : RBrace; +} + +// 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<CXXRecordDecl *>(N->ASTNode.get<CXXRecordDecl>()); + 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<Effect> apply(const Selection &Inputs) override { + // We need to ensure implicit special members exist so we can tell whether + // they should be =default or =delete. + Inputs.AST->getSema().ForceDeclarationOfImplicitMembers(Class); + std::string Code = buildSpecialMemberDeclarations(*Class); + + bool NeedsPublic = false; + SourceLocation InsertBefore = + chooseSpecialMemberInsertionPoint(*Class, NeedsPublic); + if (NeedsPublic) + Code = "public:\n" + Code; + + const auto &SM = Inputs.AST->getSourceManager(); + if (!SM.isInMainFile(InsertBefore)) + return error("Class body not in the main file! {0}", + InsertBefore.printToString(SM)); + return Effect::mainFileEdit( + Inputs.AST->getSourceManager(), + tooling::Replacements(tooling::Replacement(SM, InsertBefore, 0, Code))); + } + +private: + bool NeedCopy, NeedMove; + CXXRecordDecl *Class = nullptr; +}; +REGISTER_TWEAK(DeclareCopyMove) + +} // namespace +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt +++ clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt @@ -25,6 +25,7 @@ PopulateSwitch.cpp RawStringLiteral.cpp RemoveUsingNamespace.cpp + SpecialMembers.cpp SwapIfBranches.cpp LINK_LIBS Index: clang-tools-extra/clangd/ParsedAST.h =================================================================== --- clang-tools-extra/clangd/ParsedAST.h +++ clang-tools-extra/clangd/ParsedAST.h @@ -41,6 +41,7 @@ #include <vector> namespace clang { +class Sema; namespace clangd { class HeuristicResolver; class SymbolIndex; @@ -68,6 +69,8 @@ ASTContext &getASTContext(); const ASTContext &getASTContext() const; + Sema &getSema(); + Preprocessor &getPreprocessor(); std::shared_ptr<Preprocessor> getPreprocessorPtr(); const Preprocessor &getPreprocessor() const; Index: clang-tools-extra/clangd/ParsedAST.cpp =================================================================== --- clang-tools-extra/clangd/ParsedAST.cpp +++ clang-tools-extra/clangd/ParsedAST.cpp @@ -558,6 +558,8 @@ return Clang->getASTContext(); } +Sema &ParsedAST::getSema() { return Clang->getSema(); } + Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); } std::shared_ptr<Preprocessor> ParsedAST::getPreprocessorPtr() {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
