Author: sureyeaah Date: Wed Oct 2 06:51:06 2019 New Revision: 373471 URL: http://llvm.org/viewvc/llvm-project?rev=373471&view=rev Log: [Clangd] Ensure children are always RootStmt in ExtractFunction (Fixes #153)
Summary: We weren't always checking if children are RootStmts in ExtractFunction. For `void f([[int a]]);`, the ParmVarDecl appeared as a RootStmt since we didn't perform the check and ended up being casted to a (null) Stmt. Reviewers: sammccall, kadircet Subscribers: kristof.beyls, ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D68182 Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp Modified: clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp?rev=373471&r1=373470&r2=373471&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp (original) +++ clang-tools-extra/trunk/clangd/refactor/tweaks/ExtractFunction.cpp Wed Oct 2 06:51:06 2019 @@ -104,6 +104,11 @@ bool isRootStmt(const Node *N) { } // Returns the (unselected) parent of all RootStmts given the commonAncestor. +// Returns null if: +// 1. any node is partially selected +// 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 @@ -111,25 +116,30 @@ bool isRootStmt(const Node *N) { const Node *getParentOfRootStmts(const Node *CommonAnc) { if (!CommonAnc) return nullptr; + const Node *Parent = nullptr; switch (CommonAnc->Selected) { case SelectionTree::Selection::Unselected: // Typicaly a block, with the { and } unselected, could also be ForStmt etc // Ensure all Children are RootStmts. - return llvm::all_of(CommonAnc->Children, isRootStmt) ? CommonAnc : nullptr; + Parent = CommonAnc; + break; case SelectionTree::Selection::Partial: // Only a fully-selected single statement can be selected. return nullptr; 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; + 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; + if (Parent->ASTNode.get<DeclStmt>()) + Parent = Parent->Parent; + break; } - llvm_unreachable("Unhandled SelectionTree::Selection enum"); + // 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. Modified: clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp?rev=373471&r1=373470&r2=373471&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/TweakTests.cpp Wed Oct 2 06:51:06 2019 @@ -629,6 +629,9 @@ void f(const int c) { 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
