[clangd] Helper for determining member insertion point.

[sammccall]

Summary

To be used in D116490 and D116385, and an upcoming patch to generate C++
constructors.

[sammccall]

Fix stale comment

[sammccall]

[clangd] Add code action to generate a constructor for a C++ class.

[sammccall]

Oops, revert wrong patch

[kadircet]

We have some logic in AddUsing tweak to determine insertion point based on AST. i think it makes sense to migrate it to these helpers too. There's some more logic in extract variable/function too. Extract variable seems too elaborate as it actually looks at statements, rather than decls and extract function is quite simple already as we already figure out current function's range and whatnot, but having another usage might help.

(thinking out loud)
We've got some helpers in SourceCode.h to determine insertion point in the absence of ASTs. Concepts here and there around an "insertion point" seems to be quite different (it's just a sourcelocation here and a set of locations + a namespace in SourceCode.h).
I suppose those two are somewhat hard to merge and serve different purposes, so It's better to keep them separate.

[kadircet]

clang-tools-extra/clangd/refactor/InsertionPoint.cpp

52

nit: use a switch?

135

what if we had:

class Foo {
public:
  void foo();
};

and wanted to insert a private member/field?
I suppose we should check for the specifier of last decl in InClass instead.

[sammccall]

Fix access protection bug, add more tests

[sammccall]

In D116502#3217084, @kadircet wrote:

We have some logic in AddUsing tweak to determine insertion point based on AST. i think it makes sense to migrate it to these helpers too.

Thanks, I'd forgotten about those. I might tackle those next?

There's some more logic in extract variable/function too. Extract variable seems too elaborate as it actually looks at statements, rather than decls and extract function is quite simple already as we already figure out current function's range and whatnot, but having another usage might help.

Yeah I wasn't sure I could do a good job of generalizing these ones yet (and we haven't needed it yet).

We've got some helpers in SourceCode.h to determine insertion point in the absence of ASTs. Concepts here and there around an "insertion point" seems to be quite different (it's just a sourcelocation here and a set of locations + a namespace in SourceCode.h).
I suppose those two are somewhat hard to merge and serve different purposes, so It's better to keep them separate.

For sure ast-based and pseudoparsing-based cases are going to be different APIs, but I would like to move those into this header too.
The problem I hit was they share private infrastructure (in SourceCode.cpp) with other functionality (the namespace pseudoparsing for completion, I think). So it's a bit of work to extract.

clang-tools-extra/clangd/refactor/InsertionPoint.cpp

135

Oops, good catch.

In that particular case, you could make a case for inserting at the very top of the class (no access specifier needed). But some coding styles want the opposite.
I think using the presence/absence of decls in the chunk before the first access specifier is a reasonable hint, and it happens to be the easiest thing to implement :-)

[kadircet]

Thanks, LGTM!

In D116502#3217270, @sammccall wrote:

In D116502#3217084, @kadircet wrote:

We have some logic in AddUsing tweak to determine insertion point based on AST. i think it makes sense to migrate it to these helpers too.

Thanks, I'd forgotten about those. I might tackle those next?

SGTM.

We've got some helpers in SourceCode.h to determine insertion point in the absence of ASTs. Concepts here and there around an "insertion point" seems to be quite different (it's just a sourcelocation here and a set of locations + a namespace in SourceCode.h).
I suppose those two are somewhat hard to merge and serve different purposes, so It's better to keep them separate.

For sure ast-based and pseudoparsing-based cases are going to be different APIs, but I would like to move those into this header too.

Makes sense.

The problem I hit was they share private infrastructure (in SourceCode.cpp) with other functionality (the namespace pseudoparsing for completion, I think). So it's a bit of work to extract.

Ah I see :/

clang-tools-extra/clangd/AST.h

 /// macro (e.g. a change in definition offset can result in a different USR). We
 /// could change these semantics in the future by reimplementing this funcure
 /// (e.g. avoid USR for macros).
 SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
                      const SourceManager &SM);
 
 /// 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.
 ///     template <class T> struct vector {};
 ///     vector<int> v; // 'vector<int>' is an implicit instantiation
 bool isImplicitTemplateInstantiation(const NamedDecl *D);
 /// Indicates if \p D is an explicit template specialization, e.g.
 ///   template <class T> struct vector {};

clang-tools-extra/clangd/AST.cpp

SymbolID getSymbolID(const llvm::StringRef MacroName, const MacroInfo *MI,
   if (MI == nullptr)
     return {};
   llvm::SmallString<128> USR;
   if (index::generateUSRForMacro(MacroName, MI->getDefinitionLoc(), SM, USR))
     return {};
   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());
   PP.SuppressTagKeyword = true;
   PP.SuppressUnwrittenScope = true;
 
   class PrintCB : public PrintingCallbacks {
   public:
     PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
     virtual ~PrintCB() {}
     virtual bool isScopeVisible(const DeclContext *DC) const override {
       return DC->Encloses(CurContext);
     }
 
   private:
     const DeclContext *CurContext;
   };
   PrintCB PCB(&CurContext);
   PP.Callbacks = &PCB;
 
-  QT.print(OS, PP);
+  QT.print(OS, PP, Placeholder);
   return OS.str();
 }
 
 QualType declaredType(const TypeDecl *D) {
   if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
     if (const auto *TSI = CTSD->getTypeAsWritten())
       return TSI->getType();
   return D->getASTContext().getTypeDeclType(D);

clang-tools-extra/clangd/CMakeLists.txt

add_clang_library(clangDaemon
   index/SymbolOrigin.cpp
   index/YAMLSerialization.cpp
 
   index/dex/Dex.cpp
   index/dex/Iterator.cpp
   index/dex/PostingList.cpp
   index/dex/Trigram.cpp
 
+  refactor/InsertionPoint.cpp
   refactor/Rename.cpp
   refactor/Tweak.cpp
 
   DEPENDS
   omp_gen
   )
 
 # Include generated CompletionModel headers.

clang-tools-extra/clangd/refactor/InsertionPoint.h

+//===--- InsertionPoint.h - Where should we add new code? --------*- C++-*-===//
+//
+// 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 "clang/AST/DeclCXX.h"
+#include "clang/Basic/Specifiers.h"
+#include "clang/Tooling/Core/Replacement.h"
+
+namespace clang {
+namespace clangd {
+
+// An anchor describes where to insert code into a decl sequence.
+//
+// It allows inserting above or below a block of decls matching some criterion.
+// For example, "insert after existing constructors".
+struct Anchor {
+  // A predicate describing which decls are considered part of a block.
+  // Match need not handle TemplateDecls, which are unwrapped before matching.
+  std::function<bool(const Decl *)> Match;
+  // Whether the insertion point should be before or after the matching block.
+  enum Dir { Above, Below } Direction = Below;
+};
+
+// Returns the point to insert a declaration according to Anchors.
+// Anchors are tried in order. For each, the first matching location is chosen.
+SourceLocation insertionPoint(const DeclContext &Ctx,
+                              llvm::ArrayRef<Anchor> Anchors);
+
+// Returns an edit inserting Code inside Ctx.
+// Location is chosen according to Anchors, falling back to the end of Ctx.
+// Fails if the chosen insertion point is in a different file than Ctx itself.
+llvm::Expected<tooling::Replacement> insertDecl(llvm::StringRef Code,
+                                                const DeclContext &Ctx,
+                                                llvm::ArrayRef<Anchor> Anchors);
+
+// Variant for C++ classes that ensures the right access control.
+SourceLocation insertionPoint(const CXXRecordDecl &InClass,
+                              std::vector<Anchor> Anchors,
+                              AccessSpecifier Protection);
+
+// Variant for C++ classes that ensures the right access control.
+// May insert a new access specifier if needed.
+llvm::Expected<tooling::Replacement> insertDecl(llvm::StringRef Code,
+                                                const CXXRecordDecl &InClass,
+                                                std::vector<Anchor> Anchors,
+                                                AccessSpecifier Protection);
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/refactor/InsertionPoint.cpp

+//===--- InsertionPoint.cpp - Where should we add new code? ---------------===//
+//
+// 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 "refactor/InsertionPoint.h"
+#include "support/Logger.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Choose the decl to insert before, according to an anchor.
+// Nullptr means insert at end of DC.
+// None means no valid place to insert.
+llvm::Optional<const Decl *> insertionDecl(const DeclContext &DC,
+                                           const Anchor &A) {
+  bool LastMatched = false;
+  bool ReturnNext = false;
+  for (const auto *D : DC.decls()) {
+    if (D->isImplicit())
+      continue;
+    if (ReturnNext)
+      return D;
+
+    const Decl *NonTemplate = D;
+    if (auto *TD = llvm::dyn_cast<TemplateDecl>(D))
+      NonTemplate = TD->getTemplatedDecl();
+    bool Matches = A.Match(NonTemplate);
+    dlog("    {0} {1} {2}", Matches, D->getDeclKindName(), D);
+
+    if (A.Direction == Anchor::Above) {
+      if (Matches && !LastMatched) {
+        // Special case: if "above" matches an access specifier, we actually
+        // want to insert below it!
+        if (llvm::isa<AccessSpecDecl>(D)) {
+          ReturnNext = true;
+          continue;
+        }
+        return D;
+      }
+    } else {
+      assert(A.Direction == Anchor::Below);
+      if (LastMatched && !Matches)
+        return D;
+    }
+
+    LastMatched = Matches;
+  }
+  if (ReturnNext || (LastMatched && A.Direction == Anchor::Below))
+    return nullptr;
+  return llvm::None;
+}
+
+SourceLocation beginLoc(const Decl &D) {
+  auto Loc = D.getBeginLoc();
+  if (RawComment *Comment = D.getASTContext().getRawCommentForDeclNoCache(&D)) {
+    auto CommentLoc = Comment->getBeginLoc();
+    if (CommentLoc.isValid() && Loc.isValid() &&
+        D.getASTContext().getSourceManager().isBeforeInTranslationUnit(
+            CommentLoc, Loc))
+      Loc = CommentLoc;
+  }
+  return Loc;
+}
+
+bool any(const Decl *D) { return true; }
+
+SourceLocation endLoc(const DeclContext &DC) {
+  const Decl *D = llvm::cast<Decl>(&DC);
+  if (auto *OCD = llvm::dyn_cast<ObjCContainerDecl>(D))
+    return OCD->getAtEndRange().getBegin();
+  return D->getEndLoc();
+}
+
+} // namespace
+
+SourceLocation insertionPoint(const DeclContext &DC,
+                              llvm::ArrayRef<Anchor> Anchors) {
+  dlog("Looking for insertion point in {0}", DC.getDeclKindName());
+  for (const auto &A : Anchors) {
+    dlog("  anchor ({0})", A.Direction == Anchor::Above ? "above" : "below");
+    if (auto D = insertionDecl(DC, A)) {
+      dlog("  anchor matched before {0}", *D);
+      return *D ? beginLoc(**D) : endLoc(DC);
+    }
+  }
+  dlog("no anchor matched");
+  return SourceLocation();
+}
+
+llvm::Expected<tooling::Replacement>
+insertDecl(llvm::StringRef Code, const DeclContext &DC,
+           llvm::ArrayRef<Anchor> Anchors) {
+  auto Loc = insertionPoint(DC, Anchors);
+  // Fallback: insert at the end.
+  if (Loc.isInvalid())
+    Loc = endLoc(DC);
+  const auto &SM = DC.getParentASTContext().getSourceManager();
+  if (!SM.isWrittenInSameFile(Loc, cast<Decl>(DC).getLocation()))
+    return error("{0} body in wrong file: {1}", DC.getDeclKindName(),
+                 Loc.printToString(SM));
+  return tooling::Replacement(SM, Loc, 0, Code);
+}
+
+SourceLocation insertionPoint(const CXXRecordDecl &InClass,
+                              std::vector<Anchor> Anchors,
+                              AccessSpecifier Protection) {
+  for (auto &A : Anchors)
+    A.Match = [Inner(std::move(A.Match)), Protection](const Decl *D) {
+      return D->getAccess() == Protection && Inner(D);
+    };
+  return insertionPoint(InClass, Anchors);
+}
+
+llvm::Expected<tooling::Replacement> insertDecl(llvm::StringRef Code,
+                                                const CXXRecordDecl &InClass,
+                                                std::vector<Anchor> Anchors,
+                                                AccessSpecifier Protection) {
+  // Fallback: insert at the bottom of the relevant access section.
+  Anchors.push_back({any, Anchor::Below});
+  auto Loc = insertionPoint(InClass, std::move(Anchors), Protection);
+  std::string CodeBuffer;
+  auto &SM = InClass.getASTContext().getSourceManager();
+  // Fallback: insert at the end of the class. Check if protection matches!
+  if (Loc.isInvalid()) {
+    Loc = InClass.getBraceRange().getEnd();
+    if (Protection !=
+        (InClass.getTagKind() == TTK_Class ? AS_private : AS_public)) {
+      CodeBuffer = (getAccessSpelling(Protection) + ":\n" + Code).str();
+      Code = CodeBuffer;
+    }
+  }
+  if (!SM.isWrittenInSameFile(Loc, InClass.getLocation()))
+    return error("Class body in wrong file: {0}", Loc.printToString(SM));
+  return tooling::Replacement(SM, Loc, 0, Code);
+}
+
+} // namespace clangd
+} // namespace clang

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

add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
   DefineInline.cpp
   DefineOutline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp
   ExtractVariable.cpp
+  MemberwiseConstructor.cpp
   ObjCLocalizeStringLiteral.cpp
   PopulateSwitch.cpp
   RawStringLiteral.cpp
   RemoveUsingNamespace.cpp
   SwapIfBranches.cpp
 
   LINK_LIBS
   clangAST

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

+//===--- 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 "ParsedAST.h"
+#include "refactor/InsertionPoint.h"
+#include "refactor/Tweak.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.
+//
+// e.g. given `struct S{ int x; unique_ptr<double> y; };`, produces:
+//   struct S {
+//     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())
+      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<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 : Skip;
+      }
+      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());
+    if (C.hasUserDeclaredCopyConstructor() ||
+        C.hasUserDeclaredMoveConstructor()) {
+      for (const CXXConstructorDecl *CCD : C.ctors()) {
+        if (CCD->isCopyConstructor())
+          CanCopy = CanCopy && !CCD->isDeleted();
+        if (CCD->isMoveConstructor())
+          CanMove = CanMove && !CCD->isDeleted();
+      }
+    }
+    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;
+    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(";
+      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

clang-tools-extra/clangd/unittests/CMakeLists.txt

add_unittest(ClangdUnitTests ClangdTests
   GlobalCompilationDatabaseTests.cpp
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
   IncludeCleanerTests.cpp
   IndexActionTests.cpp
   IndexTests.cpp
   InlayHintTests.cpp
+  InsertionPointTests.cpp
   JSONTransportTests.cpp
   LoggerTests.cpp
   LSPBinderTests.cpp
   LSPClient.cpp
   ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp
add_unittest(ClangdUnitTests ClangdTests
   tweaks/DefineOutlineTests.cpp
   tweaks/DumpASTTests.cpp
   tweaks/DumpRecordLayoutTests.cpp
   tweaks/DumpSymbolTests.cpp
   tweaks/ExpandAutoTypeTests.cpp
   tweaks/ExpandMacroTests.cpp
   tweaks/ExtractFunctionTests.cpp
   tweaks/ExtractVariableTests.cpp
+  tweaks/MemberwiseConstructorTests.cpp
   tweaks/ObjCLocalizeStringLiteralTests.cpp
   tweaks/PopulateSwitchTests.cpp
   tweaks/RawStringLiteralTests.cpp
   tweaks/RemoveUsingNamespaceTests.cpp
   tweaks/ShowSelectionTreeTests.cpp
   tweaks/SwapIfBranchesTests.cpp
   tweaks/TweakTesting.cpp
   tweaks/TweakTests.cpp

clang-tools-extra/clangd/unittests/InsertionPointTests.cpp

+//===-- InsertionPointTess.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 "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "refactor/InsertionPoint.h"
+#include "clang/AST/DeclBase.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(InsertionPointTests, Generic) {
+  Annotations Code(R"cpp(
+  namespace ns {
+    $a^int a1;
+    $b^// leading comment
+    int b;
+    $c^int c1; // trailing comment
+    int c2;
+    $a2^int a2;
+  $end^};
+  )cpp");
+
+  auto StartsWith =
+      [&](llvm::StringLiteral S) -> std::function<bool(const Decl *)> {
+    return [S](const Decl *D) {
+      if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+        return llvm::StringRef(ND->getNameAsString()).startswith(S);
+      return false;
+    };
+  };
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  auto &NS = cast<NamespaceDecl>(findDecl(AST, "ns"));
+
+  // Test single anchors.
+  auto Point = [&](llvm::StringLiteral Prefix, Anchor::Dir Direction) {
+    auto Loc = insertionPoint(NS, {Anchor{StartsWith(Prefix), Direction}});
+    return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point("a", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("a", Anchor::Below), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Above), Code.point("b"));
+  EXPECT_EQ(Point("b", Anchor::Below), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Above), Code.point("c"));
+  EXPECT_EQ(Point("c", Anchor::Below), Code.point("a2"));
+  EXPECT_EQ(Point("", Anchor::Above), Code.point("a"));
+  EXPECT_EQ(Point("", Anchor::Below), Code.point("end"));
+  EXPECT_EQ(Point("no_match", Anchor::Below), Position{});
+
+  // Test anchor chaining.
+  auto Chain = [&](llvm::StringLiteral P1, llvm::StringLiteral P2) {
+    auto Loc = insertionPoint(NS, {Anchor{StartsWith(P1), Anchor::Above},
+                                   Anchor{StartsWith(P2), Anchor::Above}});
+    return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Chain("a", "b"), Code.point("a"));
+  EXPECT_EQ(Chain("b", "a"), Code.point("b"));
+  EXPECT_EQ(Chain("no_match", "a"), Code.point("a"));
+
+  // Test edit generation.
+  auto Edit = insertDecl("foo;", NS, {Anchor{StartsWith("a"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()), Code.point("b"));
+  EXPECT_EQ(Edit->getReplacementText(), "foo;");
+  // If no match, the edit is inserted at the end.
+  Edit = insertDecl("x;", NS, {Anchor{StartsWith("no_match"), Anchor::Below}});
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+            Code.point("end"));
+}
+
+// For CXX, we should check:
+// - special handling for access specifiers
+// - unwrapping of template decls
+TEST(InsertionPointTests, CXX) {
+  Annotations Code(R"cpp(
+    class C {
+    public:
+      $Method^void pubMethod();
+      $Field^int PubField;
+
+    $private^private:
+      $field^int PrivField;
+      $method^void privMethod();
+      template <typename T> void privTemplateMethod();
+    $end^};
+  )cpp");
+
+  auto AST = TestTU::withCode(Code.code()).build();
+  const CXXRecordDecl &C = cast<CXXRecordDecl>(findDecl(AST, "C"));
+
+  auto IsMethod = [](const Decl *D) { return llvm::isa<CXXMethodDecl>(D); };
+  auto Any = [](const Decl *D) { return true; };
+
+  // Test single anchors.
+  auto Point = [&](Anchor A, AccessSpecifier Protection) {
+    auto Loc = insertionPoint(C, {A}, Protection);
+    return sourceLocToPosition(AST.getSourceManager(), Loc);
+  };
+  EXPECT_EQ(Point({IsMethod, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Below}, AS_public), Code.point("Field"));
+  EXPECT_EQ(Point({Any, Anchor::Above}, AS_public), Code.point("Method"));
+  EXPECT_EQ(Point({Any, Anchor::Below}, AS_public), Code.point("private"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Above}, AS_private), Code.point("method"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Below}, AS_private), Code.point("end"));
+  EXPECT_EQ(Point({Any, Anchor::Above}, AS_private), Code.point("field"));
+  EXPECT_EQ(Point({Any, Anchor::Below}, AS_private), Code.point("end"));
+  EXPECT_EQ(Point({IsMethod, Anchor::Above}, AS_protected), Position{});
+  EXPECT_EQ(Point({IsMethod, Anchor::Below}, AS_protected), Position{});
+  EXPECT_EQ(Point({Any, Anchor::Above}, AS_protected), Position{});
+  EXPECT_EQ(Point({Any, Anchor::Below}, AS_protected), Position{});
+
+  // Edits when there's no match --> end of matching access control section.
+  auto Edit = insertDecl("x", C, {}, AS_public);
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+            Code.point("private"));
+
+  Edit = insertDecl("x", C, {}, AS_private);
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+            Code.point("end"));
+
+  Edit = insertDecl("x", C, {}, AS_protected);
+  ASSERT_THAT_EXPECTED(Edit, llvm::Succeeded());
+  EXPECT_EQ(offsetToPosition(Code.code(), Edit->getOffset()),
+            Code.point("end"));
+  EXPECT_EQ(Edit->getReplacementText(), "protected:\nx");
+}
+
+// In ObjC we need to take care to get the @end fallback right.
+TEST(InsertionPointTests, ObjC) {
+  Annotations Code(R"objc(
+    @interface Foo
+     -(void) v;
+    $endIface^@end
+    @implementation Foo
+     -(void) v {}
+    $endImpl^@end
+  )objc");
+  auto TU = TestTU::withCode(Code.code());
+  TU.Filename = "TestTU.m";
+  auto AST = TU.build();
+
+  auto &Impl =
+      cast<ObjCImplementationDecl>(findDecl(AST, [&](const NamedDecl &D) {
+        return llvm::isa<ObjCImplementationDecl>(D);
+      }));
+  auto &Iface = *Impl.getClassInterface();
+  Anchor End{[](const Decl *) { return true; }, Anchor::Below};
+
+  const auto &SM = AST.getSourceManager();
+  EXPECT_EQ(sourceLocToPosition(SM, insertionPoint(Iface, {End})),
+            Code.point("endIface"));
+  EXPECT_EQ(sourceLocToPosition(SM, insertionPoint(Impl, {End})),
+            Code.point("endImpl"));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

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

+//===-- 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::AnyOf;
+using testing::Contains;
+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; }; };");
+}
+
+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; };
+  )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("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