Skip to content

[clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward #77056

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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())))));

Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
<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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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