https://github.com/legrosbuffle created
https://github.com/llvm/llvm-project/pull/145871
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.
>From 8ffb4e7f1c3855917ccc5197bb3ecea7c716ab8a Mon Sep 17 00:00:00 2001
From: Clement Courbet <cour...@google.com>
Date: Thu, 26 Jun 2025 11:22:28 +0000
Subject: [PATCH] [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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
