Author: Zeyi Xu Date: 2026-06-01T08:38:38+08:00 New Revision: f68ef5b4303de763ecf37fd57cbefc3891ee48c0
URL: https://github.com/llvm/llvm-project/commit/f68ef5b4303de763ecf37fd57cbefc3891ee48c0 DIFF: https://github.com/llvm/llvm-project/commit/f68ef5b4303de763ecf37fd57cbefc3891ee48c0.diff LOG: [clang-tidy] Fix FP/FN in cppcoreguidelines-missing-std-forward (#178651) Closes #176873 Added: Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp index 29218d6ba0565..8c998778f096b 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -64,24 +64,6 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) { FuncTemplate->getTemplateParameters()->getDepth(); } -AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) { - const IdentifierInfo *II = Node.getIdentifier(); - if (nullptr == II) - return false; - const StringRef Name = II->getName(); - - return Builder->removeBindings( - [this, Name](const ast_matchers::internal::BoundNodesMap &Nodes) { - const DynTypedNode &BN = Nodes.getNode(this->BindingID); - if (const auto *ND = BN.get<NamedDecl>()) { - if (!isa<FieldDecl, CXXMethodDecl, VarDecl>(ND)) - return true; - return ND->getName() != Name; - } - return true; - }); -} - AST_MATCHER_P(LambdaCapture, hasCaptureKind, LambdaCaptureKind, Kind) { return Node.getCaptureKind() == Kind; } @@ -95,21 +77,40 @@ AST_MATCHER(VarDecl, hasIdentifier) { return ID != nullptr && !ID->isPlaceholder(); } +AST_MATCHER_P(ValueDecl, refersToBoundParm, std::string, ParamID) { + return Builder->removeBindings( + [&](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto *Param = Nodes.getNodeAs<ParmVarDecl>(ParamID); + if (!Param) + return true; + + for (const ValueDecl *V = &Node; V;) { + if (V == Param) + return false; + + const auto *VD = dyn_cast<VarDecl>(V); + const Expr *Init = (VD && VD->getType()->isReferenceType()) + ? VD->getInit() + : nullptr; + const auto *DRE = + Init ? dyn_cast<DeclRefExpr>(Init->IgnoreParenImpCasts()) + : nullptr; + V = DRE ? DRE->getDecl() : nullptr; + } + return true; + }); +} + } // namespace void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { - auto RefToParmImplicit = allOf( - equalsBoundNode("var"), hasInitializer(ignoringParenImpCasts( - declRefExpr(to(equalsBoundNode("param")))))); - auto RefToParm = capturesVar( - varDecl(anyOf(hasSameNameAsBoundNode("param"), RefToParmImplicit))); + auto CapturedVar = varDecl(refersToBoundParm("param")); auto CaptureInRef = allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef), - unless(hasAnyCapture( - capturesVar(varDecl(hasSameNameAsBoundNode("param")))))); - auto CaptureByRefExplicit = hasAnyCapture( - allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm)); + unless(hasAnyCapture(capturesVar(CapturedVar)))); + auto CaptureByRefExplicit = hasAnyCapture(allOf( + hasCaptureKind(LambdaCaptureKind::LCK_ByRef), capturesVar(CapturedVar))); auto CapturedInBody = lambdaExpr(anyOf(CaptureInRef, CaptureByRefExplicit)); auto IsBoundCall = ignoringParenImpCasts(equalsBoundNode("call")); @@ -118,25 +119,21 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { parenListExpr(has(expr(IsBoundCall)))))))); auto CapturedInLambda = hasDeclContext(cxxRecordDecl( - isLambda(), - hasParent(lambdaExpr(forCallable(equalsBoundNode("func")), - anyOf(CapturedInCaptureList, CapturedInBody))))); + isLambda(), hasParent(lambdaExpr( + anyOf(CapturedInCaptureList, CapturedInBody), + hasAncestor(functionDecl(equalsBoundNode("func"))))))); auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); - auto ForwardCallMatcher = callExpr( - callExpr().bind("call"), argumentCountIs(1), - hasArgument(0, declRefExpr(to(varDecl().bind("var")))), - forCallable( - anyOf(allOf(equalsBoundNode("func"), - functionDecl(hasAnyParameter(parmVarDecl(allOf( - equalsBoundNode("param"), equalsBoundNode("var")))))), - CapturedInLambda)), - callee(unresolvedLookupExpr(hasAnyDeclaration( - namedDecl(hasUnderlyingDecl(hasName(ForwardFunction)))))), - - unless(anyOf(hasAncestor(typeLoc()), - hasAncestor(expr(hasUnevaluatedContext()))))); + auto ForwardCallMatcher = + callExpr(callExpr().bind("call"), argumentCountIs(1), + hasArgument(0, declRefExpr(to(CapturedVar)).bind("var")), + forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)), + callee(unresolvedLookupExpr(hasAnyDeclaration( + namedDecl(hasUnderlyingDecl(hasName(ForwardFunction)))))), + + unless(anyOf(hasAncestor(typeLoc()), + hasAncestor(expr(hasUnevaluatedContext()))))); Finder->addMatcher( parmVarDecl( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e40150f3603da..7b423990ae77b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -461,7 +461,12 @@ Changes in existing checks Objective-C for-in loop variable declaration. - Improved :doc:`cppcoreguidelines-missing-std-forward - <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check: + <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by: + + - Correctly handling forwarding in deeply nested lambdas. + + - Fixed false negative when multiple parameters are used in a lambda and + only some of them are forwarded. - Fixed false positive for constrained template parameters diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp index dbb40b77f2dca..136e4af7c8ee5 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp @@ -103,6 +103,36 @@ void foo(X &&x, Y &&y) { use(y); } +template <typename T> +void nested_but_no_forward(T &&arg) { + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: forwarding reference parameter 'arg' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&]() + { + [&]() + { consumes_all(arg); }(); + }(); +} + +template <typename T, typename U> +void nested_forward_only_one(T &&arg1, U &&arg2) { + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: forwarding reference parameter 'arg2' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&]() + { + [&]() + { consumes_all(std::forward<T>(arg1)); }(); + }(); +} + +template <typename T> +void nested_rename_no_forward(T &&t) { + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&x = t]() { + [&y = x]() { + (void)y; + }(); + }(); +} + } // namespace positive_cases namespace negative_cases { @@ -180,6 +210,36 @@ void lambda_value_reference_auxiliary_var(T&& t) { [&x = t]() { T other = std::forward<T>(x); }; } +template <class T> +void lambda_multi_level_rename(T &&t) { + [&x = t]() { + [&y = x]() { + T other = std::forward<T>(y); + }(); + }(); +} + +template <class T> +void lambda_implicit_and_rename(T &&t) { + [&]() { + [&y = t]() { + T other = std::forward<T>(y); + }(); + }(); +} + +template <typename T> +void nested_forward(T &&arg) { + [&]() { [&]() { consumes_all(std::forward<T>(arg)); }(); }(); +} + +template <typename T> +void triple_nested_forward(T &&arg) { + [&]() { + [&]() { [&]() { consumes_all(std::forward<T>(arg)); }(); }(); + }(); +} + template <typename T> void lambda_value_reference_capture_list_brace_init(T&& t) { [t2{std::forward<T>(t)}] { t2(); }; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
