https://github.com/ckandeler created
https://github.com/llvm/llvm-project/pull/225434
Selecting just the initializer of a variable declaration, e.g.
`auto A = [[func()]];`, was incorrectly offered for extraction,
producing a void-returning function called where a value is required:
```cpp
void extracted() { func(); }
void f() { auto A = extracted(); }
```
This doesn't compile (auto can't deduce from void; a fixed type would
likewise fail to convert from void).
`getEnclosingStmt()` ascends from the selected node to what should be
its enclosing statement, rejecting the case where that turns out to be
an `Expr` (meaning the selection is a subexpression of a larger
expression, not a genuine statement). A `VarDecl`'s initializer isn't
caught by that check, though: ascending from it lands on the `VarDecl`
itself, which is a `Decl`, not a `Stmt` or `Expr`. Broaden the check to
reject unless the ascended-to parent is a `Stmt` (note `Expr` is itself
a `Stmt` subtype in Clang's AST, so this still needs the separate
`Expr` check to catch the original case).
Assisted-by: Claude
>From 9e3a99410c4d70af62866b1a5bdf98bf906410d7 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <[email protected]>
Date: Tue, 22 Sep 2026 17:34:12 +0200
Subject: [PATCH] [clangd] Don't offer Extract to Function for a VarDecl's
initializer
Selecting just the initializer of a variable declaration, e.g.
`auto A = [[func()]];`, was incorrectly offered for extraction,
producing a void-returning function called where a value is required:
void extracted() { func(); }
void f() { auto A = extracted(); }
This doesn't compile (auto can't deduce from void; a fixed type would
likewise fail to convert from void).
getEnclosingStmt() ascends from the selected node to what should be
its enclosing statement, rejecting the case where that turns out to be
an Expr (meaning the selection is a subexpression of a larger
expression, not a genuine statement). A VarDecl's initializer isn't
caught by that check, though: ascending from it lands on the VarDecl
itself, which is a Decl, not a Stmt or Expr. Broaden the check to
reject unless the ascended-to parent is a Stmt (note Expr is itself a
Stmt subtype in Clang's AST, so this still needs the separate Expr
check to catch the original case).
Assisted-by: Claude
---
.../refactor/tweaks/ExtractFunction.cpp | 16 ++++++----
.../unittests/tweaks/ExtractFunctionTests.cpp | 29 +++++++++++++++++++
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index a68b48536a22b..eb7a9faa65a81 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -170,15 +170,21 @@ bool isRootStmt(const Node *N) {
// VarDecls claim the entire selection range of the Declaration and DeclStmt
// is always unselected.
//
-// Returns null if the (possibly DeclStmt-adjusted) parent is an Expr: this
-// means Child is merely a subexpression of a larger expression rather than
-// a genuine standalone statement, e.g. selecting just the "3" in
-// `stream << 3;`, and extracting it would produce broken code.
+// Returns null if the (possibly DeclStmt-adjusted) parent isn't a "plain"
+// Stmt, or is an Expr: this means Child is merely a subexpression of a
+// larger expression or declaration rather than a genuine standalone
+// statement, and extracting it would produce broken code. This covers two
+// distinct cases:
+// - Parent is an Expr, e.g. selecting just the "3" in `stream << 3;`
+// (Child is a subexpression of a larger expression).
+// - Parent isn't a Stmt at all, e.g. selecting just the "func()" in
+// `auto A = func();` (Child is a VarDecl's initializer, so Parent is
+// that VarDecl -- a Decl, not a Stmt).
const Node *getEnclosingStmt(const Node *Child) {
const Node *Parent = Child->Parent;
if (Parent->ASTNode.get<DeclStmt>())
Parent = Parent->Parent;
- if (Parent->ASTNode.get<Expr>())
+ if (!Parent->ASTNode.get<Stmt>() || Parent->ASTNode.get<Expr>())
return nullptr;
return Parent;
}
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 02e99b45cc263..c55fc17ae83e9 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -748,6 +748,35 @@ TEST_F(ExtractFunctionTest, RangeBasedFor) {
HasSubstr("extracted"));
}
+TEST_F(ExtractFunctionTest, VarDeclInitializer) {
+ Context = File;
+ // The initializer of a variable declaration is not a discardable
+ // statement either: its value is required to initialize the variable, so
+ // replacing it with a call to a void-returning extracted function would
+ // not compile. Ascending from the initializer lands on the VarDecl itself (a
+ // Decl, not a Stmt or Expr), which the general "is this a genuine statement"
+ // check must also reject.
+ EXPECT_EQ(apply(R"cpp(
+ int func();
+ void f() { auto A = [[func()]]; }
+ )cpp"),
+ "unavailable");
+ // Same without `auto`: the type doesn't matter, only that a value is
+ // required.
+ EXPECT_EQ(apply(R"cpp(
+ int func();
+ void f() { int A = [[func()]]; }
+ )cpp"),
+ "unavailable");
+ // Sanity check: extracting the whole declaration statement (as opposed to
+ // just its initializer) is unaffected.
+ EXPECT_THAT(apply(R"cpp(
+ int func();
+ void f() { [[int A = func();]] }
+ )cpp"),
+ HasSubstr("extracted"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
