[clangd] Code action to declare missing move/copy constructor/assignment

[sammccall]

Summary

Fixes https://github.com/clangd/clangd/issues/973

[sammccall]

Oops, forgot the implementation

[sammccall]

oops, and the test

[sammccall]

Rebase on D116502

[hokein]

clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp

72

nit: add trigger points.

120

I think we need this because these members are created lazily in clang, e.g. if the empty struct s is not used, there is no constructor decl being created.

The ForceDeclarationOfImplicitMembers is a member function which can mutate the parameter Class, I was wondering whether it would lead some bad side-effect, but I didn't come out one (and the mutation is mostly creating a new ctor-decl and adding it to the Class).

146

nit: add default value

[sammccall]

Oops, this never got landed!

Address comments

[sammccall]

clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp

120

That's right. Rewrote the comment to make it clearer that these are lazily created (which is why I think it's safe to create them)

clang-tools-extra/clangd/ParsedAST.h

 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace clang {
+class Sema;
 namespace clangd {
 class HeuristicResolver;
 class SymbolIndex;
 
 /// Stores and provides access to parsed AST.
 class ParsedAST {
 public:
   /// Attempts to run Clang and store the parsed AST.
public:
   ~ParsedAST();
 
   /// Note that the returned ast will not contain decls from the preamble that
   /// were not deserialized during parsing. Clients should expect only decls
   /// from the main file to be in the AST.
   ASTContext &getASTContext();
   const ASTContext &getASTContext() const;
 
+  Sema &getSema();
+
   Preprocessor &getPreprocessor();
   std::shared_ptr<Preprocessor> getPreprocessorPtr();
   const Preprocessor &getPreprocessor() const;
 
   SourceManager &getSourceManager() {
     return getASTContext().getSourceManager();
   }
   const SourceManager &getSourceManager() const {

clang-tools-extra/clangd/ParsedAST.cpp

 }
 
 ASTContext &ParsedAST::getASTContext() { return Clang->getASTContext(); }
 
 const ASTContext &ParsedAST::getASTContext() const {
   return Clang->getASTContext();
 }
 
+Sema &ParsedAST::getSema() { return Clang->getSema(); }
+
 Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); }
 
 std::shared_ptr<Preprocessor> ParsedAST::getPreprocessorPtr() {
   return Clang->getPreprocessorPtr();
 }
 
 const Preprocessor &ParsedAST::getPreprocessor() const {
   return Clang->getPreprocessor();

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

add_clang_library(clangDaemonTweaks OBJECT
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
   ObjCLocalizeStringLiteral.cpp
   PopulateSwitch.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
+  SpecialMembers.cpp
   SwapIfBranches.cpp
 
   LINK_LIBS
   clangAST
   clangBasic
   clangDaemon
   clangdSupport
   clangFormat
   clangLex
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
 
   DEPENDS
   omp_gen
   )

clang-tools-extra/clangd/refactor/tweaks/SpecialMembers.cpp

+//===--- 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<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;
+}
+
+// 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);
+
+    // Prefer to place the new members...
+    std::vector<Anchor> Anchors = {
+        // Below the default constructor
+        {[](const Decl *D) {
+           if (const auto *CCD = llvm::dyn_cast<CXXConstructorDecl>(D))
+             return CCD->isDefaultConstructor();
+           return false;
+         },
+         Anchor::Below},
+        // Above existing 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:
+  bool NeedCopy, NeedMove;
+  CXXRecordDecl *Class = nullptr;
+};
+REGISTER_TWEAK(DeclareCopyMove)
+
+} // namespace
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/unittests/tweaks/SpecialMembersTests.cpp

+//===-- 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