[clang-tools-extra] [clang-tidy] Fix `performance-unnecessary-value-param` FN when passing `std::function` rvalue into `std::map::try_emplace` (PR #180806)
Roberto Turrado Camblor via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 12 09:23:08 PST 2026
https://github.com/rturrado updated https://github.com/llvm/llvm-project/pull/180806
>From 0d1f413716bd3f3479a1f4f50b21f2bb386a5ad5 Mon Sep 17 00:00:00 2001
From: rturrado <rturrado at gmail.com>
Date: Tue, 10 Feb 2026 19:37:04 +0100
Subject: [PATCH 1/6] Initial code
Add a check for handling a move fix when the argument is trivially movable and perfectly forwarded.
Add a utils::decl_ref_expr::isPerfectlyForwardedArgument.
Add a PositiveMoveOnPassAsUniversalReference test.
---
.../UnnecessaryValueParamCheck.cpp | 13 ++++++---
.../clang-tidy/utils/DeclRefExprUtils.cpp | 28 +++++++++++++++++++
.../clang-tidy/utils/DeclRefExprUtils.h | 5 ++++
.../performance/unnecessary-value-param.cpp | 24 ++++++++++++++++
4 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index ffb2a81b862f6..715e1aca7bee9 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -95,14 +95,19 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
if (AllDeclRefExprs.size() == 1) {
auto CanonicalType = Param->getType().getCanonicalType();
const auto &DeclRefExpr = **AllDeclRefExprs.begin();
+ auto IsPerfectlyForwardedArg =
+ utils::decl_ref_expr::isPerfectlyForwardedArgument(
+ DeclRefExpr, *Function, *Result.Context);
if (!hasLoopStmtAncestor(DeclRefExpr, *Function, *Result.Context) &&
((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
- utils::decl_ref_expr::isCopyConstructorArgument(
- DeclRefExpr, *Function, *Result.Context)) ||
+ (utils::decl_ref_expr::isCopyConstructorArgument(
+ DeclRefExpr, *Function, *Result.Context) ||
+ IsPerfectlyForwardedArg)) ||
(utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) &&
- utils::decl_ref_expr::isCopyAssignmentArgument(
- DeclRefExpr, *Function, *Result.Context)))) {
+ (utils::decl_ref_expr::isCopyAssignmentArgument(
+ DeclRefExpr, *Function, *Result.Context) ||
+ IsPerfectlyForwardedArg)))) {
handleMoveFix(*Param, DeclRefExpr, *Result.Context);
return;
}
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 3510b3546bbba..06aafcd4f13de 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -12,6 +12,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/ExprCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
#include <cassert>
namespace clang::tidy::utils::decl_ref_expr {
@@ -415,4 +416,31 @@ bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
return !Matches.empty();
}
+bool isPerfectlyForwardedArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
+ ASTContext &Context) {
+ auto UsedAsArg = forEachArgumentWithParam(
+ ignoringParenImpCasts(declRefExpr(equalsNode(&DeclRef))),
+ parmVarDecl().bind("param"));
+ auto Matches =
+ match(decl(hasDescendant(invocation(UsedAsArg).bind("invocationExpr"))),
+ Decl, Context);
+ return std::any_of(Matches.begin(), Matches.end(), [](const auto &M) {
+ if (const auto *P = M.template getNodeAs<ParmVarDecl>("param")) {
+ if (P->getType()->isRValueReferenceType())
+ return true;
+ if (const auto *Function = dyn_cast<FunctionDecl>(P->getDeclContext())) {
+ if (const auto *Pattern = Function->getTemplateInstantiationPattern()) {
+ const unsigned Index = P->getFunctionScopeIndex();
+ if (Index < Pattern->getNumParams()) {
+ const ParmVarDecl *PatternParam = Pattern->getParamDecl(Index);
+ if (PatternParam->getType()->isRValueReferenceType())
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ });
+}
+
} // namespace clang::tidy::utils::decl_ref_expr
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
index 794adc04dc478..eb71892228bd8 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
@@ -61,6 +61,11 @@ bool isCopyConstructorArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
ASTContext &Context);
+/// Returns ``true`` if ``DeclRefExpr`` is perfectly forwarded to a call
+/// expression within ``Decl``.
+bool isPerfectlyForwardedArgument(const DeclRefExpr &DeclRef, const Decl &Decl,
+ ASTContext &Context);
+
} // namespace clang::tidy::utils::decl_ref_expr
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
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 8dc13d3ed7f85..54bf30e6e518f 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
@@ -25,6 +25,11 @@ template <typename _Tp>
constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) {
return static_cast<typename std::remove_reference<_Tp>::type &&>(__t);
}
+
+template<typename _Tp>
+_Tp&& forward(typename std::remove_reference<_Tp>::type &t) {
+ return static_cast<_Tp&&>(t);
+}
} // namespace std
struct ExpensiveToCopyType {
@@ -387,3 +392,22 @@ template <typename T>
void NegativeCallWithDependentAndNondependentArgs(ExpensiveMovableType a, T b) {
B::bar(std::move(a), b);
}
+
+// This function template tries to mimic std::map.try_emplace behavior
+// It uses perfect forwarding to then move the arguments just sometimes
+template <typename T>
+void doNotAlwaysMove(T &&a) {
+ static bool shouldMove = false;
+ if (shouldMove) {
+ T b = std::forward<T>(a);
+ } else {
+ T b = a;
+ }
+ shouldMove = !shouldMove;
+}
+
+void PositiveMoveOnPassAsUniversalReference(ExpensiveMovableType a) {
+ doNotAlwaysMove(a);
+ // CHECK-MESSAGES: [[@LINE-1]]:19: warning: parameter 'a' of type 'ExpensiveMovableType' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]
+ // CHECK-FIXES: doNotAlwaysMove(std::move(a));
+}
>From ba0cd174f7dbd02b37b8693514293b32583e0a69 Mon Sep 17 00:00:00 2001
From: rturrado <rturrado at gmail.com>
Date: Tue, 10 Feb 2026 21:21:13 +0100
Subject: [PATCH 2/6] Update release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 04c970402d4e1..19a729daa5f56 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -172,6 +172,10 @@ Changes in existing checks
<clang-tidy/checks/performance/move-const-arg>` check by avoiding false
positives on trivially copyable types with a non-public copy constructor.
+- Improved :doc:`performance-unnecessary-value-param
+ <clang-tidy/checks/performance/unnecessary-value-param>` check to suggest
+ moving arguments that are passed to perfectly forwarding parameters.
+
- Improved :doc:`readability-enum-initial-value
<clang-tidy/checks/readability/enum-initial-value>` check: the warning message
now uses separate note diagnostics for each uninitialized enumerator, making
>From 5a7b58c1da80a87b80f767bbf49fa3c07c71cd51 Mon Sep 17 00:00:00 2001
From: rturrado <rturrado at gmail.com>
Date: Tue, 10 Feb 2026 21:41:14 +0100
Subject: [PATCH 3/6] Update docs
---
.../performance/unnecessary-value-param.rst | 37 +++++++++++++++++--
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
index cd25d7d94d99b..0b681a6904c5d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
@@ -32,9 +32,12 @@ Example:
ExpensiveToCopy Copy(Value);
}
-If the parameter is not const, only copied or assigned once and has a
-non-trivial move-constructor or move-assignment operator respectively the check
-will suggest to move it.
+If the parameter is not const, and only copied or assigned once, the check will
+suggest a move in the following scenarios:
+
+1. the parameter has a non-trivial move-constructor or move-assignment operator
+ respectively;
+2. the parameter is passed to a function accepting it as a universal reference.
Example:
@@ -54,6 +57,34 @@ Will become:
Field = std::move(Value);
}
+Example:
+
+.. code-block:: c++
+
+ template <typename T>
+ void setValue(T &&Value) {
+ Field = std::forward<T>(Value);
+ }
+
+ void bar(string Value) {
+ setValue(Value);
+ }
+
+Will become:
+
+.. code-block:: c++
+
+ #include <utility>
+
+ template <typename T>
+ void setValue(T &&Value) {
+ Field = std::forward<T>(Value);
+ }
+
+ void bar(string Value) {
+ setValue(std::move(Value);
+ }
+
Because the fix-it needs to change the signature of the function, it may break
builds if the function is used in multiple translation units or some codes
depends on function signatures.
>From b9d7ef5fc7326512a6fc2a38e8f27d0ad35f8961 Mon Sep 17 00:00:00 2001
From: Roberto Turrado Camblor <rturrado at gmail.com>
Date: Wed, 11 Feb 2026 21:45:13 +0100
Subject: [PATCH 4/6] Update
clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
Co-authored-by: Yat Ho <lagoho7 at gmail.com>
---
.../clang-tidy/checks/performance/unnecessary-value-param.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
index 0b681a6904c5d..a93038b48aaf1 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
@@ -82,7 +82,7 @@ Will become:
}
void bar(string Value) {
- setValue(std::move(Value);
+ setValue(std::move(Value));
}
Because the fix-it needs to change the signature of the function, it may break
>From 2d8e57d23e5ae4188edb8b746727480e1b2a852d Mon Sep 17 00:00:00 2001
From: Roberto Turrado Camblor <rturrado at gmail.com>
Date: Wed, 11 Feb 2026 21:45:30 +0100
Subject: [PATCH 5/6] Update
clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
Co-authored-by: EugeneZelenko <eugene.zelenko at gmail.com>
---
.../clang-tidy/checks/performance/unnecessary-value-param.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
index a93038b48aaf1..d83b0fcfa0a16 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
@@ -32,7 +32,7 @@ Example:
ExpensiveToCopy Copy(Value);
}
-If the parameter is not const, and only copied or assigned once, the check will
+If the parameter is not ``const``, and only copied or assigned once, the check will
suggest a move in the following scenarios:
1. the parameter has a non-trivial move-constructor or move-assignment operator
>From a1a9dcb7fe6011125b33cbc8660cf685dbadcfeb Mon Sep 17 00:00:00 2001
From: rturrado <rturrado at gmail.com>
Date: Thu, 12 Feb 2026 18:22:47 +0100
Subject: [PATCH 6/6] Fix linters
---
.../clang-tidy/checks/performance/unnecessary-value-param.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
index d83b0fcfa0a16..d4e043dba93aa 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst
@@ -32,8 +32,8 @@ Example:
ExpensiveToCopy Copy(Value);
}
-If the parameter is not ``const``, and only copied or assigned once, the check will
-suggest a move in the following scenarios:
+If the parameter is not ``const``, and only copied or assigned once, the check
+will suggest a move in the following scenarios:
1. the parameter has a non-trivial move-constructor or move-assignment operator
respectively;
More information about the cfe-commits
mailing list