https://github.com/rturrado updated https://github.com/llvm/llvm-project/pull/180806
>From 0d1f413716bd3f3479a1f4f50b21f2bb386a5ad5 Mon Sep 17 00:00:00 2001 From: rturrado <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> --- .../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 <[email protected]> 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 <[email protected]> --- .../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 <[email protected]> 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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
