Author: Qizhi Hu Date: 2024-01-06T19:50:00+08:00 New Revision: d08482924efe8b2c44913583af7b8f60a29975d1
URL: https://github.com/llvm/llvm-project/commit/d08482924efe8b2c44913583af7b8f60a29975d1 DIFF: https://github.com/llvm/llvm-project/commit/d08482924efe8b2c44913583af7b8f60a29975d1.diff LOG: [clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward (#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) Co-authored-by: huqizhi <836744...@qq.com> 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 0b85ea19735eef..370de12999aceb 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -53,16 +53,72 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) { FuncTemplate->getTemplateParameters()->getDepth(); } +AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) { + IdentifierInfo *II = Node.getIdentifier(); + if (nullptr == II) + return false; + 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; +} + +AST_MATCHER_P(LambdaExpr, hasCaptureDefaultKind, LambdaCaptureDefault, Kind) { + return Node.getCaptureDefault() == Kind; +} + } // 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 HasRefToParm = hasAnyCapture(RefToParm); + + auto CaptureInRef = + allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef), + unless(hasAnyCapture( + capturesVar(varDecl(hasSameNameAsBoundNode("param")))))); + auto CaptureInCopy = allOf( + hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByCopy), HasRefToParm); + auto CaptureByRefExplicit = hasAnyCapture( + allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm)); + + auto CapturedInBody = + lambdaExpr(anyOf(CaptureInRef, CaptureInCopy, CaptureByRefExplicit)); + auto CapturedInCaptureList = hasAnyCapture(capturesVar( + varDecl(hasInitializer(ignoringParenImpCasts(equalsBoundNode("call")))))); + + auto CapturedInLambda = hasDeclContext(cxxRecordDecl( + isLambda(), + hasParent(lambdaExpr(forCallable(equalsBoundNode("func")), + anyOf(CapturedInCaptureList, CapturedInBody))))); + auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); auto ForwardCallMatcher = callExpr( - forCallable(equalsBoundNode("func")), argumentCountIs(1), + callExpr().bind("call"), argumentCountIs(1), + hasArgument( + 0, declRefExpr(to( + varDecl(optionally(equalsBoundNode("param"))).bind("var")))), + forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)), callee(unresolvedLookupExpr(hasAnyDeclaration( namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), - hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")), + unless(anyOf(hasAncestor(typeLoc()), hasAncestor(expr(hasUnevaluatedContext()))))); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 08ade306b5a077..1bd5a72126c10b 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -301,6 +301,10 @@ Changes in existing checks coroutine functions and increase issue detection for cases involving type aliases with references. +- Improved :doc:`cppcoreguidelines-missing-std-forward + <clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check to + address false positives in the capture list and body of lambdas. + - Improved :doc:`cppcoreguidelines-narrowing-conversions <clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` check by extending the `IgnoreConversionFromTypes` option to include types without a 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..443f338ba2046a 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,29 @@ 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(); }; +} + +template <class T> +void lambda_value_reference_auxiliary_var(T&& t) { + [&x = t]() { T other = std::forward<T>(x); }; +} + } // namespace negative_cases _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
