[clangd] Extract Function: add hoisting support

[5chmidti]

Summary

Adds support to hoist variables declared inside the selected region
and used afterwards back out of the extraced function for later use.
Uses the explicit variable type if only one decl needs hoisting,
otherwise pair or tuple with auto return type deduction
(requires c++14) and a structured binding (requires c++17) or
explicitly unpacking the variables with get<>.

[5chmidti]

Fixup: rm added includes

[5chmidti]

Fix windows build by setting the c++ standard for the hoisting tests explicitly to c++17.

[5chmidti]

Ping

[5chmidti]

Ping

[5chmidti]

Ping

[5chmidti]

Ping, and:

• change find_if of post-use detection to range-for with a filtered range
• make renderHoistedCall a private member of NewFunction
• remove the Render lambda in renderHoistSet in favor of putting the body into the loop, improving readability imo.
• swapped order in tertiary operator to remove the ! in the condition for better readability
• add abort condtition if any of the NamedDecls to be hoisted is a TypeDecl. The HoistedSet can contain TypeDecls, which block the extraction because types cannot be hoisted as easily as variables. Hoisting types could be done by moving the declaration into the scope that the functions are declared in (global, namespace or records). I could take a look at this if there is interest, but I think that should be its own diff.
• add tests about full and partial selection of type aliases and classes

Open questions:

• I think that <tuple>/<utility> should be included if a tuple or pair is used, but couldn't figure out a clean way to include the headers. It looks like the way to go would be through IncludeCleaners insert function, but for that I woul dneed to construct the IncludeInserter. I don't think I can get the FormatStyle or BuildDir from inside of a tweak. Does anyone have an idea or is this a non-issue?
• The tweak is unavailable if a tuple or pair is required for the hoisting, but no auto return-type deduction is available. This is the case because I implemented the return type of the hoisted variables to use tuple or pair if two or more variables are returned, but to keep the return type short, I set the return type in these cases to auto. Should the return type be fully spelled out? Should it be spelled out always or should there be a config option (seems a bit overkill)?
• I'm not a fan of the const& HoistSet member that I used. Should I change this to a pointer (its always non-null)?

[5chmidti]

I found a new issue. I'll probably fix it by next weekend.

[5chmidti]

Never mind, the problem is unrelated to this patch. Filed clangd issue #1710

[5chmidti]

Ping

[5chmidti]

Ping. Because Phabricator is set to be made read-only on the 1. of October, it might be better to not review this patch here and instead open it on GitHub instead. Thoughts?

[5chmidti]

Ping

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

 // 4. CapturedZoneInfo uses a RecursiveASTVisitor to capture information about
 //    the extraction like declarations, existing return statements, etc.
 // 5. getExtractedFunction is responsible for analyzing the CapturedZoneInfo and
 //    creating a NewFunction.
 //===----------------------------------------------------------------------===//
 
 #include "AST.h"
 #include "FindTarget.h"
+#include "Headers.h"
 #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 <optional>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 using Node = SelectionTree::Node;
 
+struct HoistSetComparator {
+  bool operator()(const Decl *const Lhs, const Decl *const Rhs) const {
+    return Lhs->getLocation() < Rhs->getLocation();
+  }
+};
+using HoistSet = llvm::SmallSet<const NamedDecl *, 1, HoistSetComparator>;
+
 // 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.
struct ExtractionZone {
   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(); }
 
-  // Checks if declarations inside extraction zone are accessed afterwards.
+  // Checks if declarations inside extraction zone are accessed afterwards and
+  // adds these declarations to the returned set.
   //
   // This performs a partial AST traversal proportional to the size of the
   // enclosing function, so it is possibly expensive.
-  bool requiresHoisting(const SourceManager &SM,
+  HoistSet getDeclsToHoist(const SourceManager &SM,
                            const HeuristicResolver *Resolver) const {
     // First find all the declarations that happened inside extraction zone.
     llvm::SmallSet<const Decl *, 1> DeclsInExtZone;
     for (auto *RootStmt : RootStmts) {
       findExplicitReferences(
           RootStmt,
           [&DeclsInExtZone](const ReferenceLoc &Loc) {
             if (!Loc.IsDecl)
               return;
             DeclsInExtZone.insert(Loc.Targets.front());
           },
           Resolver);
     }
     // Early exit without performing expensive traversal below.
     if (DeclsInExtZone.empty())
-      return false;
-    // Then make sure they are not used outside the zone.
+      return {};
+    // Add any decl used after the selection to the returned set
+    HoistSet DeclsToHoist{};
     for (const auto *S : EnclosingFunction->getBody()->children()) {
       if (SM.isBeforeInTranslationUnit(S->getSourceRange().getEnd(),
                                        ZoneRange.getEnd()))
         continue;
-      bool HasPostUse = false;
       findExplicitReferences(
           S,
           [&](const ReferenceLoc &Loc) {
-            if (HasPostUse ||
-                SM.isBeforeInTranslationUnit(Loc.NameLoc, ZoneRange.getEnd()))
+            if (SM.isBeforeInTranslationUnit(Loc.NameLoc, ZoneRange.getEnd()))
               return;
-            HasPostUse = llvm::any_of(Loc.Targets,
-                                      [&DeclsInExtZone](const Decl *Target) {
-                                        return DeclsInExtZone.contains(Target);
-                                      });
+            for (const NamedDecl *const PostUse : llvm::make_filter_range(
+                     Loc.Targets, [&DeclsInExtZone](const Decl *Target) {
+                     })) {
+              DeclsToHoist.insert(PostUse);
+            }
           },
           Resolver);
-      if (HasPostUse)
-        return true;
     }
-    return false;
+    return DeclsToHoist;
   }
 };
 
 // Whether the code in the extraction zone is guaranteed to return, assuming
 // no broken control flow (unbound break/continue).
 // This is a very naive check (does it end with a return stmt).
 // Doing some rudimentary control flow analysis would cover more cases.
 bool alwaysReturns(const ExtractionZone &EZ) {
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;
+  const HoistSet &ToHoist;
 
   // 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,
+  NewFunction(const HoistSet &ToHoist,
+              tooling::ExtractionSemicolonPolicy SemicolonPolicy,
               const LangOptions *LangOpts)
-      : SemicolonPolicy(SemicolonPolicy), LangOpts(LangOpts) {}
+      : ToHoist(ToHoist), SemicolonPolicy(SemicolonPolicy), LangOpts(LangOpts) {
+  }
   // Render the call for this function.
   std::string renderCall() const;
   // Render the definition for this function.
   std::string renderDeclaration(FunctionDeclKind K,
                                 const DeclContext &SemanticDC,
                                 const DeclContext &SyntacticDC,
                                 const SourceManager &SM) const;
 
 private:
   std::string
   renderParametersForDeclaration(const DeclContext &Enclosing) const;
   std::string renderParametersForCall() const;
   std::string renderSpecifiers(FunctionDeclKind K) const;
   std::string renderQualifiers() const;
   std::string renderDeclarationName(FunctionDeclKind K) const;
+  std::string renderHoistedCall() const;
   // Generate the function body.
   std::string getFuncBody(const SourceManager &SM) const;
 };
 
 std::string NewFunction::renderParametersForDeclaration(
     const DeclContext &Enclosing) const {
   std::string Result;
   bool NeedCommaBefore = false;
std::string NewFunction::renderDeclarationName(FunctionDeclKind K) const {
   }
 
   std::string QualifierName;
   llvm::raw_string_ostream Oss(QualifierName);
   DefinitionQualifier->print(Oss, *LangOpts);
   return llvm::formatv("{0}{1}", QualifierName, Name);
 }
 
+// Renders the HoistSet to a comma separated list or a single named decl.
+std::string renderHoistSet(const HoistSet &ToHoist) {
+  std::string Res{};
+  bool NeedsComma = false;
+
+  for (const NamedDecl *DeclToHoist : ToHoist) {
+    if (llvm::isa<VarDecl>(DeclToHoist) ||
+        llvm::isa<BindingDecl>(DeclToHoist)) {
+      if (NeedsComma) {
+        Res += ", ";
+      }
+      Res += DeclToHoist->getNameAsString();
+      NeedsComma = true;
+    }
+  }
+  return Res;
+}
+
+std::string NewFunction::renderHoistedCall() const {
+  auto HoistedVarDecls = std::string{};
+  auto ExplicitUnpacking = std::string{};
+  const auto HasStructuredBinding = LangOpts->CPlusPlus17;
+
+  if (ToHoist.size() > 1) {
+    if (HasStructuredBinding) {
+      HoistedVarDecls = "auto [" + renderHoistSet(ToHoist) + "] = ";
+    } else {
+      HoistedVarDecls = "auto returned = ";
+      auto DeclIter = ToHoist.begin();
+      for (size_t Index = 0U; Index < ToHoist.size(); ++Index, ++DeclIter) {
+        ExplicitUnpacking +=
+            llvm::formatv("\nauto {0} = std::get<{1}>(returned);",
+                          (*DeclIter)->getNameAsString(), Index);
+      }
+    }
+  } else {
+    HoistedVarDecls = "auto " + renderHoistSet(ToHoist) + " = ";
+  }
+
+  return llvm::formatv(
+      "{0}{1}({2}){3}{4}", HoistedVarDecls, Name, renderParametersForCall(),
+      (SemicolonPolicy.isNeededInOriginalFunction() ? ";" : ""),
+      ExplicitUnpacking);
+}
+
 std::string NewFunction::renderCall() const {
+  if (!ToHoist.empty())
+    return renderHoistedCall();
+
   return std::string(
       llvm::formatv("{0}{1}({2}){3}", CallerReturnsValue ? "return " : "", Name,
                     renderParametersForCall(),
                     (SemicolonPolicy.isNeededInOriginalFunction() ? ";" : "")));
 }
 
 std::string NewFunction::renderDeclaration(FunctionDeclKind K,
                                            const DeclContext &SemanticDC,
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() +
+  auto Body = toSourceCode(SM, BodyRange).str() +
               (SemicolonPolicy.isNeededInExtractedFunction() ? ";" : "");
+
+  if (ToHoist.empty())
+    return Body;
+
+  if (const bool NeedsTupleOrPair = ToHoist.size() > 1; NeedsTupleOrPair) {
+    const auto NeedsPair = ToHoist.size() == 2;
+
+    Body += "\nreturn " +
+            std::string(NeedsPair ? "std::pair{" : "std::tuple{") +
+            renderHoistSet(ToHoist) + "};";
+  } else {
+    Body += "\nreturn " + renderHoistSet(ToHoist) + ";";
+  }
+  return Body;
 }
 
 std::string NewFunction::Parameter::render(const DeclContext *Context) const {
   return printType(TypeInfo, *Context) + (PassByReference ? " &" : " ") + Name;
 }
 
 // Stores captured information about Extraction Zone.
 struct CapturedZoneInfo {

 // needed.
 // FIXME: Check if the declaration has a local/anonymous type
 bool createParameters(NewFunction &ExtractedFunc,
                       const CapturedZoneInfo &CapturedInfo) {
   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;
     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
getSemicolonPolicy(ExtractionZone &ExtZone, const SourceManager &SM,
   auto SemicolonPolicy = tooling::ExtractionSemicolonPolicy::compute(
       ExtZone.getLastRootStmt()->ASTNode.get<Stmt>(), FuncBodyRange, SM,
       LangOpts);
   // Update ZoneRange.
   ExtZone.ZoneRange.setEnd(FuncBodyRange.getEnd().getLocWithOffset(1));
   return SemicolonPolicy;
 }
 
+QualType getReturnTypeForHoisted(const FunctionDecl &EnclosingFunc,
+                                 const HoistSet &ToHoist) {
+  // Hoisting just one variable, use that variables type instead of auto
+  if (ToHoist.size() == 1) {
+    if (const auto *const VDecl = llvm::dyn_cast<VarDecl>(*ToHoist.begin());
+        VDecl != nullptr) {
+      return VDecl->getType();
+    }
+  }
+
+  return EnclosingFunc.getParentASTContext().getAutoDeductType();
+}
+
 // Generate return type for ExtractedFunc. Return false if unable to do so.
 bool generateReturnProperties(NewFunction &ExtractedFunc,
                               const FunctionDecl &EnclosingFunc,
                               const CapturedZoneInfo &CapturedInfo) {
   // 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).
   if (CapturedInfo.HasReturnStmt) {
     // If the return is conditional, neither replacing the code with
     // `extracted()` nor `return extracted()` is correct.
     if (!CapturedInfo.AlwaysReturns)
       return false;
     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.
     if (Ret->isDependentType())
       return false;
     ExtractedFunc.ReturnType = Ret;
     return true;
   }
   // FIXME: Generate new return statement if needed.
-  ExtractedFunc.ReturnType = EnclosingFunc.getParentASTContext().VoidTy;
+  ExtractedFunc.ReturnType =
+      ExtractedFunc.ToHoist.empty()
+          ? EnclosingFunc.getParentASTContext().VoidTy
+          : getReturnTypeForHoisted(EnclosingFunc, ExtractedFunc.ToHoist);
+
   return true;
 }
 
 void captureMethodInfo(NewFunction &ExtractedFunc,
                        const CXXMethodDecl *Method) {
   ExtractedFunc.Static = Method->isStatic();
   ExtractedFunc.Const = Method->isConst();
   ExtractedFunc.EnclosingClass = Method->getParent();
 }
 
 // FIXME: add support for adding other function return types besides void.
 // FIXME: assign the value returned by non void extracted function.
 llvm::Expected<NewFunction> getExtractedFunction(ExtractionZone &ExtZone,
+                                                 const HoistSet &ToHoist,
                                                  const SourceManager &SM,
                                                  const LangOptions &LangOpts) {
   CapturedZoneInfo CapturedInfo = captureZoneInfo(ExtZone);
   // Bail out if any break of continue exists
   if (CapturedInfo.BrokenControlFlow)
     return error("Cannot extract break/continue without corresponding "
                  "loop/switch statement.");
-  NewFunction ExtractedFunc(getSemicolonPolicy(ExtZone, SM, LangOpts),
+  NewFunction ExtractedFunc(ToHoist, getSemicolonPolicy(ExtZone, SM, LangOpts),
                             &LangOpts);
 
   ExtractedFunc.SyntacticDC =
       ExtZone.EnclosingFunction->getLexicalDeclContext();
   ExtractedFunc.SemanticDC = ExtZone.EnclosingFunction->getDeclContext();
   ExtractedFunc.DefinitionQualifier = ExtZone.EnclosingFunction->getQualifier();
   ExtractedFunc.Constexpr = ExtZone.EnclosingFunction->getConstexprKind();
 
public:
   Expected<Effect> apply(const Selection &Inputs) override;
   std::string title() const override { return "Extract to function"; }
   llvm::StringLiteral kind() const override {
     return CodeAction::REFACTOR_KIND;
   }
 
 private:
   ExtractionZone ExtZone;
+  HoistSet ToHoist;
 };
 
 REGISTER_TWEAK(ExtractFunction)
 tooling::Replacement replaceWithFuncCall(const NewFunction &ExtractedFunc,
                                          const SourceManager &SM,
                                          const LangOptions &LangOpts) {
   std::string FuncCall = ExtractedFunc.renderCall();
   return tooling::Replacement(
if (!LangOpts.CPlusPlus)
     return false;
   const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
   const SourceManager &SM = Inputs.AST->getSourceManager();
   auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts);
   if (!MaybeExtZone ||
       (hasReturnStmt(*MaybeExtZone) && !alwaysReturns(*MaybeExtZone)))
     return false;
 
-  // FIXME: Get rid of this check once we support hoisting.
-  if (MaybeExtZone->requiresHoisting(SM, Inputs.AST->getHeuristicResolver()))
+  ToHoist =
+      MaybeExtZone->getDeclsToHoist(SM, Inputs.AST->getHeuristicResolver());
+
+  // Cannot extract a selection that contains a type declaration that is used
+  // outside of the selected range
+  if (llvm::any_of(ToHoist, [](const NamedDecl *NDecl) {
+        return llvm::isa<TypeDecl>(NDecl);
+      }))
+    return false;
+
+  const auto HasAutoReturnTypeDeduction = LangOpts.CPlusPlus14;
+  const auto RequiresPairOrTuple = ToHoist.size() > 1;
+  if (RequiresPairOrTuple && !HasAutoReturnTypeDeduction)
     return false;
 
   ExtZone = std::move(*MaybeExtZone);
   return true;
 }
 
 Expected<Tweak::Effect> ExtractFunction::apply(const Selection &Inputs) {
   const SourceManager &SM = Inputs.AST->getSourceManager();
   const LangOptions &LangOpts = Inputs.AST->getLangOpts();
-  auto ExtractedFunc = getExtractedFunction(ExtZone, SM, LangOpts);
+  auto ExtractedFunc = getExtractedFunction(ExtZone, ToHoist, SM, LangOpts);
   // FIXME: Add more types of errors.
   if (!ExtractedFunc)
     return ExtractedFunc.takeError();
   tooling::Replacements Edit;
   if (auto Err = Edit.add(createFunctionDefinition(*ExtractedFunc, SM)))
     return std::move(Err);
   if (auto Err = Edit.add(replaceWithFuncCall(*ExtractedFunc, SM, LangOpts)))
     return std::move(Err);
 
   if (auto FwdLoc = ExtractedFunc->ForwardDeclarationPoint) {
     // If the fwd-declaration goes in the same file, merge into Replacements.
     // Otherwise it needs to be a separate file edit.
     if (SM.isWrittenInSameFile(ExtractedFunc->DefinitionPoint, *FwdLoc)) {
       if (auto Err = Edit.add(createForwardDeclaration(*ExtractedFunc, SM)))
         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/unittests/tweaks/ExtractFunctionTests.cpp

TEST_F(ExtractFunctionTest, FunctionTest) {
   // 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");
+
+  // Extract regions that require hoisting
+  EXPECT_THAT(apply(" [[int a = 5;]] a++; "), HasSubstr("extracted"));
 
   // Ensure that end of Zone and Beginning of PostZone being adjacent doesn't
   // lead to break being included in the extraction zone.
   EXPECT_THAT(apply("for(;;) { [[int x;]]break; }"), HasSubstr("extracted"));
   // FIXME: ExtractFunction should be unavailable inside loop construct
   // initializer/condition.
   EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("extracted"));
   // Extract certain return
F (extracted();)
   std::string CompoundFailInput = R"cpp(
     void f() [[{
       int a;
     }]]
   )cpp";
   EXPECT_EQ(apply(CompoundFailInput), "unavailable");
 }
 
+TEST_F(ExtractFunctionTest, Hoisting) {
+  ExtraArgs.emplace_back("-std=c++17");
+  std::string HoistingInput = R"cpp(
+    int foo() {
+      int a = 3;
+      [[int x = 39 + a;
+      ++x;
+      int y = x * 2;
+      int z = 4;]]
+      return x + y + z;
+    }
+  )cpp";
+  std::string HoistingOutput = R"cpp(
+    auto extracted(int &a) {
+int x = 39 + a;
+      ++x;
+      int y = x * 2;
+      int z = 4;
+return std::tuple{x, y, z};
+}
+int foo() {
+      int a = 3;
+      auto [x, y, z] = extracted(a);
+      return x + y + z;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput), HoistingOutput);
+
+  std::string HoistingInput2 = R"cpp(
+    int foo() {
+      int a{};
+      [[int b = a + 1;]]
+      return b;
+    }
+  )cpp";
+  std::string HoistingOutput2 = R"cpp(
+    int extracted(int &a) {
+int b = a + 1;
+return b;
+}
+int foo() {
+      int a{};
+      auto b = extracted(a);
+      return b;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput2), HoistingOutput2);
+
+  std::string HoistingInput3 = R"cpp(
+    int foo(int b) {
+      int a{};
+      if (b == 42) {
+        [[a = 123;
+        return a + b;]]
+      }
+      a = 456;
+      return a;
+    }
+  )cpp";
+  std::string HoistingOutput3 = R"cpp(
+    int extracted(int &b, int &a) {
+a = 123;
+        return a + b;
+}
+int foo(int b) {
+      int a{};
+      if (b == 42) {
+        return extracted(b, a);
+      }
+      a = 456;
+      return a;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput3), HoistingOutput3);
+
+  std::string HoistingInput4 = R"cpp(
+    struct A {
+      bool flag;
+      int val;
+    };
+    A bar();
+    int foo(int b) {
+      int a = 0;
+      [[auto [flag, val] = bar();
+      int c = 4;
+      val = c + a;]]
+      return a + b + c + val;
+    }
+  )cpp";
+  std::string HoistingOutput4 = R"cpp(
+    struct A {
+      bool flag;
+      int val;
+    };
+    A bar();
+    auto extracted(int &a) {
+auto [flag, val] = bar();
+      int c = 4;
+      val = c + a;
+return std::pair{val, c};
+}
+int foo(int b) {
+      int a = 0;
+      auto [val, c] = extracted(a);
+      return a + b + c + val;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput4), HoistingOutput4);
+
+  // Cannot extract a selection that contains a type declaration that is used
+  // outside of the selected range
+  EXPECT_THAT(apply(R"cpp(
+      [[using MyType = int;]]
+      MyType x = 42;
+      MyType y = x;
+    )cpp"),
+              "unavailable");
+  EXPECT_THAT(apply(R"cpp(
+      [[using MyType = int;
+      MyType x = 42;]]
+      MyType y = x;
+    )cpp"),
+              "unavailable");
+  EXPECT_THAT(apply(R"cpp(
+      [[struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};]]
+      auto Z = Bar{Y};
+    )cpp"),
+              "unavailable");
+
+  // Check that selections containing type declarations can be extracted if
+  // there are no uses of the type after the selection
+  std::string FullTypeAliasInput = R"cpp(
+    void foo() {
+      [[using MyType = int;
+      MyType x = 42;
+      MyType y = x;]]
+    }
+    )cpp";
+  std::string FullTypeAliasOutput = R"cpp(
+    void extracted() {
+using MyType = int;
+      MyType x = 42;
+      MyType y = x;
+}
+void foo() {
+      extracted();
+    }
+    )cpp";
+  EXPECT_EQ(apply(FullTypeAliasInput), FullTypeAliasOutput);
+
+  std::string FullStructInput = R"cpp(
+    int foo() {
+      [[struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};
+      auto Z = Bar{Y};
+      return 42;]]
+    }
+    )cpp";
+  std::string FullStructOutput = R"cpp(
+    int extracted() {
+struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};
+      auto Z = Bar{Y};
+      return 42;
+}
+int foo() {
+      return extracted();
+    }
+    )cpp";
+  EXPECT_EQ(apply(FullStructInput), FullStructOutput);
+
+  std::string ReturnTypeIsAliasedInput = R"cpp(
+    int foo() {
+      [[struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};
+      auto Z = Bar{Y};
+      using MyInt = int;
+      MyInt A = 42;
+      return A;]]
+    }
+    )cpp";
+  std::string ReturnTypeIsAliasedOutput = R"cpp(
+    int extracted() {
+struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};
+      auto Z = Bar{Y};
+      using MyInt = int;
+      MyInt A = 42;
+      return A;
+}
+int foo() {
+      return extracted();
+    }
+    )cpp";
+  EXPECT_EQ(apply(ReturnTypeIsAliasedInput), ReturnTypeIsAliasedOutput);
+
+  EXPECT_THAT(apply(R"cpp(
+      [[struct Bar {
+        int X;
+      };
+      auto Y = Bar{42};]]
+      auto Z = Bar{Y};
+    )cpp"),
+              "unavailable");
+}
+
+TEST_F(ExtractFunctionTest, HoistingCXX11) {
+  ExtraArgs.emplace_back("-std=c++11");
+  std::string HoistingInput = R"cpp(
+    int foo() {
+      int a = 3;
+      [[int x = 39 + a;
+      ++x;
+      int y = x * 2;
+      int z = 4;]]
+      return x + y + z;
+    }
+  )cpp";
+  EXPECT_THAT(apply(HoistingInput), HasSubstr("unavailable"));
+
+  std::string HoistingInput2 = R"cpp(
+    int foo() {
+      int a;
+      [[int b = a + 1;]]
+      return b;
+    }
+  )cpp";
+  std::string HoistingOutput2 = R"cpp(
+    int extracted(int &a) {
+int b = a + 1;
+return b;
+}
+int foo() {
+      int a;
+      auto b = extracted(a);
+      return b;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput2), HoistingOutput2);
+}
+
+TEST_F(ExtractFunctionTest, HoistingCXX14) {
+  ExtraArgs.emplace_back("-std=c++14");
+  std::string HoistingInput = R"cpp(
+    int foo() {
+      int a = 3;
+      [[int x = 39 + a;
+      ++x;
+      int y = x * 2;
+      int z = 4;]]
+      return x + y + z;
+    }
+  )cpp";
+  std::string HoistingOutput = R"cpp(
+    auto extracted(int &a) {
+int x = 39 + a;
+      ++x;
+      int y = x * 2;
+      int z = 4;
+return std::tuple{x, y, z};
+}
+int foo() {
+      int a = 3;
+      auto returned = extracted(a);
+auto x = std::get<0>(returned);
+auto y = std::get<1>(returned);
+auto z = std::get<2>(returned);
+      return x + y + z;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput), HoistingOutput);
+
+  std::string HoistingInput2 = R"cpp(
+    int foo() {
+      int a;
+      [[int b = a + 1;]]
+      return b;
+    }
+  )cpp";
+  std::string HoistingOutput2 = R"cpp(
+    int extracted(int &a) {
+int b = a + 1;
+return b;
+}
+int foo() {
+      int a;
+      auto b = extracted(a);
+      return b;
+    }
+  )cpp";
+  EXPECT_EQ(apply(HoistingInput2), HoistingOutput2);
+}
+
 TEST_F(ExtractFunctionTest, DifferentHeaderSourceTest) {
   Header = R"cpp(
     class SomeClass {
       void f();
     };
   )cpp";
 
   std::string OutOfLineSource = R"cpp(

clang-tools-extra/docs/ReleaseNotes.rst

 ^^^^^^^^^^^^^^^^
 
 Objective-C
 ^^^^^^^^^^^
 
 Miscellaneous
 ^^^^^^^^^^^^^
 
+- The extract function tweak gained support for hoisting, i.e. returning decls declared
+  inside the selection that are used outside of the selection.
+
 Improvements to clang-doc
 -------------------------
 
 Improvements to clang-query
 ---------------------------
 
 The improvements are...