diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -12,8 +12,11 @@ #include "SourceCode.h" #include "refactor/Tweak.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/LambdaCapture.h" #include "clang/AST/OperationKinds.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" @@ -74,10 +77,52 @@ public: std::vector ReferencedDecls; bool VisitDeclRefExpr(DeclRefExpr *DeclRef) { // NOLINT + // Stop the call operator of lambdas from being marked as a referenced + // DeclRefExpr in immediately invoked lambdas. + if (const auto *const Method = + llvm::dyn_cast(DeclRef->getDecl()); + Method != nullptr && Method->getParent()->isLambda()) { + return true; + } ReferencedDecls.push_back(DeclRef->getDecl()); return true; } + + // Local variables declared inside of the selected lambda cannot go out of + // scope. The DeclRefExprs that are important are the variables captured, + // the DeclRefExprs inside the initializers of init-capture variables, + // variables mentioned in trailing return types, constraints and explicit + // defaulted template parameters. + bool TraverseLambdaExpr(LambdaExpr *LExpr) { + for (const auto &[Capture, Initializer] : + llvm::zip(LExpr->captures(), LExpr->capture_inits())) { + TraverseLambdaCapture(LExpr, &Capture, Initializer); + } + + if (clang::Expr *const RequiresClause = + LExpr->getTrailingRequiresClause()) { + TraverseStmt(RequiresClause); + } + + for (auto *const TemplateParam : LExpr->getExplicitTemplateParameters()) + TraverseDecl(TemplateParam); + + if (auto *const CallOperator = LExpr->getCallOperator()) { + TraverseType(CallOperator->getDeclaredReturnType()); + + for (auto *const Param : CallOperator->parameters()) { + TraverseParmVarDecl(Param); + } + + for (auto *const Attr : CallOperator->attrs()) { + TraverseAttr(Attr); + } + } + + return true; + } }; + FindDeclRefsVisitor Visitor; Visitor.TraverseStmt(const_cast(cast(Expr))); return Visitor.ReferencedDecls; @@ -152,10 +197,16 @@ auto CanExtractOutside = [](const SelectionTree::Node *InsertionPoint) -> bool { if (const clang::Stmt *Stmt = InsertionPoint->ASTNode.get()) { - // Allow all expressions except LambdaExpr since we don't want to extract - // from the captures/default arguments of a lambda - if (isa(Stmt)) - return !isa(Stmt); + if (isa(Stmt)) { + // Do not allow extraction from the initializer of a defaulted parameter + // to a local variable (e.g. a function-local lambda). + if (InsertionPoint->Parent->ASTNode.get() != nullptr) { + return false; + } + + return true; + } + // We don't yet allow extraction from switch/case stmt as we would need to // jump over the switch stmt even if there is a CompoundStmt inside the // switch. And there are other Stmts which we don't care about (e.g. @@ -240,7 +291,7 @@ SelectedOperands.clear(); if (const BinaryOperator *Op = - llvm::dyn_cast_or_null(N.ASTNode.get())) { + llvm::dyn_cast_or_null(N.ASTNode.get())) { Kind = Op->getOpcode(); ExprLoc = Op->getExprLoc(); SelectedOperands = N.Children; @@ -255,7 +306,7 @@ Kind = BinaryOperator::getOverloadedOpcode(Op->getOperator()); ExprLoc = Op->getExprLoc(); // Not all children are args, there's also the callee (operator). - for (const auto* Child : N.Children) { + for (const auto *Child : N.Children) { const Expr *E = Child->ASTNode.get(); assert(E && "callee and args should be Exprs!"); if (E == Op->getArg(0) || E == Op->getArg(1)) @@ -376,15 +427,15 @@ if (llvm::isa(Outer)) return true; // Control flow statements use condition etc, but not the body. - if (const auto* WS = llvm::dyn_cast(Outer)) + if (const auto *WS = llvm::dyn_cast(Outer)) return Inner == WS->getBody(); - if (const auto* DS = llvm::dyn_cast(Outer)) + if (const auto *DS = llvm::dyn_cast(Outer)) return Inner == DS->getBody(); - if (const auto* FS = llvm::dyn_cast(Outer)) + if (const auto *FS = llvm::dyn_cast(Outer)) return Inner == FS->getBody(); - if (const auto* FS = llvm::dyn_cast(Outer)) + if (const auto *FS = llvm::dyn_cast(Outer)) return Inner == FS->getBody(); - if (const auto* IS = llvm::dyn_cast(Outer)) + if (const auto *IS = llvm::dyn_cast(Outer)) return Inner == IS->getThen() || Inner == IS->getElse(); // Assume all other cases may be actual expressions. // This includes the important case of subexpressions (where Outer is Expr). diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp --- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp @@ -98,6 +98,7 @@ return [[t]].bar([[t]].z); } void v() { return; } + // function default argument void f(int b = [[1]]) { // empty selection @@ -131,10 +132,80 @@ goto label; label: a = [[1]]; + + // lambdas: captures + int x = 0; + [ [[=]] ](){}; + [ [[&]] ](){}; + [ [[x]] ](){}; + [ [[&x]] ](){}; + [y = [[x]] ](){}; + [ [[y = x]] ](){}; + + // lambdas: default args, cannot extract into function-local scope + [](int x = [[10]]){}; + [](auto h = [[ [i = [](){}](){} ]]) {}; + + // lambdas: default args + // Extracting from capture initializers is usually fine, + // but not if the capture itself is nested inside a default argument + [](auto h = [i = [[ [](){} ]]](){}) {}; + [](auto h = [i = [[ 42 ]]](){}) {}; + + // lambdas: scope + if (int a = 1) + if ([[ [&](){ return a + 1; } ]]() == 4) + a = a + 1; + + for (int c = 0; [[ [&]() { return c < b; } ]](); ++c) { + } + for (int c = 0; [[ [&]() { return c < b; } () ]]; ++c) { + } + + // lambdas: scope with structured binding + struct Coordinates { + int x{}; + int y{}; + }; + Coordinates c{}; + if (const auto [x, y] = c; x > y) + auto f = [[ [&]() { return x + y; } ]]; + + // lambdas: referencing outside variables that block extraction + // in trailing return type or in a decltype used + // by a parameter + if (int a = 1) + if ([[ []() -> decltype(a) { return 1; } ]] () == 4) + a = a + 1; + if (int a = 1) + if ([[ [](int x = decltype(a){}) { return 1; } ]] () == 4) + a = a + 1; + if (int a = 1) + if ([[ [](decltype(a) x) { return 1; } ]] (42) == 4) + a = a + 1; } )cpp"; EXPECT_UNAVAILABLE(UnavailableCases); + ExtraArgs = {"-std=c++20"}; + const char *UnavailableCasesCXX20 = R"cpp( + template + concept Constraint = requires (T t) { true; }; + void foo() { + // lambdas: referencing outside variables that block extraction + // in requires clause or defaulted explicit template parameters + if (int a = 1) + if ([[ [](auto b) requires (Constraint && Constraint) { return true; } ]] (a)) + a = a + 1; + + if (int a = 1) + if ([[ [](T b) { return true; } ]] (a)) + a = a + 1; + } + )cpp"; + EXPECT_UNAVAILABLE(UnavailableCasesCXX20); + ExtraArgs.clear(); + // vector of pairs of input and output strings std::vector> InputOutputs = { // extraction from variable declaration/assignment @@ -282,6 +353,219 @@ void f() { auto placeholder = S(2) + S(3) + S(4); S x = S(1) + placeholder + S(5); })cpp"}, + // lambda expressions + {R"cpp(template void f(T) {} + void f2() { + f([[ [](){ return 42; }]]); + } + )cpp", + R"cpp(template void f(T) {} + void f2() { + auto placeholder = [](){ return 42; }; f( placeholder); + } + )cpp"}, + {R"cpp(template void f(T) {} + void f2() { + f([x = [[40 + 2]] ](){ return 42; }); + } + )cpp", + R"cpp(template void f(T) {} + void f2() { + auto placeholder = 40 + 2; f([x = placeholder ](){ return 42; }); + } + )cpp"}, + {R"cpp(auto foo(int VarA) { + return [VarA]() { + return [[ [VarA, VarC = 42 + VarA](int VarB) { return VarA + VarB + VarC; }]]; + }; + } + )cpp", + R"cpp(auto foo(int VarA) { + return [VarA]() { + auto placeholder = [VarA, VarC = 42 + VarA](int VarB) { return VarA + VarB + VarC; }; return placeholder; + }; + } + )cpp"}, + {R"cpp(template void f(T) {} + void f2(int var) { + f([[ [&var](){ auto internal_val = 42; return var + internal_val; }]]); + } + )cpp", + R"cpp(template void f(T) {} + void f2(int var) { + auto placeholder = [&var](){ auto internal_val = 42; return var + internal_val; }; f( placeholder); + } + )cpp"}, + {R"cpp(template void f(T) { } + struct A { + void f2(int& var) { + auto local_var = 42; + f([[ [&var, &local_var, this]() { + auto internal_val = 42; + return var + local_var + internal_val + member; + }]]); + } + + int member = 42; +}; + )cpp", + R"cpp(template void f(T) { } + struct A { + void f2(int& var) { + auto local_var = 42; + auto placeholder = [&var, &local_var, this]() { + auto internal_val = 42; + return var + local_var + internal_val + member; + }; f( placeholder); + } + + int member = 42; +}; + )cpp"}, + {R"cpp(void f() { auto x = [[ [](){ return 42; }]]; })cpp", + R"cpp(void f() { auto placeholder = [](){ return 42; }; auto x = placeholder; })cpp"}, + {R"cpp( + template + auto sink(T f) { return f(); } + int bar() { + return sink([[ []() { return 42; }]]); + } + )cpp", + R"cpp( + template + auto sink(T f) { return f(); } + int bar() { + auto placeholder = []() { return 42; }; return sink( placeholder); + } + )cpp"}, + {R"cpp( + int main() { + if (int a = 1) { + if ([[ [&](){ return a + 1; } ]]() == 4) + a = a + 1; + } + })cpp", + R"cpp( + int main() { + if (int a = 1) { + auto placeholder = [&](){ return a + 1; }; if ( placeholder () == 4) + a = a + 1; + } + })cpp"}, + {R"cpp( + int main() { + if (int a = 1) { + if ([[ [&](){ return a + 1; }() ]] == 4) + a = a + 1; + } + })cpp", + R"cpp( + int main() { + if (int a = 1) { + auto placeholder = [&](){ return a + 1; }(); if ( placeholder == 4) + a = a + 1; + } + })cpp"}, + {R"cpp( + template + auto call(T t) { return t(); } + + int main() { + return [[ call([](){ int a = 1; return a + 1; }) ]] + 5; + })cpp", + R"cpp( + template + auto call(T t) { return t(); } + + int main() { + auto placeholder = call([](){ int a = 1; return a + 1; }); return placeholder + 5; + })cpp"}, + {R"cpp( + class Foo { + int bar() { + return [f = [[ [this](int g) { return g + x; } ]] ]() { return 42; }(); + } + int x; + }; + )cpp", + R"cpp( + class Foo { + int bar() { + auto placeholder = [this](int g) { return g + x; }; return [f = placeholder ]() { return 42; }(); + } + int x; + }; + )cpp"}, + {R"cpp( + int main() { + return [[ []() { return 42; }() ]]; + })cpp", + R"cpp( + int main() { + auto placeholder = []() { return 42; }(); return placeholder ; + })cpp"}, + {R"cpp( + template + void foo(Ts ...args) { + auto x = [[ [&args...]() {} ]]; + } + )cpp", + R"cpp( + template + void foo(Ts ...args) { + auto placeholder = [&args...]() {}; auto x = placeholder ; + } + )cpp"}, + {R"cpp( + struct Coordinates { + int x{}; + int y{}; + }; + + int main() { + Coordinates c = {}; + const auto [x, y] = c; + auto f = [[ [&]() { return x + y; } ]]; + } + )cpp", + R"cpp( + struct Coordinates { + int x{}; + int y{}; + }; + + int main() { + Coordinates c = {}; + const auto [x, y] = c; + auto placeholder = [&]() { return x + y; }; auto f = placeholder ; + } + )cpp"}, + {R"cpp( + struct Coordinates { + int x{}; + int y{}; + }; + + int main() { + Coordinates c = {}; + if (const auto [x, y] = c; x > y) { + auto f = [[ [&]() { return x + y; } ]]; + } + } + )cpp", + R"cpp( + struct Coordinates { + int x{}; + int y{}; + }; + + int main() { + Coordinates c = {}; + if (const auto [x, y] = c; x > y) { + auto placeholder = [&]() { return x + y; }; auto f = placeholder ; + } + } + )cpp"}, // Don't try to analyze across macro boundaries // FIXME: it'd be nice to do this someday (in a safe way) {R"cpp(#define ECHO(X) X diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -66,6 +66,11 @@ Code completion ^^^^^^^^^^^^^^^ +Code actions +^^^^^^^^^^^^ + +- The extract variable tweak gained support for extracting lambda expressions to a variable. + Signature help ^^^^^^^^^^^^^^