[PATCH] D68182: [Clangd] Ensure children are always RootStmt in ExtractFunction (Fixes #153)

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 222499.
SureYeaah added a comment.

Moved Unittest


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68182/new/

https://reviews.llvm.org/D68182

Files:
  clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp


Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -554,7 +554,7 @@
   EXPECT_THAT(apply(" [[int a = 5;]] a++; "), StartsWith("fail"));
   // Don't extract return
   EXPECT_THAT(apply(" if(true) [[return;]] "), StartsWith("fail"));
-  
+
 }
 
 TEST_F(ExtractFunctionTest, FileTest) {
@@ -631,6 +631,9 @@
     F ([[int x = 0;]])
   )cpp";
   EXPECT_EQ(apply(MacroFailInput), "unavailable");
+
+  // Shouldn't crash.
+  EXPECT_EQ(apply("void f([[int a]]);"), "unavailable");
 }
 
 TEST_F(ExtractFunctionTest, ControlFlow) {
Index: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -111,26 +111,29 @@
 const Node *getParentOfRootStmts(const Node *CommonAnc) {
   if (!CommonAnc)
     return nullptr;
+  const Node* Parent = CommonAnc;
   switch (CommonAnc->Selected) {
-  case SelectionTree::Selection::Unselected:
-    // Ensure all Children are RootStmts.
-    return llvm::all_of(CommonAnc->Children, isRootStmt) ? CommonAnc : nullptr;
-  case SelectionTree::Selection::Partial:
-    // Treat Partially selected VarDecl as completely selected since
-    // SelectionTree doesn't always select VarDecls correctly.
-    // FIXME: Remove this after D66872 is upstream)
-    if (!CommonAnc->ASTNode.get<VarDecl>())
-      return nullptr;
-    LLVM_FALLTHROUGH;
-  case SelectionTree::Selection::Complete:
-    // If the Common Ancestor is completely selected, then it's a root statement
-    // and its parent will be unselected.
-    const Node *Parent = CommonAnc->Parent;
-    // If parent is a DeclStmt, even though it's unselected, we consider it a
-    // root statement and return its parent. This is done because the VarDecls
-    // claim the entire selection range of the Declaration and DeclStmt is
-    // always unselected.
-    return Parent->ASTNode.get<DeclStmt>() ? Parent->Parent : Parent;
+    case SelectionTree::Selection::Partial:
+      // Treat Partially selected VarDecl as completely selected since
+      // SelectionTree doesn't always select VarDecls correctly.
+      // FIXME: Remove this after D66872 is upstream)
+      if (!CommonAnc->ASTNode.get<VarDecl>())
+        return nullptr;
+      LLVM_FALLTHROUGH;
+    case SelectionTree::Selection::Complete:
+      // If the Common Ancestor is completely selected, then it's a root statement
+      // and its parent will be unselected.
+      Parent = CommonAnc->Parent;
+      // If parent is a DeclStmt, even though it's unselected, we consider it a
+      // root statement and return its parent. This is done because the VarDecls
+      // claim the entire selection range of the Declaration and DeclStmt is
+      // always unselected.
+      if(Parent->ASTNode.get<DeclStmt>())
+        Parent = Parent->Parent;
+      LLVM_FALLTHROUGH;
+    case SelectionTree::Selection::Unselected:
+      // Ensure all Children are RootStmts.
+      return llvm::all_of(Parent->Children, isRootStmt) ? Parent : nullptr;
   }
   llvm_unreachable("Unhandled SelectionTree::Selection enum");
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D68182.222499.patch
Type: text/x-patch
Size: 3360 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190930/080ddb48/attachment-0001.bin>