diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp index c633683570f74..ecfad9cd44bfb 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp @@ -9,7 +9,6 @@ #include "MissingStdForwardCheck.h" #include "../utils/Matchers.h" #include "clang/AST/ASTContext.h" -#include "clang/AST/ExprConcepts.h" #include "clang/ASTMatchers/ASTMatchFinder.h" using namespace clang::ast_matchers; @@ -53,68 +52,22 @@ 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))))); + hasParent( + lambdaExpr(forCallable(equalsBoundNode("func")), + hasAnyCapture(capturesVar(varDecl(hasInitializer( + ignoringParenImpCasts(equalsBoundNode("call")))))))))); auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param"))); auto ForwardCallMatcher = callExpr( callExpr().bind("call"), argumentCountIs(1), hasArgument( - 0, declRefExpr(to( - varDecl(optionally(equalsBoundNode("param"))).bind("var")))), + 0, declRefExpr(to(varDecl(equalsBoundNode("param")).bind("var")))), forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)), callee(unresolvedLookupExpr(hasAnyDeclaration( namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))), diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0d2467210fc66..feac60491dda8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,7 +142,9 @@ Changes in existing checks - Improved :doc:`cppcoreguidelines-missing-std-forward ` check by no longer - giving false positives for deleted functions. + giving false positives for deleted functions, avoiding false negative when + multiple arguments in a function, reporting diagnostics when using forward in + body of lambdas. - Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer ` 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 20e43f04180ff..3bf16a401682d 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 @@ -75,6 +75,11 @@ class AClass { T data; }; +template void foo(X &&x, Y &&y) { + // CHECK-MESSAGES: :[[@LINE-1]]:55: warning: forwarding reference parameter 'y' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + auto newx = std::forward(x); +} + template void does_not_forward_in_evaluated_code(T&& t) { // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] @@ -90,9 +95,27 @@ void lambda_value_capture(T&& t) { } template -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); }; +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); }; +} + +template +void lambda_value_reference_capture_list_ref_1(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [=, &t] { T other = std::forward(t); }; +} + +template +void lambda_value_reference_capture_list_ref_2(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&t] { T other = std::forward(t); }; +} + +template +void lambda_value_reference_auxiliary_var(T&& t) { + // CHECK-MESSAGES: :[[@LINE-1]]:47: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward] + [&x = t]() { T other = std::forward(x); }; } } // namespace positive_cases @@ -147,31 +170,11 @@ 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 namespace deleted_functions {