https://github.com/jcsxky created https://github.com/llvm/llvm-project/pull/77056
Parameter variable which is forwarded in lambda capture list or in body by reference is reasonable and current version of this check produces false positive on these cases. This patch try to fix the [issue](https://github.com/llvm/llvm-project/issues/68105) >From 799efba6c8fd3acfc96db8b5b2185bcd93aecb84 Mon Sep 17 00:00:00 2001 From: huqizhi <huqi...@feysh.com> Date: Wed, 3 Jan 2024 09:44:26 +0800 Subject: [PATCH] [clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward --- .../MissingStdForwardCheck.cpp | 73 +++++++++++++++++-- .../cppcoreguidelines/missing-std-forward.cpp | 26 ++++++- 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp index 0b85ea19735eef..f0e021039f094d 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -53,18 +53,76 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) { FuncTemplate->getTemplateParameters()->getDepth(); } +AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) { + const auto &Name = Node.getNameAsString(); + + return Builder->removeBindings( + [this, Name](const ast_matchers::internal::BoundNodesMap &Nodes) { + const auto &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; +} + +AST_MATCHER_P(LambdaExpr, hasCaptureDefaultKind, LambdaCaptureDefault, Kind) { + return Node.getCaptureDefault() == Kind; +} + +AST_MATCHER(LambdaExpr, hasCaptureToParm) { + auto RefToParm = capturesVar(varDecl(hasSameNameAsBoundNode("param"))); + auto HasRefToParm = hasAnyCapture(RefToParm); + + auto CaptureInRef = + allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef), + unless(HasRefToParm)); + auto CaptureInCopy = allOf( + hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByCopy), HasRefToParm); + auto CaptureByRefExplicit = hasAnyCapture( + allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm)); + + auto Captured = + lambdaExpr(anyOf(CaptureInRef, CaptureInCopy, CaptureByRefExplicit)); + if (Captured.matches(Node, Finder, Builder)) + return true; + + return false; +} + +AST_MATCHER(CallExpr, forCallableNode) { + auto InvokeInCaptureList = hasAnyCapture(capturesVar( + varDecl(hasInitializer(ignoringParenImpCasts(equalsNode(&Node)))))); + auto InvokeInLambda = hasDeclContext(cxxRecordDecl( + isLambda(), + hasParent(lambdaExpr(forCallable(equalsBoundNode("func")), + anyOf(InvokeInCaptureList, hasCaptureToParm()))))); + + if (forCallable(anyOf(equalsBoundNode("func"), InvokeInLambda)) + .matches(Node, Finder, Builder)) + return true; + + return false; +} + } // namespace void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); - auto ForwardCallMatcher = callExpr( - forCallable(equalsBoundNode("func")), argumentCountIs(1), - callee(unresolvedLookupExpr(hasAnyDeclaration( - namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), - hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")), - unless(anyOf(hasAncestor(typeLoc()), - hasAncestor(expr(hasUnevaluatedContext()))))); + auto ForwardCallMatcher = + callExpr(forCallableNode(), argumentCountIs(1), + callee(unresolvedLookupExpr(hasAnyDeclaration( + namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), + hasArgument(0, declRefExpr(to(equalsBoundNode("param")))), + unless(anyOf(hasAncestor(typeLoc()), + hasAncestor(expr(hasUnevaluatedContext()))))); Finder->addMatcher( parmVarDecl(parmVarDecl().bind("param"), isTemplateTypeParameter(), @@ -76,6 +134,7 @@ void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) { } void MissingStdForwardCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param"); if (!Param) 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 b9720db272e406..55d6be743c22ab 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 @@ -90,9 +90,9 @@ void lambda_value_capture(T&& t) { } template <class T> -void lambda_value_reference(T&& t) { - // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] - [&]() { T other = std::forward<T>(t); }; +void lambda_value_capture_copy(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&,t]() { T other = std::forward<T>(t); }; } } // namespace positive_cases @@ -147,4 +147,24 @@ class AClass { T data; }; +template <class T> +void lambda_value_reference(T&& t) { + [&]() { T other = std::forward<T>(t); }; +} + +template<typename T> +void lambda_value_reference_capture_list_ref_1(T&& t) { + [=, &t] { T other = std::forward<T>(t); }; +} + +template<typename T> +void lambda_value_reference_capture_list_ref_2(T&& t) { + [&t] { T other = std::forward<T>(t); }; +} + +template<typename T> +void lambda_value_reference_capture_list(T&& t) { + [t = std::forward<T>(t)] { t(); }; +} + } // namespace negative_cases _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
