diff options
| -rw-r--r-- | clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp | 29 | ||||
| -rw-r--r-- | clang-tools-extra/clangd/unittests/TweakTests.cpp | 7 |
2 files changed, 28 insertions, 8 deletions
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp index 8c2403a70c7..1551f41a131 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp @@ -225,10 +225,24 @@ computeEnclosingFuncRange(const FunctionDecl *EnclosingFunction, 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() && + "We should always be extracting from a function body."); + if (Child->ASTNode.get<Stmt>() == EnclosingFunc->getBody()) + return false; + return true; +} + // FIXME: Check we're not extracting from the initializer/condition of a control // flow structure. -// FIXME: Check that we don't extract the compound statement of the -// enclosingFunction. llvm::Optional<ExtractionZone> findExtractionZone(const Node *CommonAnc, const SourceManager &SM, const LangOptions &LangOpts) { @@ -236,15 +250,14 @@ llvm::Optional<ExtractionZone> findExtractionZone(const Node *CommonAnc, ExtZone.Parent = getParentOfRootStmts(CommonAnc); if (!ExtZone.Parent || ExtZone.Parent->Children.empty()) return llvm::None; - // Don't extract expressions. - // FIXME: We should extract expressions that are "statements" i.e. not - // subexpressions - if (ExtZone.Parent->Children.size() == 1 && - ExtZone.getLastRootStmt()->ASTNode.get<Expr>()) - return llvm::None; ExtZone.EnclosingFunction = findEnclosingFunction(ExtZone.Parent); if (!ExtZone.EnclosingFunction) return llvm::None; + // 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)) + return llvm::None; if (auto FuncRange = computeEnclosingFuncRange(ExtZone.EnclosingFunction, SM, LangOpts)) ExtZone.EnclosingFuncRange = *FuncRange; diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp index 2c4aaa54468..97cd4a2cafa 100644 --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -632,6 +632,13 @@ void f(const int c) { // Shouldn't crash. EXPECT_EQ(apply("void f([[int a]]);"), "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"); } TEST_F(ExtractFunctionTest, ControlFlow) { |
