Author: Christian Kandeler Date: 2026-10-21T12:50:11+02:00 New Revision: 186e70f4e6a1521991d2340d3d49b23b1beff11f
URL: https://github.com/llvm/llvm-project/commit/186e70f4e6a1521991d2340d3d49b23b1beff11f DIFF: https://github.com/llvm/llvm-project/commit/186e70f4e6a1521991d2340d3d49b23b1beff11f.diff LOG: [clangd] Extract to function: Suppress pure forwarding wrappers (#232654) Extracting a single statement that's just a lone call (or a return of one), where every operand that would need to become a parameter is already exactly that parameter value unchanged, produces nothing but a renamed call: `foo(x);` becomes `void extracted(int x) { foo(x); }` called as `extracted(x);`, with no abstraction gained. A value-less `return;` is the same kind of no-op: there's nothing to abstract over at all. Both are now rejected. This is deliberately narrow and per-operand, not a naive argument-count check: `foo(100)` (a literal), `foo(globalVar)` (an already-visible global), and `foo(x + y)` (real computation) are all still offered, since each does something a plain rename wouldn't. Assisted-by: Claude Code Added: Modified: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp index de9a5fa45b9c0..7b60b95bd61f2 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp @@ -390,13 +390,152 @@ computeEnclosingFuncRange(const FunctionDecl *EnclosingFunction, return toHalfOpenFileRange(SM, LangOpts, EnclosingFunction->getSourceRange()); } +// Whether D would need to become a forwarded parameter of the extracted +// function if referenced from inside the extraction zone: a name (a +// parameter, local variable, or structured-binding) declared inside the +// enclosing function. Such a name is only visible within that function, +// regardless of storage duration -- unlike hasLocalStorage(), this is +// also true of a function-scope `static` or `thread_local` variable, +// which (unlike a namespace-scope global or a static data member, both +// reachable via a qualified name from anywhere) isn't visible wherever +// the extracted function ends up. +bool mustBeForwarded(const Decl *D) { + if (!isa<VarDecl, BindingDecl>(D)) + return false; + return D->getDeclContext()->isFunctionOrMethod(); +} + +// Whether E, as an operand of a call being considered by isTrivialExtraction, +// is a bare (parenthesized/implicitly cast) reference to a Decl that would +// need to become a parameter, forwarded unchanged -- and if so, that Decl. +// Returns null otherwise: a literal, a reference to an already-visible +// global, or any non-bare expression all mean this operand alone justifies +// offering the extraction, since it hardcodes or computes something a +// plain rename wouldn't. +const Decl *bareForwardedDecl(const Expr *E) { + const auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()); + if (!DRE || !mustBeForwarded(DRE->getDecl())) + return nullptr; + return DRE->getDecl(); +} + +// The operands of Call relevant to isTrivialExtraction: the receiver of +// a method call, a non-method callee that isn't a plain reference to a +// fixed function (e.g. a function-pointer variable instead), and every +// explicit argument. An operator call's operands are already included in +// its own arguments() (clang models e.g. `a + b` as `operator+(a, b)`), +// so its callee -- always a reference to the operator function/overload +// itself, never itself customizable -- is always excluded. +llvm::SmallVector<const Expr *, 4> relevantCallOperands(const CallExpr *Call) { + llvm::SmallVector<const Expr *, 4> Operands; + if (const auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) { + // getImplicitObjectArgument() can itself be null (its FIXME notes + // this isn't fully handled for all cases); other clang analyses + // (e.g. LiveVariables, dataflow's Environment::pushCall) guard it + // the same way rather than assume it's always present. + if (const Expr *ImplicitObject = MCE->getImplicitObjectArgument()) { + // `this` (implicit or explicit) always refers to the same object + // -- the extracted function becomes a sibling member itself, so + // `this` stays implicit there too, never becoming a parameter -- + // so, like an ordinary function name, it's excluded rather than + // required to be "bare". + if (!isa<CXXThisExpr>(ImplicitObject->IgnoreParenImpCasts())) + Operands.push_back(ImplicitObject); + } + // A pointer-to-member call (`(s.*pmf)(x)`) has no statically known + // method to exclude the way an ordinary method call's name is + // excluded below: which member is invoked is itself a real, + // customizable operand here (e.g. the `pmf` it's called through), + // not inherent to what the wrapper does. + if (!MCE->getMethodDecl()) + Operands.push_back(MCE->getCallee()); + } else if (!isa<CXXOperatorCallExpr>(Call)) { + const Expr *Callee = Call->getCallee()->IgnoreParenImpCasts(); + if (!(isa<DeclRefExpr>(Callee) && + isa<FunctionDecl>(cast<DeclRefExpr>(Callee)->getDecl()))) + Operands.push_back(Call->getCallee()); + } + // An omitted argument filled in from the callee's own default isn't + // something this call site wrote or hardcoded: it's inherent to + // calling the function at all, same as the function's name, so it's + // excluded rather than required to be "bare". Default arguments are + // always trailing, so the first one found marks the end of the + // explicitly-written ones. + auto Args = Call->arguments(); + auto FirstDefault = llvm::find_if( + Args, [](const Expr *Arg) { return isa<CXXDefaultArgExpr>(Arg); }); + Operands.append(Args.begin(), FirstDefault); + return Operands; +} + +// Whether extracting S on its own, as a single root statement, would add +// no real abstraction over leaving it where it is: +// - A value-less `return;` has nothing to abstract over at all. +// - A call (to a function, method, or operator), or a return of one, is +// a pure 1:1 forwarding wrapper if every relevant operand (see +// relevantCallOperands) is already exactly the parameter value it +// would become, unchanged (or there are none). Extracting e.g. +// `foo();`, `foo(x)` (x a local), or `return foo(x);` this way would +// just rename the call; `foo(100)`, `foo(globalVar)`, or `foo(x + y)` +// would not, since each does something a plain rename wouldn't +// (hardcode a value/global, or compute one). For the return case, the +// returned value may be parenthesized and/or need an implicit +// conversion to the enclosing function's own return type (e.g. foo +// returns int, the enclosing function long); IgnoreParenImpCasts() +// looks through both -- neither changes the verdict, since the +// extracted function would perform the exact same conversion +// internally. +// - Using the same captured Decl in more than one operand (e.g. +// `foo(A, A)`) is, despite every individual operand being bare, +// *not* pure forwarding: it constrains a relationship (both slots +// must hold the same value) that a plain rename wouldn't, the same +// kind of real specialization as hardcoding a literal. +// - createParameters orders the extracted function's own parameters by +// each captured Decl's declaration order in the *enclosing* function, +// not by how they're used in this call. So if a later operand here +// refers to a Decl declared *earlier* than an operand before it (e.g. +// `foo(B, A)` inside `bar(int A, int B)`), the parameters end up +// reordered relative to this call: `extracted(int A, int B) { foo(B, +// A); }`. That's not a transparent pass-through either, even though +// this one call site still behaves the same after the compensating +// reorder at the call site (`extracted(A, B)`) -- `extracted`'s own +// signature no longer predicts its behavior the way a plain rename's +// would, since it silently permutes its own parameters internally. +bool isTrivialExtraction(const Stmt *S, const SourceManager &SM) { + if (const auto *RS = dyn_cast<ReturnStmt>(S)) { + if (!RS->getRetValue()) + return true; + S = RS->getRetValue()->IgnoreParenImpCasts(); + } + const auto *Call = dyn_cast<CallExpr>(S); + if (!Call) + return false; + llvm::SmallPtrSet<const Decl *, 4> SeenDecls; + SourceLocation LastDeclLoc; + for (const Expr *Operand : relevantCallOperands(Call)) { + const Decl *D = bareForwardedDecl(Operand); + if (!D || !SeenDecls.insert(D).second) + return false; + SourceLocation DeclLoc = D->getLocation(); + if (LastDeclLoc.isValid() && + !SM.isBeforeInTranslationUnit(LastDeclLoc, DeclLoc)) + return false; + LastDeclLoc = DeclLoc; + } + return true; +} + // returns true if Child can be a single RootStmt being extracted from // EnclosingFunc. -bool validSingleChild(const Node *Child, const FunctionDecl *EnclosingFunc) { +bool validSingleChild(const Node *Child, const FunctionDecl *EnclosingFunc, + const SourceManager &SM) { // Extracting the body of EnclosingFunc would remove it's definition. assert(EnclosingFunc->hasBody() && "We should always be extracting from a function body."); - if (Child->ASTNode.get<Stmt>() == EnclosingFunc->getBody()) + const Stmt *S = Child->ASTNode.get<Stmt>(); + if (S == EnclosingFunc->getBody()) + return false; + if (isTrivialExtraction(S, SM)) return false; return true; } @@ -414,7 +553,8 @@ std::optional<ExtractionZone> findExtractionZone(const Node *CommonAnc, // 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)) + !validSingleChild(ExtZone.getLastRootStmt(), ExtZone.EnclosingFunction, + SM)) return std::nullopt; if (auto FuncRange = computeEnclosingFuncRange(ExtZone.EnclosingFunction, SM, LangOpts)) diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp index fd2472655af37..981373b893347 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp @@ -744,7 +744,7 @@ TEST_F(ExtractFunctionTest, RangeBasedFor) { struct Vec { int *begin(); int *end(); }; Vec V; void foo(int); - void f() { for (auto X : V) { [[foo(X);]] } } + void f() { for (auto X : V) { [[foo(X + 1);]] } } )cpp"), HasSubstr("extracted")); } @@ -782,10 +782,11 @@ TEST_F(ExtractFunctionTest, ConstParameters) { Context = File; // A captured scalar that's only read becomes a by-value parameter; // non-scalars instead become a const reference (see the `S` cases - // below). + // below). The duplicated calls below are there to prevent "pure forwarding" + // suppression. EXPECT_THAT(apply(R"cpp( void use(int); - void f(int x) { [[use(x);]] } + void f(int x) { [[use(x); use(x);]] } )cpp"), HasSubstr("void extracted(int x)")); // Direct assignment: stays non-const. @@ -800,45 +801,46 @@ TEST_F(ExtractFunctionTest, ConstParameters) { // A non-const method call may mutate the object: stays non-const. EXPECT_THAT(apply(R"cpp( struct S { void mutate(); }; - void f(S s) { [[s.mutate();]] } + void f(S s) { [[s.mutate(); s.mutate();]] } )cpp"), HasSubstr("void extracted(S &s)")); // A const method call cannot mutate the object: becomes const. EXPECT_THAT(apply(R"cpp( struct S { void inspect() const; }; - void f(S s) { [[s.inspect();]] } + void f(S s) { [[s.inspect(); s.inspect();]] } )cpp"), HasSubstr("void extracted(const S &s)")); // Passed to a parameter taking a non-const reference: stays non-const, // since the callee could mutate it through that reference. EXPECT_THAT(apply(R"cpp( void mayMutate(int &); - void f(int x) { [[mayMutate(x);]] } + void f(int x) { [[mayMutate(x); mayMutate(x);]] } )cpp"), HasSubstr("void extracted(int &x)")); // Passed to a parameter taking a const reference or by value: becomes // an unmutated scalar, so by value. EXPECT_THAT(apply(R"cpp( void readOnly(const int &); - void f(int x) { [[readOnly(x);]] } + void f(int x) { [[readOnly(x); readOnly(x);]] } )cpp"), HasSubstr("void extracted(int x)")); EXPECT_THAT(apply(R"cpp( void byValue(int); - void f(int x) { [[byValue(x);]] } + void f(int x) { [[byValue(x); byValue(x);]] } )cpp"), HasSubstr("void extracted(int x)")); // A scalar parameter that's already declared const keeps that // qualifier when passed by value: It might be relevant for overload // resolution. - EXPECT_THAT(apply("void use(int); void f(const int x) { [[use(x);]] }"), - HasSubstr("void extracted(const int x)")); + EXPECT_THAT( + apply("void use(int); void f(const int x) { [[use(x); use(x);]] }"), + HasSubstr("void extracted(const int x)")); // A non-scalar parameter that's already declared const keeps that // qualifier, since it's still passed by reference. EXPECT_THAT(apply(R"cpp( struct S {}; void use(const S &); - void f(const S s) { [[use(s);]] } + void f(const S s) { [[use(s); use(s);]] } )cpp"), HasSubstr("void extracted(const S &s)")); } @@ -895,7 +897,7 @@ TEST_F(ExtractFunctionTest, ConstParametersMemberCallArguments) { // the argument check from the object check. EXPECT_THAT(apply(R"cpp( struct S { void mayMutate(int &) const; }; - void f(S s, int x) { [[s.mayMutate(x);]] } + void f(S s, int x) { [[s.mayMutate(x); s.mayMutate(x);]] } )cpp"), HasSubstr("extracted(const S &s, int &x)")); // Same, but through an overloaded operator: the implicit object is @@ -903,7 +905,7 @@ TEST_F(ExtractFunctionTest, ConstParametersMemberCallArguments) { // found at the right offset. EXPECT_THAT(apply(R"cpp( struct S { void operator()(int &) const; }; - void f(S s, int x) { [[s(x);]] } + void f(S s, int x) { [[s(x); s(x);]] } )cpp"), HasSubstr("extracted(const S &s, int &x)")); // A free (non-member) operator overload has no implicit object, so all @@ -911,7 +913,7 @@ TEST_F(ExtractFunctionTest, ConstParametersMemberCallArguments) { EXPECT_THAT(apply(R"cpp( struct S {}; void operator+(const S &, int &); - void f(S s, int x) { [[s + x;]] } + void f(S s, int x) { [[s + x; s + x;]] } )cpp"), HasSubstr("extracted(const S &s, int &x)")); } @@ -970,7 +972,7 @@ TEST_F(ExtractFunctionTest, ConstParametersStaticOperatorCall) { ExtraArgs.push_back("-std=c++23"); EXPECT_THAT(apply(R"cpp( struct S { static void operator()(int); }; - void f(S s, int x) { [[s(x);]] } + void f(S s, int x) { [[s(x); s(x);]] } )cpp"), HasSubstr("extracted(const S &s, int x)")); } @@ -998,13 +1000,13 @@ TEST_F(ExtractFunctionTest, ConstParametersFunctionReference) { TEST_F(ExtractFunctionTest, ConstParametersScalarsByValue) { Context = File; // An unmutated pointer is a scalar too: passed by value. - EXPECT_THAT(apply("void use(int *); void f(int *p) { [[use(p);]] }"), + EXPECT_THAT(apply("void use(int *); void f(int *p) { [[use(p); use(p);]] }"), HasSubstr("extracted(int *p)")); // An unmutated enum: passed by value. EXPECT_THAT(apply(R"cpp( enum E { A, B }; void use(E); - void f(E e) { [[use(e);]] } + void f(E e) { [[use(e); use(e);]] } )cpp"), HasSubstr("extracted(E e)")); // A class type, even one that's small and trivially copyable, is never @@ -1332,6 +1334,290 @@ TEST_F(ExtractFunctionTest, CFileRejectMutatedPointerToVariableLengthArray) { "fail: Too complex to extract."); } +TEST_F(ExtractFunctionTest, RejectPureForwardingZeroArgCall) { + Context = File; + // No abstraction gained: the extracted function would just rename the + // call, with no customizable behavior at all. + EXPECT_EQ(apply("void foo(); void f() { [[foo();]] }"), "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingSingleArgCall) { + Context = File; + // x is forwarded to foo unchanged: still just a rename. + EXPECT_EQ(apply("void foo(int); void f(int x) { [[foo(x);]] }"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_LiteralArg) { + Context = File; + // Hardcoding 100 is a real specialization, unlike a plain rename. + EXPECT_THAT(apply("void foo(int); void f() { [[foo(100);]] }"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_GlobalArg) { + Context = File; + // Hardcoding which already-visible global to use is likewise a real + // specialization: unlike a captured local, globalVar doesn't need to + // become a parameter at all. + EXPECT_THAT(apply(R"cpp( + int globalVar; + void foo(int); + void f() { [[foo(globalVar);]] } + )cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_ComputedArg) { + Context = File; + // foo(x + y) needs 2 parameters for 1 argument slot: real computation, + // not just forwarding x and y unchanged. + EXPECT_THAT(apply("void foo(int); void f(int x, int y) { [[foo(x + y);]] }"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingMethodCall) { + Context = File; + // Both the receiver and the argument are forwarded unchanged. + EXPECT_EQ(apply(R"cpp( + struct S { void method(int); }; + void f(S s, int x) { [[s.method(x);]] } + )cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_GlobalReceiver) { + Context = File; + // Hardcoding which already-visible object to call the method on is a + // real specialization, same as a global argument. + EXPECT_THAT(apply(R"cpp( + struct S { void method(); }; + S globalS; + void f() { [[globalS.method();]] } + )cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingOperatorCall) { + Context = File; + // Both operands of a + b are forwarded unchanged; the operator function + // itself (never customizable) is correctly excluded from the check. + EXPECT_EQ(apply(R"cpp( + struct S {}; + void operator+(const S &, int); + void f(S s, int x) { [[s + x;]] } + )cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingFunctionPointerCall) { + Context = File; + // Unlike an ordinary function name, a function-pointer variable callee + // does need to become a parameter -- but it's still forwarded + // unchanged here, same as x. + EXPECT_EQ(apply("void f(void (*fp)(int), int x) { [[fp(x);]] }"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_ComputedCallee) { + Context = File; + // Which function pointer gets called is itself computed here, not a + // bare forwarded reference: real logic, not just forwarding. + EXPECT_THAT( + apply("void f(void (*fp[2])(int), int i, int x) { [[fp[i](x);]] }"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingReturnStmt) { + Context = File; + // A call wrapped in a ReturnStmt is just as much a pure rename as a + // bare call statement. + EXPECT_EQ(apply("int foo(int); int f(int x) { [[return foo(x);]] }"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingReturnStmtImplicitConversion) { + Context = File; + // The implicit int-to-long conversion doesn't change the verdict: the + // extracted function would perform the exact same conversion + // internally, so this is still just a rename. + EXPECT_EQ(apply("int foo(int); long f(int x) { [[return foo(x);]] }"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_ReturnStmtComputedArg) { + Context = File; + // Same as the non-return case: a computed argument is real abstraction. + EXPECT_THAT( + apply("int foo(int); int f(int x, int y) { [[return foo(x + y);]] }"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectBareReturnStmt) { + Context = File; + // A value-less return has nothing to abstract over at all: extracting + // it would just wrap it in a function that does the exact same thing. + EXPECT_EQ(apply("void f() { [[return;]] }"), "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_PointerToMemberCallee) { + Context = File; + // Unlike an ordinary method call, a pointer-to-member call has no + // statically known method (getMethodDecl() is null): which member is + // invoked is itself computed here (table[I]), not inherent to what the + // wrapper does, so it must be checked like any other operand. + EXPECT_THAT(apply(R"cpp( + struct St {}; + void f(St S, void (St::*table[2])(int), int I, int A) { + [[(S.*table[I])(A);]] + })cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingImplicitThis) { + Context = File; + // An implicit `this` receiver always refers to the same object -- the + // extracted function becomes a sibling member itself, so `this` stays + // implicit there too -- same as an ordinary function name, it must not + // disqualify suppression just for not being a "bare" reference. + EXPECT_EQ(apply(R"cpp( + struct S { + void foo(int); + void bar(int A) { + [[foo(A);]] + } + };)cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingDefaultArg) { + Context = File; + // An argument filled in from the callee's own default (as opposed to + // one actually written at this call site) isn't something to hardcode + // a judgement about: it must not disqualify suppression. + EXPECT_EQ(apply(R"cpp( + void foo(int A, int B = 0); + void bar(int A) { + [[foo(A);]] + })cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingParenthesizedReturn) { + Context = File; + // Parentheses around the returned call must be looked through, same as + // an implicit conversion. + EXPECT_EQ(apply(R"cpp( + int foo(int); + int bar(int A) { + [[return (foo(A));]] + })cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_RepeatedArg) { + Context = File; + // Using the same captured local for more than one argument constrains a + // relationship (both slots must hold the same value) that a plain + // rename wouldn't: the same kind of real specialization as hardcoding + // a literal, even though each individual operand is bare. + EXPECT_THAT(apply(R"cpp( + void foo(int A, int B); + void bar(int A, int B) { + [[foo(A, A);]] + })cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_ReorderedArgs) { + Context = File; + // createParameters orders the extracted function's parameters by each + // Decl's declaration order in bar (A, then B), not by how they're used + // here -- so despite every argument being an independently bare, + // distinct local, the extracted function ends up permuting its own + // parameters internally (extracted(int A, int B) { foo(B, A); }): not + // a transparent pass-through, since its signature alone no longer + // predicts its behavior the way a plain rename's would. + EXPECT_THAT(apply(R"cpp( + void foo(int A, int B); + void bar(int A, int B) { + [[foo(B, A);]] + })cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingPreservedArgOrder) { + Context = File; + // Same distinct locals as the reordered case above, but passed in the + // same order they're declared in bar: no permutation is hidden in the + // extracted function, so this is still just a rename. + EXPECT_EQ(apply(R"cpp( + void foo(int A, int B); + void bar(int A, int B) { + [[foo(A, B);]] + })cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingFunctionScopeStatic) { + Context = File; + // A function-scope static variable's name, unlike a namespace-scope + // global's, isn't visible wherever the extracted function ends up: it + // does need forwarding, same as an ordinary local variable, so this is + // still just a rename. + EXPECT_EQ(apply(R"cpp( + void foo(int); + void bar() { + static int A; + [[foo(A);]] + })cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingFunctionScopeThreadLocal) { + Context = File; + // Same as the static case: thread_local doesn't change the variable's + // name visibility, only its storage. + EXPECT_EQ(apply(R"cpp( + void foo(int); + void bar() { + thread_local int A; + [[foo(A);]] + })cpp"), + "unavailable"); +} + +TEST_F(ExtractFunctionTest, OfferDespitePureForwarding_NamespaceScopeStatic) { + Context = File; + // Unlike a function-scope static, a namespace-scope one (internal + // linkage notwithstanding) is visible wherever the extracted function + // ends up, same as an ordinary global: hardcoding it is a real + // specialization. + EXPECT_THAT(apply(R"cpp( + static int A; + void foo(int); + void bar() { [[foo(A);]] } + )cpp"), + HasSubstr("extracted")); +} + +TEST_F(ExtractFunctionTest, RejectPureForwardingStructuredBinding) { + Context = File; + // A structured-binding name is declared by a BindingDecl, not a + // VarDecl, but is just as function-local and needs forwarding the + // same way. + ExtraArgs.push_back("-std=c++17"); + EXPECT_EQ(apply(R"cpp( + void foo(int); + struct Pair { int A, B; }; + Pair make(); + void bar() { + auto [A, B] = make(); + [[foo(A);]] + })cpp"), + "unavailable"); +} + } // namespace } // namespace clangd } // namespace clang _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
