[clang-tools-extra] [clang-tidy][missing-std-forward]report diagnotics for using forward in lambda body (PR #83930)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 4 15:42:44 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Congcong Cai (HerrCai0907)
<details>
<summary>Changes</summary>
Fixes: #<!-- -->83845
Capturing variable in lambda by reference and doing forward in lambda body are like constructing a struct by reference and using std::forward in some member function. It is dangerous if the lifetime of lambda expression is longer then current function. And it goes against what this check is intended to check.
This PR wants to revert this behavior introduced in #<!-- -->77056 partially.
---
Full diff: https://github.com/llvm/llvm-project/pull/83930.diff
3 Files Affected:
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp (+5-52)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-1)
- (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp (+26-23)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
index c633683570f748..ecfad9cd44bfb1 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<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)))));
+ 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 0d2467210fc664..feac60491dda8e 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
<clang-tidy/checks/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
<clang-tidy/checks/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 20e43f04180ff3..3bf16a401682d5 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 <typename X, typename Y> 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>(x);
+}
+
template <class T>
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 <class 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); };
+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); };
+}
+
+template<typename T>
+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>(t); };
+}
+
+template<typename T>
+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>(t); };
+}
+
+template <class T>
+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<T>(x); };
}
} // namespace positive_cases
@@ -147,31 +170,11 @@ 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
namespace deleted_functions {
``````````
</details>
https://github.com/llvm/llvm-project/pull/83930
More information about the cfe-commits
mailing list