diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp index 0b85ea19735ee..370de12999ace 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()) { + if (!isa(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 4d25e2ebe85f5..49ac70d999851 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -295,6 +295,10 @@ Changes in existing checks coroutine functions and increase issue detection for cases involving type aliases with references. +- Improved :doc:`cppcoreguidelines-missing-std-forward + ` check to + address false positives in the capture list and body of lambdas. + - Improved :doc:`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 b9720db272e40..443f338ba2046 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 -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); }; +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); }; } } // namespace positive_cases @@ -147,4 +147,29 @@ class AClass { T data; }; +template +void lambda_value_reference(T&& t) { + [&]() { T other = std::forward(t); }; +} + +template +void lambda_value_reference_capture_list_ref_1(T&& t) { + [=, &t] { T other = std::forward(t); }; +} + +template +void lambda_value_reference_capture_list_ref_2(T&& t) { + [&t] { T other = std::forward(t); }; +} + +template +void lambda_value_reference_capture_list(T&& t) { + [t = std::forward(t)] { t(); }; +} + +template +void lambda_value_reference_auxiliary_var(T&& t) { + [&x = t]() { T other = std::forward(x); }; +} + } // namespace negative_cases