https://github.com/rturrado updated https://github.com/llvm/llvm-project/pull/180806
>From 349ff5424c9a1687c5497de9160dfed72fc8f248 Mon Sep 17 00:00:00 2001 From: rturrado <[email protected]> Date: Tue, 10 Feb 2026 19:37:04 +0100 Subject: [PATCH] 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 | 27 +++++++++++++++++++ .../clang-tidy/utils/DeclRefExprUtils.h | 5 ++++ .../performance/unnecessary-value-param.cpp | 24 +++++++++++++++++ 4 files changed, 65 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..d53d4d5598dd5 100644 --- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp @@ -415,4 +415,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); + for (const auto &M : Matches) { + if (const auto *P = M.getNodeAs<ParmVarDecl>("param")) { + if (P->getType()->isRValueReferenceType()) + return true; + if (const auto *Function = dyn_cast<FunctionDecl>(P->getDeclContext())) { + if (const auto *Pattern = Function->getTemplateInstantiationPattern()) { + 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)); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
