diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp +++ b/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()) - 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() ? 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()) + 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()) + 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"); } diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/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) {