ExtractFunction: support extracting expressions and selected part of it

[KKoovalsky]

Summary

This is mainly insipred by ExtractVariable's ParsedBinaryOperator,
tweaked a little to collect references within the selected area.

On the side:
This is my first PR to the LLVM (and clangd) - I wanted to see how it
goes for the future, to contribute even more, as I am a C++ fan and I
love open-source, plus recently, I really like to automate stuff when
coding!

Getting back to the implemented functionality:
it upports extraction of expressions to a function or method:

• int x{[[a + b + c]]},
• int x{a + [[b + c]] + d};

Handles:

• Binary operators.
• Operator overloads, which resemble binary operators, e.g. struct S{ S& operator+(const S&); }
• Infers return type of the expression basing on the operation return type; refs and const-refs are properly inferred as well.
• Collects parameters from the selected area. Skips global variables and members. (Uses already-in-place DeclInfoMap containing info about the declarations within the enclosing function).
• Handles deeply nested arguments within function calls, and collects them as the extracted function parameters.

Known limitations:

• Macros within the selected area; the selected area is treated as the expr would be selected to the most-LHS entity; effectively: int x{a + [[SOME_MACRO(A) + b]] + c} becomes: int x{[[a + SOME_MACRO(A) + b]] + c} This is known limitation of ExtractVariable, thus nothing new here.
• Does not make the method 'const' if the selected members are not mutated, in case the enclosing method is non-const.

PS. This contribution is a little messy, since it was hard to support
partially selected expressions, because the code already-in-place used
the term "root statement", which assumes that the entire statement is
selected. With this PR this assumption will be broken.
Yet, adding this feature taught me a lot, and I understand the code
structure better. I would like to boil down all the details of
implementing the Extract Function feature as a whole.

[nridge]

Linking to potentially relevant issue on file: https://github.com/clangd/clangd/issues/1254

[KKoovalsky]

Hey! I can see that the build: https://reviews.llvm.org/harbormaster/build/311598/ failed, but I am not sure whether this is related to my change. Could someone take a look?

[nridge]

In D140619#4058232, @KKoovalsky wrote:

Hey! I can see that the build: https://reviews.llvm.org/harbormaster/build/311598/ failed, but I am not sure whether this is related to my change. Could someone take a look?

I can reproduce the failures locally with the patch applied. To do so, run:

<build>/bin/llvm-lit <src>/clang-tools-extra/clangd/test/check.test

and similarly for check-fail.test and check-lines.test.

You can read more about how these tests work at https://llvm.org/docs/CommandGuide/lit.html.

[nridge]

By the way, I also got linker errors when trying to build this patch in a shared-libs build. Resolved by adding clangASTMatchers to clang_target_link_libraries in clang-tools-extra/clangd/tool/CMakeLists.txt.

[nridge]

In D140619#4067940, @nridge wrote:

I can reproduce the failures locally with the patch applied.

(The failures seem to have to do with ExtractFunction segfaulting when invoked at a particular location in the code in check.test.)

[KKoovalsky]

1. Fixed lit fail: dereferencing a null pointer when calling ignoreImplicit() on CommonAncestor being nullptr - fixed with sanitization of CommonAncestor being nullptr.
2. Fixed link error: missing ASTMatchers lib.+

[KKoovalsky]

1. Fixed lit fail: dereferencing a null pointer when calling ignoreImplicit() on CommonAncestor being nullptr - fixed with sanitization of CommonAncestor being nullptr.
2. Fixed link error: missing ASTMatchers lib.

[KKoovalsky]

Fixed nullptr dereferencing and missing ASTMatchers lib linking

1. Fixed lit fail: dereferencing a null pointer when calling ignoreImplicit() on CommonAncestor being nullptr - fixed with sanitization of CommonAncestor being nullptr.
2. Fixed link error: missing ASTMatchers lib.

[KKoovalsky]

@nridge I have fixed the issues. Thank you for supporting.

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

 #include "ParsedAST.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "refactor/Tweak.h"
 #include "support/Logger.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
 
+#include <algorithm>
+#include <optional>
+
 namespace clang {
 namespace clangd {
 namespace {
 
 using Node = SelectionTree::Node;
 
 // ExtractionZone is the part of code that is being extracted.
 // EnclosingFunction is the function/method inside which the zone lies.
 // We split the file into 4 parts relative to extraction zone.
 enum class ZoneRelative {
   Before,     // Before Zone and inside EnclosingFunction.
   Inside,     // Inside Zone.
   After,      // After Zone and inside EnclosingFunction.
   OutsideFunc // Outside EnclosingFunction.
 };
 
 enum FunctionDeclKind {
   InlineDefinition,
   ForwardDeclaration,
   OutOfLineDefinition
 };
 
+// Helpers for handling "binary subexpressions" like a + [[b + c]] + d. This is
+// taken from ExtractVariable, and adapted a little to handle collection of
+// parameters.
+struct ExtractedBinarySubexpressionSelection;
+
+class BinarySubexpressionSelection {
+
+public:
+  static inline std::optional<BinarySubexpressionSelection>
+  tryParse(const SelectionTree::Node &N, const SourceManager *SM) {
+    if (const BinaryOperator *Op =
+            llvm::dyn_cast_or_null<BinaryOperator>(N.ASTNode.get<Expr>())) {
+      return BinarySubexpressionSelection{SM, Op->getOpcode(), Op->getExprLoc(),
+                                          N.Children};
+    }
+    if (const CXXOperatorCallExpr *Op =
+            llvm::dyn_cast_or_null<CXXOperatorCallExpr>(
+                N.ASTNode.get<Expr>())) {
+      if (!Op->isInfixBinaryOp())
+        return std::nullopt;
+
+      llvm::SmallVector<const SelectionTree::Node *> SelectedOps;
+      // Not all children are args, there's also the callee (operator).
+      for (const auto *Child : N.Children) {
+        const Expr *E = Child->ASTNode.get<Expr>();
+        assert(E && "callee and args should be Exprs!");
+        if (E == Op->getArg(0) || E == Op->getArg(1))
+          SelectedOps.push_back(Child);
+      }
+      return BinarySubexpressionSelection{
+          SM, BinaryOperator::getOverloadedOpcode(Op->getOperator()),
+          Op->getExprLoc(), std::move(SelectedOps)};
+    }
+    return std::nullopt;
+  }
+
+  bool associative() const {
+    // Must also be left-associative!
+    switch (Kind) {
+    case BO_Add:
+    case BO_Mul:
+    case BO_And:
+    case BO_Or:
+    case BO_Xor:
+    case BO_LAnd:
+    case BO_LOr:
+      return true;
+    default:
+      return false;
+    }
+  }
+
+  bool crossesMacroBoundary() const {
+    FileID F = SM->getFileID(ExprLoc);
+    for (const SelectionTree::Node *Child : SelectedOperations)
+      if (SM->getFileID(Child->ASTNode.get<Expr>()->getExprLoc()) != F)
+        return true;
+    return false;
+  }
+
+  bool isExtractable() const {
+    return associative() and not crossesMacroBoundary();
+  }
+
+  void dumpSelectedOperations(llvm::raw_ostream &Os,
+                              const ASTContext &Cont) const {
+    for (const auto *Op : SelectedOperations)
+      Op->ASTNode.dump(Os, Cont);
+  }
+
+  std::optional<ExtractedBinarySubexpressionSelection> tryExtract() const;
+
+protected:
+  struct SelectedOperands {
+    llvm::SmallVector<const SelectionTree::Node *> Operands;
+    const SelectionTree::Node *Start;
+    const SelectionTree::Node *End;
+  };
+
+private:
+  BinarySubexpressionSelection(
+      const SourceManager *SM, BinaryOperatorKind Kind, SourceLocation ExprLoc,
+      llvm::SmallVector<const SelectionTree::Node *> SelectedOps)
+      : SM{SM}, Kind(Kind), ExprLoc(ExprLoc),
+        SelectedOperations(std::move(SelectedOps)) {}
+
+  SelectedOperands getSelectedOperands() const {
+    auto [Start, End]{getClosedRangeWithSelectedOperations()};
+
+    llvm::SmallVector<const SelectionTree::Node *> Operands;
+    Operands.reserve(SelectedOperations.size());
+    const SelectionTree::Node *BinOpSelectionIt{Start->Parent};
+
+    // Edge case: the selection starts from the most-left LHS, e.g. [[a+b+c]]+d
+    if (BinOpSelectionIt->Children.size() == 2)
+      Operands.emplace_back(BinOpSelectionIt->Children.front()); // LHS
+    // In case of operator+ call, the Children will contain the calle as well.
+    else if (BinOpSelectionIt->Children.size() == 3)
+      Operands.emplace_back(BinOpSelectionIt->Children[1]); // LHS
+
+    // Go up the Binary Operation three, up to the most-right RHS
+    for (; BinOpSelectionIt->Children.back() != End;
+         BinOpSelectionIt = BinOpSelectionIt->Parent)
+      Operands.emplace_back(BinOpSelectionIt->Children.back()); // RHS
+    // Remember to add the most-right RHS
+    Operands.emplace_back(End);
+
+    SelectedOperands Ops;
+    Ops.Start = Start;
+    Ops.End = End;
+    Ops.Operands = std::move(Operands);
+    return Ops;
+  }
+
+  std::pair<const SelectionTree::Node *, const SelectionTree::Node *>
+  getClosedRangeWithSelectedOperations() const {
+    BinaryOperatorKind OuterOp = Kind;
+    // Because the tree we're interested in contains only one operator type, and
+    // all eligible operators are left-associative, the shape of the tree is
+    // very restricted: it's a linked list along the left edges.
+    // This simplifies our implementation.
+    const SelectionTree::Node *Start = SelectedOperations.front(); // LHS
+    const SelectionTree::Node *End = SelectedOperations.back();    // RHS
+
+    // End is already correct: it can't be an OuterOp (as it's
+    // left-associative). Start needs to be pushed down int the subtree to the
+    // right spot.
+    while (true) {
+      auto MaybeOp{tryParse(Start->ignoreImplicit(), SM)};
+      if (not MaybeOp)
+        break;
+      const auto &Op{*MaybeOp};
+      if (Op.Kind != OuterOp or Op.crossesMacroBoundary())
+        break;
+      assert(!Op.SelectedOperations.empty() &&
+             "got only operator on one side!");
+      if (Op.SelectedOperations.size() == 1) { // Only Op.RHS selected
+        Start = Op.SelectedOperations.back();
+        break;
+      }
+      // Op.LHS is (at least partially) selected, so descend into it.
+      Start = Op.SelectedOperations.front();
+    }
+    return {Start, End};
+  }
+
+protected:
+  const SourceManager *SM;
+  BinaryOperatorKind Kind;
+  SourceLocation ExprLoc;
+  // May also contain partially selected operations,
+  // e.g. a + [[b + c]], will keep (a + b) BinaryOperator.
+  llvm::SmallVector<const SelectionTree::Node *> SelectedOperations;
+};
+
+struct ExtractedBinarySubexpressionSelection : BinarySubexpressionSelection {
+  ExtractedBinarySubexpressionSelection(BinarySubexpressionSelection BinSubexpr,
+                                        SelectedOperands SelectedOps)
+      : BinarySubexpressionSelection::BinarySubexpressionSelection(
+            std::move(BinSubexpr)),
+        Operands{std::move(SelectedOps)} {}
+
+  SourceRange getRange(const LangOptions &LangOpts) const {
+    auto MakeHalfOpenFileRange{[&](const SelectionTree::Node *N) {
+      return toHalfOpenFileRange(*SM, LangOpts, N->ASTNode.getSourceRange());
+    }};
+
+    return SourceRange(MakeHalfOpenFileRange(Operands.Start)->getBegin(),
+                       MakeHalfOpenFileRange(Operands.End)->getEnd());
+  }
+
+  void dumpSelectedOperands(llvm::raw_ostream &Os,
+                            const ASTContext &Cont) const {
+    for (const auto *Op : Operands.Operands)
+      Op->ASTNode.dump(Os, Cont);
+  }
+
+  llvm::SmallVector<const DeclRefExpr *>
+  collectReferences(ASTContext &Cont) const {
+    llvm::SmallVector<const DeclRefExpr *> Refs;
+    auto Matcher{
+        ast_matchers::findAll(ast_matchers::declRefExpr().bind("ref"))};
+    for (const auto *SelNode : Operands.Operands) {
+      auto Matches{ast_matchers::match(Matcher, SelNode->ASTNode, Cont)};
+      for (const auto &Match : Matches)
+        if (const DeclRefExpr * Ref{Match.getNodeAs<DeclRefExpr>("ref")}; Ref)
+          Refs.push_back(Ref);
+    }
+    return Refs;
+  }
+
+private:
+  SelectedOperands Operands;
+};
+
+std::optional<ExtractedBinarySubexpressionSelection>
+BinarySubexpressionSelection::tryExtract() const {
+  if (not isExtractable())
+    return std::nullopt;
+  return ExtractedBinarySubexpressionSelection{*this, getSelectedOperands()};
+}
+
 // A RootStmt is a statement that's fully selected including all it's children
 // and it's parent is unselected.
 // Check if a node is a root statement.
 bool isRootStmt(const Node *N) {
   if (!N->ASTNode.get<Stmt>())
     return false;
   // Root statement cannot be partially selected.
   if (N->Selected == SelectionTree::Partial)

 // 2. If all completely selected nodes don't have the same common parent
 // 3. Any child of Parent isn't a RootStmt.
 // Returns null if any child is not a RootStmt.
 // We only support extraction of RootStmts since it allows us to extract without
 // having to change the selection range. Also, this means that any scope that
 // begins in selection range, ends in selection range and any scope that begins
 // outside the selection range, ends outside as well.
 const Node *getParentOfRootStmts(const Node *CommonAnc) {
-  if (!CommonAnc)
-    return nullptr;
   const Node *Parent = nullptr;
   switch (CommonAnc->Selected) {
   case SelectionTree::Selection::Unselected:
+    // Workaround for an operator call: BinaryOperator will be selecteded
+    // completely, but the operator call would be unselected, thus we treat it
+    // as it would be completely selected.
+    if (CommonAnc->ASTNode.get<CXXOperatorCallExpr>() != nullptr)
+      return CommonAnc->Parent;
     // Typically a block, with the { and } unselected, could also be ForStmt etc
     // Ensure all Children are RootStmts.
     Parent = CommonAnc;
     break;
   case SelectionTree::Selection::Partial:
     // Only a fully-selected single statement can be selected.
     return nullptr;
   case SelectionTree::Selection::Complete:
case SelectionTree::Selection::Complete:
     break;
   }
   // Ensure all Children are RootStmts.
   return llvm::all_of(Parent->Children, isRootStmt) ? Parent : nullptr;
 }
 
 // The ExtractionZone class forms a view of the code wrt Zone.
 struct ExtractionZone {
+  const Node *CommonAncestor;
   // Parent of RootStatements being extracted.
   const Node *Parent = nullptr;
   // The half-open file range of the code being extracted.
   SourceRange ZoneRange;
   // The function inside which our zone resides.
   const FunctionDecl *EnclosingFunction = nullptr;
   // The half-open file range of the enclosing function.
   SourceRange EnclosingFuncRange;
   // Set of statements that form the ExtractionZone.
   llvm::DenseSet<const Stmt *> RootStmts;
+  // If the extraction zone is a "binary subexpression", then this will be set.
+  std::optional<BinarySubexpressionSelection> MaybeBinarySubexpr;
 
   SourceLocation getInsertionPoint() const {
     return EnclosingFuncRange.getBegin();
   }
   bool isRootStmt(const Stmt *S) const;
   // The last root statement is important to decide where we need to insert a
   // semicolon after the extraction.
   const Node *getLastRootStmt() const { return Parent->Children.back(); }

 // between the attributes and the EnclosingFunction.
 llvm::Optional<SourceRange>
 computeEnclosingFuncRange(const FunctionDecl *EnclosingFunction,
                           const SourceManager &SM,
                           const LangOptions &LangOpts) {
   return toHalfOpenFileRange(SM, LangOpts, EnclosingFunction->getSourceRange());
 }
 
-// returns true if Child can be a single RootStmt being extracted from
-// EnclosingFunc.
-bool validSingleChild(const Node *Child, const FunctionDecl *EnclosingFunc) {
-  // Don't extract expressions.
-  // FIXME: We should extract expressions that are "statements" i.e. not
-  // subexpressions
-  if (Child->ASTNode.get<Expr>())
-    return false;
-  // Extracting the body of EnclosingFunc would remove it's definition.
-  assert(EnclosingFunc->hasBody() &&
+bool isEntireFunctionBodySelected(const ExtractionZone &ExtZone) {
+  assert(ExtZone.EnclosingFunction->hasBody() &&
          "We should always be extracting from a function body.");
-  if (Child->ASTNode.get<Stmt>() == EnclosingFunc->getBody())
-    return false;
-  return true;
+  return ExtZone.Parent->Children.size() == 1 &&
+         ExtZone.getLastRootStmt()->ASTNode.get<Stmt>() ==
+             ExtZone.EnclosingFunction->getBody();
 }
 
 // FIXME: Check we're not extracting from the initializer/condition of a control
 // flow structure.
 llvm::Optional<ExtractionZone> findExtractionZone(const Node *CommonAnc,
                                                   const SourceManager &SM,
                                                   const LangOptions &LangOpts) {
+  if (CommonAnc == nullptr)
+    return std::nullopt;
   ExtractionZone ExtZone;
+  ExtZone.CommonAncestor = CommonAnc;
+  auto MaybeBinarySubexpr{
+      BinarySubexpressionSelection::tryParse(CommonAnc->ignoreImplicit(), &SM)};
+  if (MaybeBinarySubexpr) {
+    // FIXME: We shall not allow the user to extract expressions which we don't
+    // support, or which are weirdly selected (e.g. a [[+ b + c]]). If the
+    // selected subexpression is an entire expression (not only a part of
+    // expression), then we don't need the BinarySubexpressionSelection.
+    if (const auto &BinarySubexpr{*MaybeBinarySubexpr};
+        BinarySubexpr.isExtractable()) {
+      ExtZone.MaybeBinarySubexpr = std::move(MaybeBinarySubexpr);
+    }
+  }
   ExtZone.Parent = getParentOfRootStmts(CommonAnc);
   if (!ExtZone.Parent || ExtZone.Parent->Children.empty())
     return std::nullopt;
   ExtZone.EnclosingFunction = findEnclosingFunction(ExtZone.Parent);
   if (!ExtZone.EnclosingFunction)
     return std::nullopt;
-  // When there is a single RootStmt, we must check if it's valid for
-  // extraction.
-  if (ExtZone.Parent->Children.size() == 1 &&
-      !validSingleChild(ExtZone.getLastRootStmt(), ExtZone.EnclosingFunction))
+  // Extracting the body of EnclosingFunc would remove it's definition.
+  if (isEntireFunctionBodySelected(ExtZone))
     return std::nullopt;
   if (auto FuncRange =
           computeEnclosingFuncRange(ExtZone.EnclosingFunction, SM, LangOpts))
     ExtZone.EnclosingFuncRange = *FuncRange;
   if (auto ZoneRange = findZoneRange(ExtZone.Parent, SM, LangOpts))
     ExtZone.ZoneRange = *ZoneRange;
   if (ExtZone.EnclosingFuncRange.isInvalid() || ExtZone.ZoneRange.isInvalid())
     return std::nullopt;
struct NewFunction {
   const NestedNameSpecifier *DefinitionQualifier = nullptr;
   const DeclContext *SemanticDC = nullptr;
   const DeclContext *SyntacticDC = nullptr;
   const DeclContext *ForwardDeclarationSyntacticDC = nullptr;
   bool CallerReturnsValue = false;
   bool Static = false;
   ConstexprSpecKind Constexpr = ConstexprSpecKind::Unspecified;
   bool Const = false;
+  bool Expression = false;
 
   // Decides whether the extracted function body and the function call need a
   // semicolon after extraction.
   tooling::ExtractionSemicolonPolicy SemicolonPolicy;
   const LangOptions *LangOpts;
   NewFunction(tooling::ExtractionSemicolonPolicy SemicolonPolicy,
               const LangOptions *LangOpts)
       : SemicolonPolicy(SemicolonPolicy), LangOpts(LangOpts) {}
std::string NewFunction::renderDeclaration(FunctionDeclKind K,
   llvm_unreachable("Unsupported FunctionDeclKind enum");
 }
 
 std::string NewFunction::getFuncBody(const SourceManager &SM) const {
   // FIXME: Generate tooling::Replacements instead of std::string to
   // - hoist decls
   // - add return statement
   // - Add semicolon
-  return toSourceCode(SM, BodyRange).str() +
-         (SemicolonPolicy.isNeededInExtractedFunction() ? ";" : "");
+  auto NewBody{toSourceCode(SM, BodyRange).str() +
+               (SemicolonPolicy.isNeededInExtractedFunction() ? ";" : "")};
+  if (Expression)
+    return "return " + NewBody;
+  return NewBody;
 }
 
 std::string NewFunction::Parameter::render(const DeclContext *Context) const {
   return printType(TypeInfo, *Context) + (PassByReference ? " &" : " ") + Name;
 }
 
 // Stores captured information about Extraction Zone.
 struct CapturedZoneInfo {
struct CapturedZoneInfo {
   bool AlwaysReturns = false; // Does the zone always return?
   // Control flow is broken if we are extracting a break/continue without a
   // corresponding parent loop/switch
   bool BrokenControlFlow = false;
   // FIXME: capture TypeAliasDecl and UsingDirectiveDecl
   // FIXME: Capture type information as well.
   DeclInformation *createDeclInfo(const Decl *D, ZoneRelative RelativeLoc);
   DeclInformation *getDeclInfoFor(const Decl *D);
+  const DeclInformation *getDeclInfoFor(const Decl *D) const;
 };
 
 CapturedZoneInfo::DeclInformation *
 CapturedZoneInfo::createDeclInfo(const Decl *D, ZoneRelative RelativeLoc) {
   // The new Decl's index is the size of the map so far.
   auto InsertionResult = DeclInfoMap.insert(
       {D, DeclInformation(D, RelativeLoc, DeclInfoMap.size())});
   // Return the newly created DeclInfo
   return &InsertionResult.first->second;
 }
 
 CapturedZoneInfo::DeclInformation *
 CapturedZoneInfo::getDeclInfoFor(const Decl *D) {
-  // If the Decl doesn't exist, we
+  auto Iter = DeclInfoMap.find(D);
+  if (Iter == DeclInfoMap.end())
+    return nullptr;
+  return &Iter->second;
+}
+
+const CapturedZoneInfo::DeclInformation *
+CapturedZoneInfo::getDeclInfoFor(const Decl *D) const {
   auto Iter = DeclInfoMap.find(D);
   if (Iter == DeclInfoMap.end())
     return nullptr;
   return &Iter->second;
 }
 
 void CapturedZoneInfo::DeclInformation::markOccurence(
     ZoneRelative ReferenceLoc) {
public:
     unsigned CurNumberOfSwitch = 0;
   };
   ExtractionZoneVisitor Visitor(ExtZone);
   CapturedZoneInfo Result = std::move(Visitor.Info);
   Result.AlwaysReturns = alwaysReturns(ExtZone);
   return Result;
 }
 
-// Adds parameters to ExtractedFunc.
-// Returns true if able to find the parameters successfully and no hoisting
-// needed.
+static const ValueDecl *unpackDeclForParameter(const Decl *D) {
+  const ValueDecl *VD = dyn_cast_or_null<ValueDecl>(D);
+  // Can't parameterise if the Decl isn't a ValueDecl or is a FunctionDecl
+  // (this includes the case of recursive call to EnclosingFunc in Zone).
+  if (!VD || isa<FunctionDecl>(D))
+    return nullptr;
+  return VD;
+}
+
+static QualType getParameterTypeInfo(const ValueDecl *VD) {
+  // Parameter qualifiers are same as the Decl's qualifiers.
+  return VD->getType().getNonReferenceType();
+}
+
+using Parameters = std::vector<NewFunction::Parameter>;
+using MaybeParameters = std::optional<Parameters>;
+
 // FIXME: Check if the declaration has a local/anonymous type
-bool createParameters(NewFunction &ExtractedFunc,
-                      const CapturedZoneInfo &CapturedInfo) {
+// Returns actual parameters if able to find the parameters successfully and no
+// hoisting needed.
+static MaybeParameters
+createParamsForNoSubexpr(const CapturedZoneInfo &CapturedInfo) {
+  std::vector<NewFunction::Parameter> Params;
   for (const auto &KeyVal : CapturedInfo.DeclInfoMap) {
     const auto &DeclInfo = KeyVal.second;
     // If a Decl was Declared in zone and referenced in post zone, it
     // needs to be hoisted (we bail out in that case).
     // FIXME: Support Decl Hoisting.
     if (DeclInfo.DeclaredIn == ZoneRelative::Inside &&
         DeclInfo.IsReferencedInPostZone)
-      return false;
+      return std::nullopt;
     if (!DeclInfo.IsReferencedInZone)
       continue; // no need to pass as parameter, not referenced
     if (DeclInfo.DeclaredIn == ZoneRelative::Inside ||
         DeclInfo.DeclaredIn == ZoneRelative::OutsideFunc)
       continue; // no need to pass as parameter, still accessible.
-    // Parameter specific checks.
-    const ValueDecl *VD = dyn_cast_or_null<ValueDecl>(DeclInfo.TheDecl);
-    // Can't parameterise if the Decl isn't a ValueDecl or is a FunctionDecl
-    // (this includes the case of recursive call to EnclosingFunc in Zone).
-    if (!VD || isa<FunctionDecl>(DeclInfo.TheDecl))
-      return false;
-    // Parameter qualifiers are same as the Decl's qualifiers.
-    QualType TypeInfo = VD->getType().getNonReferenceType();
+    const auto *VD{unpackDeclForParameter(DeclInfo.TheDecl)};
+    if (VD == nullptr)
+      return std::nullopt;
+    QualType TypeInfo{getParameterTypeInfo(VD)};
     // FIXME: Need better qualifier checks: check mutated status for
     // Decl(e.g. was it assigned, passed as nonconst argument, etc)
     // FIXME: check if parameter will be a non l-value reference.
     // FIXME: We don't want to always pass variables of types like int,
     // pointers, etc by reference.
     bool IsPassedByReference = true;
     // We use the index of declaration as the ordering priority for parameters.
-    ExtractedFunc.Parameters.push_back({std::string(VD->getName()), TypeInfo,
-                                        IsPassedByReference,
-                                        DeclInfo.DeclIndex});
+    Params.push_back({std::string(VD->getName()), TypeInfo, IsPassedByReference,
   }
-  llvm::sort(ExtractedFunc.Parameters);
-  return true;
+  llvm::sort(Params);
+  return Params;
+}
+
+static MaybeParameters
+createParamsForSubexpr(const CapturedZoneInfo &CapturedInfo,
+                       const ExtractedBinarySubexpressionSelection &Subexpr,
+                       ASTContext &ASTCont) {
+  // We use the the Set here, to avoid duplicates, but since the Set will not
+  // care about the order, we need to use a vector to collect the unique
+  // references in the order of referencing.
+  llvm::SmallVector<const ValueDecl *> RefsAsDecls;
+  llvm::DenseSet<const ValueDecl *> UniqueRefsAsDecls;
+
+  for (const auto *Ref : Subexpr.collectReferences(ASTCont)) {
+    const auto *D{Ref->getDecl()};
+    const auto *VD{unpackDeclForParameter(D)};
+    // Only collect the ValueDecl-s.
+    if (VD == nullptr)
+      continue;
+    const auto *DeclInfo{CapturedInfo.getDeclInfoFor(D)};
+    if (DeclInfo == nullptr or DeclInfo->DeclaredIn != ZoneRelative::Before)
+      continue;
+    auto [It, IsNew]{UniqueRefsAsDecls.insert(VD)};
+    if (IsNew)
+      RefsAsDecls.emplace_back(VD);
+  }
+
+  std::vector<NewFunction::Parameter> Params;
+  std::transform(std::begin(RefsAsDecls), std::end(RefsAsDecls),
+                 std::back_inserter(Params), [](const ValueDecl *VD) {
+                   QualType TypeInfo{getParameterTypeInfo(VD)};
+                   // FIXME: Need better qualifier checks: check mutated status
+                   // for Decl(e.g. was it assigned, passed as nonconst
+                   // argument, etc)
+                   // FIXME: check if parameter will be a non l-value reference.
+                   // FIXME: We don't want to always pass variables of types
+                   // like int, pointers, etc by reference.
+                   bool IsPassedByRef = true;
+                   return NewFunction::Parameter{std::string(VD->getName()),
+                                                 TypeInfo, IsPassedByRef, 0};
+                 });
+  return Params;
+}
+
+// Adds parameters to ExtractedFunc.
+MaybeParameters createParams(
+    const std::optional<ExtractedBinarySubexpressionSelection> &MaybeSubexpr,
+    const CapturedZoneInfo &CapturedInfo, ASTContext &ASTCont) {
+  if (MaybeSubexpr)
+    return createParamsForSubexpr(CapturedInfo, *MaybeSubexpr, ASTCont);
+  return createParamsForNoSubexpr(CapturedInfo);
 }
 
 // Clangd uses open ranges while ExtractionSemicolonPolicy (in Clang Tooling)
 // uses closed ranges. Generates the semicolon policy for the extraction and
 // extends the ZoneRange if necessary.
 tooling::ExtractionSemicolonPolicy
 getSemicolonPolicy(ExtractionZone &ExtZone, const SourceManager &SM,
                    const LangOptions &LangOpts) {
   // Get closed ZoneRange.
   SourceRange FuncBodyRange = {ExtZone.ZoneRange.getBegin(),
                                ExtZone.ZoneRange.getEnd().getLocWithOffset(-1)};
   auto SemicolonPolicy = tooling::ExtractionSemicolonPolicy::compute(
       ExtZone.getLastRootStmt()->ASTNode.get<Stmt>(), FuncBodyRange, SM,
       LangOpts);
   // Update ZoneRange.
   ExtZone.ZoneRange.setEnd(FuncBodyRange.getEnd().getLocWithOffset(1));
   return SemicolonPolicy;
 }
 
+// Returns true if the selected code is an expression, false otherwise.
+bool isExpression(const ExtractionZone &ExtZone) {
+  const auto &Node{*ExtZone.Parent};
+  return Node.Children.size() == 1 and
+         ExtZone.getLastRootStmt()->ASTNode.get<Expr>() != nullptr;
+}
+
 // Generate return type for ExtractedFunc. Return false if unable to do so.
-bool generateReturnProperties(NewFunction &ExtractedFunc,
-                              const FunctionDecl &EnclosingFunc,
-                              const CapturedZoneInfo &CapturedInfo) {
+std::optional<QualType>
+generateReturnProperties(const ExtractionZone &ExtZone,
   // If the selected code always returns, we preserve those return statements.
   // The return type should be the same as the enclosing function.
   // (Others are possible if there are conversions, but this seems clearest).
+  const auto &EnclosingFunc{*ExtZone.EnclosingFunction};
   if (CapturedInfo.HasReturnStmt) {
     // If the return is conditional, neither replacing the code with
     // `extracted()` nor `return extracted()` is correct.
     if (!CapturedInfo.AlwaysReturns)
-      return false;
+      return std::nullopt;
     QualType Ret = EnclosingFunc.getReturnType();
-    // Once we support members, it'd be nice to support e.g. extracting a method
-    // of Foo<T> that returns T. But it's not clear when that's safe.
+    // Once we support members, it'd be nice to support e.g. extracting a
+    // method of Foo<T> that returns T. But it's not clear when that's safe.
     if (Ret->isDependentType())
-      return false;
-    ExtractedFunc.ReturnType = Ret;
-    return true;
+      return std::nullopt;
+    return Ret;
+  }
+  // If the selected code is an expression, then take the return type of it.
+  if (const auto &Node{*ExtZone.Parent}; Node.Children.size() == 1) {
+    if (const Expr * Expression{ExtZone.getLastRootStmt()->ASTNode.get<Expr>()};
+        Expression) {
+      if (const auto *Call{llvm::dyn_cast_or_null<CallExpr>(Expression)};
+          Call) {
+        const auto &ASTCont{ExtZone.EnclosingFunction->getParentASTContext()};
+        return Call->getCallReturnType(ASTCont);
+      }
+      return Expression->getType();
+    }
   }
   // FIXME: Generate new return statement if needed.
-  ExtractedFunc.ReturnType = EnclosingFunc.getParentASTContext().VoidTy;
-  return true;
+  return EnclosingFunc.getParentASTContext().VoidTy;
 }
 
 void captureMethodInfo(NewFunction &ExtractedFunc,
                        const CXXMethodDecl *Method) {
   ExtractedFunc.Static = Method->isStatic();
   ExtractedFunc.Const = Method->isConst();
   ExtractedFunc.EnclosingClass = Method->getParent();
 }
if (ExtZone.EnclosingFunction->isOutOfLine()) {
     auto DeclPos =
         toHalfOpenFileRange(SM, LangOpts, FirstOriginalDecl->getSourceRange());
     if (!DeclPos)
       return error("Declaration is inside a macro");
     ExtractedFunc.ForwardDeclarationPoint = DeclPos->getBegin();
     ExtractedFunc.ForwardDeclarationSyntacticDC = ExtractedFunc.SemanticDC;
   }
 
+  auto &ASTCont{ExtZone.EnclosingFunction->getASTContext()};
+  ExtractedFunc.Expression = isExpression(ExtZone);
+  std::optional<ExtractedBinarySubexpressionSelection> MaybeExtractedSubexpr;
+  if (ExtZone.MaybeBinarySubexpr) {
+    MaybeExtractedSubexpr = ExtZone.MaybeBinarySubexpr->tryExtract();
+    ExtractedFunc.BodyRange = MaybeExtractedSubexpr->getRange(LangOpts);
+  } else {
     ExtractedFunc.BodyRange = ExtZone.ZoneRange;
-  ExtractedFunc.DefinitionPoint = ExtZone.getInsertionPoint();
+  }
 
+  ExtractedFunc.DefinitionPoint = ExtZone.getInsertionPoint();
   ExtractedFunc.CallerReturnsValue = CapturedInfo.AlwaysReturns;
-  if (!createParameters(ExtractedFunc, CapturedInfo) ||
-      !generateReturnProperties(ExtractedFunc, *ExtZone.EnclosingFunction,
-                                CapturedInfo))
+
+  auto MaybeRetType{generateReturnProperties(ExtZone, CapturedInfo)};
+  auto MaybeParams{createParams(MaybeExtractedSubexpr, CapturedInfo, ASTCont)};
+  if (not MaybeRetType || not MaybeParams)
     return error("Too complex to extract.");
+  ExtractedFunc.ReturnType = std::move(*MaybeRetType);
+  ExtractedFunc.Parameters = std::move(*MaybeParams);
   return ExtractedFunc;
 }
 
 class ExtractFunction : public Tweak {
 public:
   const char *id() const final;
   bool prepare(const Selection &Inputs) override;
   Expected<Effect> apply(const Selection &Inputs) override;
if (SM.isWrittenInSameFile(ExtractedFunc->DefinitionPoint, *FwdLoc)) {
         return std::move(Err);
     } else {
       auto MultiFileEffect = Effect::mainFileEdit(SM, std::move(Edit));
       if (!MultiFileEffect)
         return MultiFileEffect.takeError();
 
       tooling::Replacements OtherEdit(
           createForwardDeclaration(*ExtractedFunc, SM));
-      if (auto PathAndEdit = Tweak::Effect::fileEdit(SM, SM.getFileID(*FwdLoc),
-                                                 OtherEdit))
+      if (auto PathAndEdit =
+              Tweak::Effect::fileEdit(SM, SM.getFileID(*FwdLoc), OtherEdit))
         MultiFileEffect->ApplyEdits.try_emplace(PathAndEdit->first,
                                                 PathAndEdit->second);
       else
         return PathAndEdit.takeError();
       return MultiFileEffect;
     }
   }
   return Effect::mainFileEdit(SM, std::move(Edit));
 }
 
 } // namespace
 } // namespace clangd
 } // namespace clang

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

 set(CLANGD_XPC_LIBS "")
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_XPC_LIBS "clangdXpcJsonConversions" "clangdXpcTransport")
 endif()
 
 clang_target_link_libraries(clangd
   PRIVATE
   clangAST
+  clangASTMatchers
   clangBasic
   clangFormat
   clangFrontend
   clangLex
   clangSema
   clangTooling
   clangToolingCore
   clangToolingRefactoring

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

 
 TWEAK_TEST(ExtractFunction);
 
 TEST_F(ExtractFunctionTest, FunctionTest) {
   Context = Function;
 
   // Root statements should have common parent.
   EXPECT_EQ(apply("for(;;) [[1+2; 1+2;]]"), "unavailable");
-  // Expressions aren't extracted.
-  EXPECT_EQ(apply("int x = 0; [[x++;]]"), "unavailable");
   // We don't support extraction from lambdas.
   EXPECT_EQ(apply("auto lam = [](){ [[int x;]] }; "), "unavailable");
   // Partial statements aren't extracted.
   EXPECT_THAT(apply("int [[x = 0]];"), "unavailable");
   // FIXME: Support hoisting.
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), "unavailable");
 
   // Ensure that end of Zone and Beginning of PostZone being adjacent doesn't
F (extracted();)
   EXPECT_EQ(apply("void f(int a = [[1]]);"), "unavailable");
   // Don't extract if we select the entire function body (CompoundStmt).
   std::string CompoundFailInput = R"cpp(
     void f() [[{
       int a;
     }]]
   )cpp";
   EXPECT_EQ(apply(CompoundFailInput), "unavailable");
+
+  std::string CompoundWithMultipleStatementsFailInput = R"cpp(
+    void f() [[{
+      int a = 1;
+      int b = 2;
+      ++b;
+      b += a;
+    }]]
+  )cpp";
+  EXPECT_EQ(apply(CompoundWithMultipleStatementsFailInput), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, DifferentHeaderSourceTest) {
   Header = R"cpp(
     class SomeClass {
       void f();
     };
   )cpp";
int getNum(bool Superstitious, int Min, int Max) {
       if (Superstitious) return extracted(Min, Max); else {
         return (Min + Max) / 2;
       }
     }
   )cpp";
   EXPECT_EQ(apply(Before), After);
 }
 
+TEST_F(ExtractFunctionTest, Expressions) {
+  std::vector<std::pair<std::string, std::string>> InputOutputs{
+      // FULL BINARY EXPRESSIONS
+      // Full binary expression, basic maths
+      {R"cpp(
+void wrapperFun() {
+  double a{2.0}, b{3.2}, c{31.55};
+  double v{[[b * b - 4 * a * c]]};
+}
+      )cpp",
+       R"cpp(
+double extracted(double &a, double &b, double &c) {
+return b * b - 4 * a * c;
+}
+void wrapperFun() {
+  double a{2.0}, b{3.2}, c{31.55};
+  double v{extracted(a, b, c)};
+}
+      )cpp"},
+      // Full binary expression composed of '+' operator overloads ops
+      {
+          R"cpp(
+struct S {
+  S operator+(const S&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{[[S1 + S2 + S3]]};
+}
+      )cpp",
+          R"cpp(
+struct S {
+  S operator+(const S&) {
+    return *this;
+  }
+};
+S extracted(S &S1, S &S2, S &S3) {
+return S1 + S2 + S3;
+}
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{extracted(S1, S2, S3)};
+}
+      )cpp"},
+      // Boolean predicate as expression
+      {
+          R"cpp(
+void wrapperFun() {
+  int a{1};
+  auto R{[[a > 1]]};
+}
+      )cpp",
+          R"cpp(
+bool extracted(int &a) {
+return a > 1;
+}
+void wrapperFun() {
+  int a{1};
+  auto R{extracted(a)};
+}
+      )cpp"},
+      // Expression: captures no global variable
+      {R"cpp(
+static int a{2};
+void wrapperFun() {
+  int b{3}, c{31}, d{311};
+  auto v{[[a + b + c + d]]};
+}
+      )cpp",
+       R"cpp(
+static int a{2};
+int extracted(int &b, int &c, int &d) {
+return a + b + c + d;
+}
+void wrapperFun() {
+  int b{3}, c{31}, d{311};
+  auto v{extracted(b, c, d)};
+}
+      )cpp"},
+      // Full expr: infers return type of call returning by ref
+      {
+          R"cpp(
+struct S {
+  S& operator+(const S&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{[[S1 + S2 + S3]]};
+}
+      )cpp",
+          R"cpp(
+struct S {
+  S& operator+(const S&) {
+    return *this;
+  }
+};
+S & extracted(S &S1, S &S2, S &S3) {
+return S1 + S2 + S3;
+}
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{extracted(S1, S2, S3)};
+}
+      )cpp"},
+      // Full expr: infers return type of call returning by const-ref
+      {
+          R"cpp(
+struct S {
+  const S& operator+(const S&) const {
+    return *this;
+  }
+};
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{[[S1 + S2 + S3]]};
+}
+      )cpp",
+          R"cpp(
+struct S {
+  const S& operator+(const S&) const {
+    return *this;
+  }
+};
+const S & extracted(S &S1, S &S2, S &S3) {
+return S1 + S2 + S3;
+}
+void wrapperFun() {
+  S S1, S2, S3;
+  auto R{extracted(S1, S2, S3)};
+}
+      )cpp"},
+      // Captures deeply nested arguments
+      {
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{[[fw(fw(fw(a))) + fw(fw(add(b, c))) + fw(fw(fw(add(d, e)))) + fw(fw(f))]]};
+}
+      )cpp",
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+int extracted(int &a, int &b, int &c, int &d, int &e, int &f) {
+return fw(fw(fw(a))) + fw(fw(add(b, c))) + fw(fw(fw(add(d, e)))) + fw(fw(f));
+}
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{extracted(a, b, c, d, e, f)};
+}
+      )cpp"},
+      // SUBEXPRESSIONS
+      // Left-aligned subexpression
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{13};
+  auto v{[[a + b]] + c + d};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &a, int &b) {
+return a + b;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{13};
+  auto v{extracted(a, b) + c + d};
+}
+      )cpp"},
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{13};
+  auto v{[[a + b + c]] + d};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &a, int &b, int &c) {
+return a + b + c;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{13};
+  auto v{extracted(a, b, c) + d};
+}
+      )cpp"},
+      // Subexpression from the middle
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{15}, e{300};
+  auto v{a + [[b + c + d]] + e};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &b, int &c, int &d) {
+return b + c + d;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{15}, e{300};
+  auto v{a + extracted(b, c, d) + e};
+}
+      )cpp"},
+      // Right-aligned subexpression
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{15}, e{300};
+  auto v{a + b + [[c + d + e]]};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &c, int &d, int &e) {
+return c + d + e;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{15}, e{300};
+  auto v{a + b + extracted(c, d, e)};
+}
+      )cpp"},
+      // Larger subexpression from the middle
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{311};
+  auto v{a + [[a + b + c + d]] + c};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &a, int &b, int &c, int &d) {
+return a + b + c + d;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{311};
+  auto v{a + extracted(a, b, c, d) + c};
+}
+      )cpp"},
+      // Subexpression with duplicated references
+      {R"cpp(
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{311};
+  auto v{a + b + [[c + c + c + d + d]] + c};
+}
+      )cpp",
+       R"cpp(
+int extracted(int &c, int &d) {
+return c + c + c + d + d;
+}
+void wrapperFun() {
+  int a{2}, b{3}, c{31}, d{311};
+  auto v{a + b + extracted(c, d) + c};
+}
+      )cpp"},
+      // Subexpression: captures no global variable
+      {R"cpp(
+static int a{2};
+void wrapperFun() {
+  int b{3}, c{31}, d{311};
+  auto v{[[a + b + c]] + d};
+}
+      )cpp",
+       R"cpp(
+static int a{2};
+int extracted(int &b, int &c) {
+return a + b + c;
+}
+void wrapperFun() {
+  int b{3}, c{31}, d{311};
+  auto v{extracted(b, c) + d};
+}
+      )cpp"},
+      // Subexpression: infers return type of call returning by ref, LHS
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2;
+  auto LS3{[[LS1.get()]] + LS2};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS1) {
+return LS1.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2;
+  auto LS3{extracted(LS1) + LS2};
+}
+      )cpp"},
+      // Subexpression: infers return type of call returning by ref, most-RHS
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3;
+  auto LS4{LS1 + LS2 + [[LS3.get()]]};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS3) {
+return LS3.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3;
+  auto LS4{LS1 + LS2 + extracted(LS3)};
+}
+      )cpp"},
+      // Subexpression: infers return type of call returning by ref, middle RHS
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct getCopy() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3;
+  auto LS4{LS1.getCopy() + [[LS2.get()]] + LS3};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct getCopy() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS2) {
+return LS2.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3;
+  auto LS4{LS1.getCopy() + extracted(LS2) + LS3};
+}
+      )cpp"},
+      // Subexpr: infers return type of call returning by const-ref
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2;
+  auto LS3{LS1 + [[LS2.get()]]};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+const LargeStruct & extracted(LargeStruct &LS2) {
+return LS2.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2;
+  auto LS3{LS1 + extracted(LS2)};
+}
+      )cpp"},
+      // Subexpression on operator overload, left-aligned
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4;
+  auto& LS5{[[LS1 + LS2.get()]] + LS3.get() + LS4};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS1, LargeStruct &LS2) {
+return LS1 + LS2.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4;
+  auto& LS5{extracted(LS1, LS2) + LS3.get() + LS4};
+}
+      )cpp"},
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4;
+  auto& LS5{[[LS1 + LS2.get() + LS3.get()]] + LS4};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS1, LargeStruct &LS2, LargeStruct &LS3) {
+return LS1 + LS2.get() + LS3.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4;
+  auto& LS5{extracted(LS1, LS2, LS3) + LS4};
+}
+      )cpp"},
+      // Subexpression on operator overload, middle-aligned
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4, LS5;
+  auto& R{LS1 + [[LS2.get() + LS3 + LS4.get()]] + LS5};
+}
+      )cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS2, LargeStruct &LS3, LargeStruct &LS4) {
+return LS2.get() + LS3 + LS4.get();
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4, LS5;
+  auto& R{LS1 + extracted(LS2, LS3, LS4) + LS5};
+}
+      )cpp"},
+      // Subexpression on operator overload, right-aligned
+      {
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4, LS5;
+  auto& R{LS1 + LS2.get() + [[LS3 + LS4.get() + LS5]]};
+})cpp",
+          R"cpp(
+struct LargeStruct {
+  char LargeMember[1024];
+  const LargeStruct& get() {
+    return *this;
+  }
+  LargeStruct& operator+(const LargeStruct&) {
+    return *this;
+  }
+};
+LargeStruct & extracted(LargeStruct &LS3, LargeStruct &LS4, LargeStruct &LS5) {
+return LS3 + LS4.get() + LS5;
+}
+void wrapperFun() {
+  LargeStruct LS1, LS2, LS3, LS4, LS5;
+  auto& R{LS1 + LS2.get() + extracted(LS3, LS4, LS5)};
+})cpp"},
+      // Boolean predicate as subexpression
+      {
+          R"cpp(
+void wrapperFun() {
+  int a{1}, b{2};
+  auto R{a > 1 ? [[b <= 0]] : false};
+}
+      )cpp",
+          R"cpp(
+bool extracted(int &b) {
+return b <= 0;
+}
+void wrapperFun() {
+  int a{1}, b{2};
+  auto R{a > 1 ? extracted(b) : false};
+}
+      )cpp"},
+      // Collects deeply nested arguments, left-aligned
+      {
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{[[fw(fw(fw(a))) + fw(fw(add(b, c))) + fw(fw(fw(add(d, e))))]] + fw(fw(f))};
+}
+      )cpp",
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+int extracted(int &a, int &b, int &c, int &d, int &e) {
+return fw(fw(fw(a))) + fw(fw(add(b, c))) + fw(fw(fw(add(d, e))));
+}
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{extracted(a, b, c, d, e) + fw(fw(f))};
+}
+      )cpp"},
+      // Collects deeply nested arguments, middle-aligned
+      {
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{fw(fw(fw(a))) + [[fw(fw(add(b, c))) + fw(fw(fw(add(d, e))))]] + fw(fw(f))};
+}
+      )cpp",
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+int extracted(int &b, int &c, int &d, int &e) {
+return fw(fw(add(b, c))) + fw(fw(fw(add(d, e))));
+}
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{fw(fw(fw(a))) + extracted(b, c, d, e) + fw(fw(f))};
+}
+      )cpp"},
+      // Collects deeply nested arguments, right-aligned
+      {
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{fw(fw(fw(a))) + [[fw(fw(add(b, c))) + fw(fw(fw(add(d, e)))) + fw(fw(f))]]};
+}
+      )cpp",
+          R"cpp(
+int fw(int a) { return a; };
+int add(int a, int b) { return a + b; }
+int extracted(int &b, int &c, int &d, int &e, int &f) {
+return fw(fw(add(b, c))) + fw(fw(fw(add(d, e)))) + fw(fw(f));
+}
+void wrapper() {
+    int a{0}, b{1}, c{2}, d{3}, e{4}, f{5};
+    int r{fw(fw(fw(a))) + extracted(b, c, d, e, f)};
+}
+      )cpp"},
+      // FIXME: Support macros: In this case the most-LHS is not omitted!
+      {R"cpp(
+#define ECHO(X) X
+void f() {
+    int x = 1 + [[ECHO(2 + 3) + 4]] + 5;
+})cpp",
+       R"cpp(
+#define ECHO(X) X
+int extracted() {
+return 1 + ECHO(2 + 3) + 4;
+}
+void f() {
+    int x = extracted() + 5;
+})cpp"},
+  };
+
+  for (const auto &[Input, Output] : InputOutputs) {
+    EXPECT_EQ(Output, apply(Input)) << Input;
+  }
+}
+
+TEST_F(ExtractFunctionTest, ExpressionsInMethodsSingleFile) {
+  // TODO: unavailable
+  // TODO: available
+
+  std::vector<std::pair<std::string, std::string>> InputOutputs{
+      // Expression: Does not capture members as parameters
+      // FIXME: If selected area does mutate members, make extracted() const
+      {R"cpp(
+struct S {
+void f() const {
+    int a{1}, b{2};
+    auto r{[[a + b + mem1 + mem2]]};
+}
+int mem1{0}, mem2{0};
+};
+)cpp",
+       R"cpp(
+struct S {
+int extracted(int &a, int &b) const {
+return a + b + mem1 + mem2;
+}
+void f() const {
+    int a{1}, b{2};
+    auto r{extracted(a, b)};
+}
+int mem1{0}, mem2{0};
+};
+)cpp"},
+      // Subexpression: Does not capture members as parameters
+      {R"cpp(
+struct S {
+void f() const {
+    int a{1}, b{2};
+    auto r{a + [[mem1 + mem2 + b + mem1]] + mem2};
+}
+int mem1{0}, mem2{0};
+};
+)cpp",
+       R"cpp(
+struct S {
+int extracted(int &b) const {
+return mem1 + mem2 + b + mem1;
+}
+void f() const {
+    int a{1}, b{2};
+    auto r{a + extracted(b) + mem2};
+}
+int mem1{0}, mem2{0};
+};
+)cpp"},
+  };
+
+  for (const auto &[Input, Output] : InputOutputs) {
+    EXPECT_EQ(Output, apply(Input)) << Input;
+  }
+}
+
+TEST_F(ExtractFunctionTest, ExpressionInMethodMultiFile) {
+  Header = R"cpp(
+    class SomeClass {
+      void f();
+      int mem1{0}, mem2{0};
+    };
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+    void SomeClass::f() {
+      int a{1}, b{2};
+      int x = [[a + mem1 + b + mem2]];
+    }
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+    int SomeClass::extracted(int &a, int &b) {
+return a + mem1 + b + mem2;
+}
+void SomeClass::f() {
+      int a{1}, b{2};
+      int x = extracted(a, b);
+    }
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+    class SomeClass {
+      int extracted(int &a, int &b);
+void f();
+      int mem1{0}, mem2{0};
+    };
+  )cpp";
+
+  llvm::StringMap<std::string> EditedFiles;
+
+  EXPECT_EQ(apply(OutOfLineSource, &EditedFiles), OutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, HeaderOutputCheck);
+}
+
+TEST_F(ExtractFunctionTest, SubexpressionInMethodMultiFile) {
+  Header = R"cpp(
+    class SomeClass {
+      void f();
+      int mem1{0}, mem2{0};
+    };
+  )cpp";
+
+  std::string OutOfLineSource = R"cpp(
+    void SomeClass::f() {
+      int a{1}, b{2};
+      int x = a + [[mem1 + b + mem2]] + mem1;
+    }
+  )cpp";
+
+  std::string OutOfLineSourceOutputCheck = R"cpp(
+    int SomeClass::extracted(int &b) {
+return mem1 + b + mem2;
+}
+void SomeClass::f() {
+      int a{1}, b{2};
+      int x = a + extracted(b) + mem1;
+    }
+  )cpp";
+
+  std::string HeaderOutputCheck = R"cpp(
+    class SomeClass {
+      int extracted(int &b);
+void f();
+      int mem1{0}, mem2{0};
+    };
+  )cpp";
+
+  llvm::StringMap<std::string> EditedFiles;
+
+  EXPECT_EQ(apply(OutOfLineSource, &EditedFiles), OutOfLineSourceOutputCheck);
+  EXPECT_EQ(EditedFiles.begin()->second, HeaderOutputCheck);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang