SureYeaah created this revision.
SureYeaah added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
This patch disable extraction of the body of the enclosing function.
`void f() [[{}]]`
Extracting this CompoundStmt would leave the enclosing function without
a body.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D68245
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
@@ -631,6 +631,14 @@
F ([[int x = 0;]])
)cpp";
EXPECT_EQ(apply(MacroFailInput), "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) {
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
@@ -218,6 +218,21 @@
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.
+ if (const Stmt *S = Child->ASTNode.get<Stmt>())
+ if (EnclosingFunc->hasBody() && EnclosingFunc->getBody() == S)
+ 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
@@ -229,15 +244,14 @@
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;
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -631,6 +631,14 @@
F ([[int x = 0;]])
)cpp";
EXPECT_EQ(apply(MacroFailInput), "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) {
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
@@ -218,6 +218,21 @@
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.
+ if (const Stmt *S = Child->ASTNode.get<Stmt>())
+ if (EnclosingFunc->hasBody() && EnclosingFunc->getBody() == S)
+ 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
@@ -229,15 +244,14 @@
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
