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.schm...@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.schm...@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.schm...@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.schm...@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 <zeratul...@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 <zeratul...@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"), _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits