Index: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -63,6 +63,31 @@ return false; } +bool isPassedToStdMove(const ParmVarDecl &Param, ASTContext &Context) { + // Check if the parameter has a name, in case of functions like - + // void func(const ExpensiveToCopyType) + // { + // + // } + // The function having an empty body will still have a FunctionDecl and its + // parmVarDecl picked up by this checker. It will be an empty string and will + // lead to an assertion failure when using hasName(std::string) being used + // in the matcher below. If empty then exit indicating no move calls present + // for the function argument being examined. + llvm::StringRef ParamName = Param.getName(); + + if (ParamName.empty()) { + return false; + } + auto Matches = match( + callExpr( + callee(functionDecl(hasName("::std::move"))), argumentCountIs(1), + hasArgument(0, declRefExpr(to(parmVarDecl(hasName(ParamName)))))), + Context); + + return !Matches.empty(); +} + } // namespace UnnecessaryValueParamCheck::UnnecessaryValueParamCheck( @@ -103,6 +128,9 @@ if (Analyzer.isMutated(Param)) return; + if (isPassedToStdMove(*Param, *Result.Context)) + return; + const bool IsConstQualified = Param->getType().getCanonicalType().isConstQualified(); Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp =================================================================== --- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp +++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param.cpp @@ -29,6 +29,36 @@ const_iterator end() const; }; +// Mocking std::move() and std::remove_reference (since move() relies on that) +// Since the regression tests run with -nostdinc++, standard library utilities have +// to be mocked. Over here the mocking is required for tests that have function arguments +// being moved. +namespace std { +template +struct remove_reference; + +template +struct remove_reference { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &> { + typedef _Tp type; +}; + +template +struct remove_reference<_Tp &&> { + typedef _Tp type; +}; + +template +constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) { + return static_cast::type &&>(__t); +} + +} // namespace std + // This class simulates std::pair<>. It is trivially copy constructible // and trivially destructible, but not trivially copy assignable. class SomewhatTrivial { @@ -55,6 +85,16 @@ ~ExpensiveMovableType(); }; +template +struct UsesExpensiveToCopyType { + Arg arg; + ExpensiveToCopyType expensiveType; + + UsesExpensiveToCopyType() = default; + UsesExpensiveToCopyType(ExpensiveToCopyType eType) : expensiveType{std::move(eType)} {} + UsesExpensiveToCopyType(ExpensiveToCopyType eType, Arg t) : expensiveType{std::move(eType)}, arg{std::move(t)} {} +}; + void positiveExpensiveConstValue(const ExpensiveToCopyType Obj); // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj); void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) { @@ -166,6 +206,20 @@ Obj.nonConstMethod(); } +void negativeNoConstRefSinceMoved(ExpensiveToCopyType arg) { + auto F = std::move(arg); +} + +template +T *negativeNoConstRefSinceTypeMoved(ExpensiveToCopyType t) { + return new T(std::move(t)); +} + +template +UsesExpensiveToCopyType negativeCreate(ExpensiveToCopyType eType) { + return UsesExpensiveToCopyType(std::move(eType)); +} + struct PositiveValueUnusedConstructor { PositiveValueUnusedConstructor(ExpensiveToCopyType Copy) {} // CHECK-MESSAGES: [[@LINE-1]]:54: warning: the parameter 'Copy'