[clang-tools-extra] [clangd] fix extract-to-function for overloaded operators (PR #81640)
Nathan Ridge via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 14 21:43:39 PST 2024
https://github.com/HighCommander4 updated https://github.com/llvm/llvm-project/pull/81640
>From 04687eb4fcc3cc424580c0a0df8044a8b17286f8 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Tue, 13 Feb 2024 18:59:16 +0100
Subject: [PATCH 1/6] [clangd] fix extract-to-function for overloaded operators
When selecting code that contains the use of overloaded operators,
the SelectionTree will attribute the operator to the operator
declaration, not to the `CXXOperatorCallExpr`. To allow
extract-to-function to work with these operators, make unselected
`CXXOperatorCallExpr`s valid root statements, just like `DeclStmt`s.
Partially fixes clangd/clangd#1254
---
.../refactor/tweaks/ExtractFunction.cpp | 15 +++---
.../unittests/tweaks/ExtractFunctionTests.cpp | 47 +++++++++++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++
3 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 0302839c58252e..aae480175b33f6 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -56,6 +56,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclBase.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/NestedNameSpecifier.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/Stmt.h"
@@ -70,7 +71,6 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Error.h"
-#include "llvm/Support/raw_os_ostream.h"
#include <optional>
namespace clang {
@@ -104,9 +104,12 @@ bool isRootStmt(const Node *N) {
// Root statement cannot be partially selected.
if (N->Selected == SelectionTree::Partial)
return false;
- // Only DeclStmt can be an unselected RootStmt since VarDecls claim the entire
- // selection range in selectionTree.
- if (N->Selected == SelectionTree::Unselected && !N->ASTNode.get<DeclStmt>())
+ // A DeclStmt can be an unselected RootStmt since VarDecls claim the entire
+ // selection range in selectionTree. Additionally, an CXXOperatorCallExpr of a
+ // binary operation can be unselected because it's children claim the entire
+ // selection range in the selection tree (e.g. <<).
+ if (N->Selected == SelectionTree::Unselected && !N->ASTNode.get<DeclStmt>() &&
+ !N->ASTNode.get<CXXOperatorCallExpr>())
return false;
return true;
}
@@ -913,8 +916,8 @@ Expected<Tweak::Effect> ExtractFunction::apply(const Selection &Inputs) {
tooling::Replacements OtherEdit(
createForwardDeclaration(*ExtractedFunc, SM));
- if (auto PathAndEdit = Tweak::Effect::fileEdit(SM, SM.getFileID(*FwdLoc),
- OtherEdit))
+ if (auto PathAndEdit =
+ Tweak::Effect::fileEdit(SM, SM.getFileID(*FwdLoc), OtherEdit))
MultiFileEffect->ApplyEdits.try_emplace(PathAndEdit->first,
PathAndEdit->second);
else
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index dec63d454d52c6..8e347b516c6ffe 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -571,6 +571,53 @@ int getNum(bool Superstitious, int Min, int Max) {
EXPECT_EQ(apply(Before), After);
}
+TEST_F(ExtractFunctionTest, OverloadedOperators) {
+ Context = File;
+ std::string Before = R"cpp(struct A {
+ int operator+(int x) { return x; }
+ };
+ A &operator<<(A &, int);
+ A &operator|(A &, int);
+
+ A stream{};
+
+ void foo(int, int);
+
+ int main() {
+ [[foo(1, 2);
+ foo(3, 4);
+ stream << 42;
+ stream + 42;
+ stream | 42;
+ foo(1, 2);
+ foo(3, 4);]]
+ })cpp";
+ std::string After =
+ R"cpp(struct A {
+ int operator+(int x) { return x; }
+ };
+ A &operator<<(A &, int);
+ A &operator|(A &, int);
+
+ A stream{};
+
+ void foo(int, int);
+
+ void extracted() {
+foo(1, 2);
+ foo(3, 4);
+ stream << 42;
+ stream + 42;
+ stream | 42;
+ foo(1, 2);
+ foo(3, 4);
+}
+int main() {
+ extracted();
+ })cpp";
+ EXPECT_EQ(apply(Before), After);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 442fb7180555ea..157263737149c6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -78,6 +78,9 @@ Code actions
- Added `Swap operands` tweak for certain binary operators.
+- Improved the extract-to-function code action to allow extracting statements
+ with overloaded operators like ``<<`` of ``std::ostream``.
+
Signature help
^^^^^^^^^^^^^^
>From 0fda4326e01afc8dc4ee909aa8f9037c080a41a9 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Tue, 13 Feb 2024 19:12:21 +0100
Subject: [PATCH 2/6] fix typo
---
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index aae480175b33f6..ee001cf399fcab 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -105,7 +105,7 @@ bool isRootStmt(const Node *N) {
if (N->Selected == SelectionTree::Partial)
return false;
// A DeclStmt can be an unselected RootStmt since VarDecls claim the entire
- // selection range in selectionTree. Additionally, an CXXOperatorCallExpr of a
+ // selection range in selectionTree. Additionally, a CXXOperatorCallExpr of a
// binary operation can be unselected because it's children claim the entire
// selection range in the selection tree (e.g. <<).
if (N->Selected == SelectionTree::Unselected && !N->ASTNode.get<DeclStmt>() &&
>From 6df0c2a1dceb5df38e0dfacb6af75a235462b058 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Thu, 31 Oct 2024 10:36:14 +0100
Subject: [PATCH 3/6] add test for single-expr-only
---
.../clangd/unittests/tweaks/ExtractFunctionTests.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 8e347b516c6ffe..39cdaeac405719 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -190,6 +190,14 @@ F (extracted();)
}]]
)cpp";
EXPECT_EQ(apply(CompoundFailInput), "unavailable");
+
+ ExtraArgs.push_back("-std=c++14");
+ // FIXME: Expressions are currently not extracted
+ EXPECT_EQ(apply(R"cpp(
+ void sink(int);
+ void call() { sink([[1+1]]); }
+ )cpp"),
+ "unavailable");
}
TEST_F(ExtractFunctionTest, DifferentHeaderSourceTest) {
>From 38003e38b867a2a4787e82580059d4bc14d28c29 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <git.julian.schmidt at gmail.com>
Date: Sun, 10 Nov 2024 18:54:05 +0100
Subject: [PATCH 4/6] add test for single expression extraction being not
available
---
.../clangd/unittests/tweaks/ExtractFunctionTests.cpp | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 39cdaeac405719..54944fefaba9bf 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -194,8 +194,12 @@ F (extracted();)
ExtraArgs.push_back("-std=c++14");
// FIXME: Expressions are currently not extracted
EXPECT_EQ(apply(R"cpp(
- void sink(int);
- void call() { sink([[1+1]]); }
+ void call() { [[1+1]]; }
+ )cpp"),
+ "unavailable");
+ // FIXME: Expression are currently not extracted
+ EXPECT_EQ(apply(R"cpp(
+ void call() { [[1+1;]] }
)cpp"),
"unavailable");
}
>From b7dc7477eefaf1d2ef3130223600b35ad3249c58 Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Fri, 15 Nov 2024 00:42:29 -0500
Subject: [PATCH 5/6] fix typos
---
.../clangd/refactor/tweaks/ExtractFunction.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index ee001cf399fcab..cd07cbf73635c2 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -95,8 +95,8 @@ enum FunctionDeclKind {
OutOfLineDefinition
};
-// A RootStmt is a statement that's fully selected including all it's children
-// and it's parent is unselected.
+// A RootStmt is a statement that's fully selected including all its children
+// and its parent is unselected.
// Check if a node is a root statement.
bool isRootStmt(const Node *N) {
if (!N->ASTNode.get<Stmt>())
@@ -106,7 +106,7 @@ bool isRootStmt(const Node *N) {
return false;
// A DeclStmt can be an unselected RootStmt since VarDecls claim the entire
// selection range in selectionTree. Additionally, a CXXOperatorCallExpr of a
- // binary operation can be unselected because it's children claim the entire
+ // binary operation can be unselected because its children claim the entire
// selection range in the selection tree (e.g. <<).
if (N->Selected == SelectionTree::Unselected && !N->ASTNode.get<DeclStmt>() &&
!N->ASTNode.get<CXXOperatorCallExpr>())
>From 84f6ec5e371c34d25548cad9fee3d5bbfe08546d Mon Sep 17 00:00:00 2001
From: Nathan Ridge <zeratul976 at hotmail.com>
Date: Fri, 15 Nov 2024 00:43:28 -0500
Subject: [PATCH 6/6] make fixme comment more precise
---
.../clangd/unittests/tweaks/ExtractFunctionTests.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 54944fefaba9bf..eff4d0f43595c5 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -197,7 +197,7 @@ F (extracted();)
void call() { [[1+1]]; }
)cpp"),
"unavailable");
- // FIXME: Expression are currently not extracted
+ // FIXME: Single expression statements are currently not extracted
EXPECT_EQ(apply(R"cpp(
void call() { [[1+1;]] }
)cpp"),
More information about the cfe-commits
mailing list