[clang-tools-extra] [clang-tidy] unnecessary-value-param: Allow moving of value arguments. (PR #145871)
Clement Courbet via cfe-commits
cfe-commits at lists.llvm.org
Thu Jun 26 05:06:01 PDT 2025
https://github.com/legrosbuffle updated https://github.com/llvm/llvm-project/pull/145871
>From 8ffb4e7f1c3855917ccc5197bb3ecea7c716ab8a Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Thu, 26 Jun 2025 11:22:28 +0000
Subject: [PATCH 1/3] [clang-tidy] unnecessary-value-param: Allow moving of
value arguments.
Some functions take an argument by value because it allows efficiently
moving them to a function that takes by value. Do not pessimize that
case.
---
.../performance/UnnecessaryValueParamCheck.cpp | 16 ++++++++++++++++
clang-tools-extra/docs/ReleaseNotes.rst | 1 +
.../performance/unnecessary-value-param.cpp | 4 ++++
3 files changed, 21 insertions(+)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index d89c3a69fc841..d18a3c914bf93 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -41,6 +41,17 @@ bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
return Matches.empty();
}
+bool isArgOfStdMove(const DeclRefExpr &DeclRef, const Decl &Decl,
+ ASTContext &Context) {
+ auto Matches = match(
+ traverse(TK_AsIs, decl(forEachDescendant(callExpr(
+ callee(functionDecl(hasName("std::move"))),
+ hasArgument(0, ignoringParenImpCasts(declRefExpr(
+ equalsNode(&DeclRef)))))))),
+ Decl, Context);
+ return Matches.empty();
+}
+
} // namespace
UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@@ -100,6 +111,11 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
auto CanonicalType = Param->getType().getCanonicalType();
const auto &DeclRefExpr = **AllDeclRefExprs.begin();
+ // The reference is in a call to `std::move`, do not warn.
+ if (isArgOfStdMove(DeclRefExpr, *Function, *Result.Context)) {
+ return;
+ }
+
if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
utils::decl_ref_expr::isCopyConstructorArgument(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 934d52086b3b9..2413b742dc480 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -294,6 +294,7 @@ Changes in existing checks
to avoid matching usage of functions within the current compilation unit.
Added an option `IgnoreCoroutines` with the default value `true` to
suppress this check for coroutines where passing by reference may be unsafe.
+ Fix false positive on by-value parameters that are only moved.
- Improved :doc:`readability-convert-member-functions-to-static
<clang-tidy/checks/readability/convert-member-functions-to-static>` check by
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
index 88c491ea1eabc..a9af4f60561a4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
@@ -344,6 +344,10 @@ void ReferenceFunctionByCallingIt() {
PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType());
}
+void NegativeMoved(ExpensiveToCopyType A) {
+ A Copy = std::move(A);
+}
+
// Virtual method overrides of dependent types cannot be recognized unless they
// are marked as override or final. Test that check is not triggered on methods
// marked with override or final.
>From 9ffc7ee7f73b53eaf0d8ae5818aa2679f833cfa1 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Thu, 26 Jun 2025 11:36:36 +0000
Subject: [PATCH 2/3] fix formatting
---
.../clang-tidy/performance/UnnecessaryValueParamCheck.cpp | 7 +++----
.../checkers/performance/unnecessary-value-param.cpp | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index d18a3c914bf93..8a984dd8c1212 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -43,13 +43,12 @@ bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
bool isArgOfStdMove(const DeclRefExpr &DeclRef, const Decl &Decl,
ASTContext &Context) {
- auto Matches = match(
- traverse(TK_AsIs, decl(forEachDescendant(callExpr(
+ return !match(
+ traverse(TK_AsIs, decl(hasDescendant(callExpr(
callee(functionDecl(hasName("std::move"))),
hasArgument(0, ignoringParenImpCasts(declRefExpr(
equalsNode(&DeclRef)))))))),
- Decl, Context);
- return Matches.empty();
+ Decl, Context).empty();
}
} // namespace
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
index a9af4f60561a4..8654cc7bf1539 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp
@@ -345,7 +345,7 @@ void ReferenceFunctionByCallingIt() {
}
void NegativeMoved(ExpensiveToCopyType A) {
- A Copy = std::move(A);
+ ExpensiveToCopyType Copy = std::move(A);
}
// Virtual method overrides of dependent types cannot be recognized unless they
>From 52f7cba764ee051707191f29768ce6a83eafc888 Mon Sep 17 00:00:00 2001
From: Clement Courbet <courbet at google.com>
Date: Thu, 26 Jun 2025 12:05:33 +0000
Subject: [PATCH 3/3] more formattinmore formatting
---
.../performance/UnnecessaryValueParamCheck.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index 8a984dd8c1212..7ba7b6c1f4684 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -43,12 +43,13 @@ bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
bool isArgOfStdMove(const DeclRefExpr &DeclRef, const Decl &Decl,
ASTContext &Context) {
- return !match(
- traverse(TK_AsIs, decl(hasDescendant(callExpr(
- callee(functionDecl(hasName("std::move"))),
- hasArgument(0, ignoringParenImpCasts(declRefExpr(
- equalsNode(&DeclRef)))))))),
- Decl, Context).empty();
+ return !match(traverse(TK_AsIs,
+ decl(hasDescendant(callExpr(
+ callee(functionDecl(hasName("std::move"))),
+ hasArgument(0, ignoringParenImpCasts(declRefExpr(
+ equalsNode(&DeclRef)))))))),
+ Decl, Context)
+ .empty();
}
} // namespace
More information about the cfe-commits
mailing list