Skip to content

[clang-tidy][missing-std-forward]report diagnotics for using forward in lambda body #83930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[clang-tidy][missing-std-forward]report diagnotics for using forward in lambda body #83930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 5 additions & 52 deletions clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")))))),
Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
Expand Down
49 changes: 26 additions & 23 deletions clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down