[clang-tidy] UnnecessaryValueParamCheck - suggest std::move() if non-const value parameter can be moved.

[flx]

Summary

Make check more useful in the following two cases:

1. The parameter is passed by non-const value, has a non-deleted move constructor and is only referenced once in the function as argument to the type's copy constructor.
2. The parameter is passed by non-const value, has a non-deleted move assignment operator and is only referenced once in the function as argument of the the type's copy assignment operator.

In this case suggest a fix to move the parameter which avoids the unnecessary copy and is closest to what the user might have intended.

[sbenza]

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

83 ↗

(On Diff #57309)

We should not call this more than once as it is a potentially expensive operation.
That is, isOnlyUsedAsConst will do it too.
Also, in this case we don't care about _all_. We only care about != 1.

144 ↗

(On Diff #57309)

Prefer two additions over a replacement.
They usually behave better in some conditions.

clang-tidy/utils/DeclRefExprUtils.cpp

102 ↗

(On Diff #57309)

why findAll() ?
You only care about one.

same as below.

clang-tidy/utils/TypeTraits.cpp

131 ↗

(On Diff #57309)

maybe also check that it is not trivial?

below too.

[alexfh]

clang-tidy/performance/UnnecessaryValueParamCheck.h

30 ↗

(On Diff #57309)

s/clang:://

[flx]

Thanks for the feedback!

[flx]

clang-tidy/utils/TypeTraits.cpp

131 ↗

(On Diff #57620)

Sorry I missed this. Will address it in the next revision.

[flx]

clang-tidy/utils/TypeTraits.cpp

131 ↗

(On Diff #58090)

Done. Shortened this now to just check whether the type has a non-trivial move constructor or assignment operator. For the test to work I had to make sure the constructors/operators there are also non-trivial.

[flx]

Upated documentation as well.

[Prazek]

Cool check! Did you think about sugesting std::move for rvalue references if they are used once?

[flx]

In D20277#436717, @Prazek wrote:

Cool check! Did you think about sugesting std::move for rvalue references if they are used once?

Thanks! I'm not sure this fits with what a user would expect from a check named "unnecessary-value-param" since in this case the parameter is not passed by value. Would a separate check make sense for this?

[flx]

This is ready for review again.

[flx]

Alex, Sam, could you take another look? It'd be great to get this change in to make the check more useful.

[Prazek]

In D20277#436725, @flx wrote:

In D20277#436717, @Prazek wrote:

Cool check! Did you think about sugesting std::move for rvalue references if they are used once?

Thanks! I'm not sure this fits with what a user would expect from a check named "unnecessary-value-param" since in this case the parameter is not passed by value. Would a separate check make sense for this?

Consider changing the name :) When I was thinking about the check for rvalue references, I wanted to name it "*-rvalue-ref-one-use".
If it covers values also, maybe "performance-move-single-used-variable" or "performance-variable-one-use", or "performance-disposable-object" (If I translated it correctly).

In my opinion there is no need to have separate check to to this, because the user would like to have both or none.

[flx]

In D20277#444023, @Prazek wrote:

In D20277#436725, @flx wrote:

In D20277#436717, @Prazek wrote:

Cool check! Did you think about sugesting std::move for rvalue references if they are used once?

Thanks! I'm not sure this fits with what a user would expect from a check named "unnecessary-value-param" since in this case the parameter is not passed by value. Would a separate check make sense for this?

Consider changing the name :) When I was thinking about the check for rvalue references, I wanted to name it "*-rvalue-ref-one-use".
If it covers values also, maybe "performance-move-single-used-variable" or "performance-variable-one-use", or "performance-disposable-object" (If I translated it correctly).

In my opinion there is no need to have separate check to to this, because the user would like to have both or none.

Renaming the check is a breaking change since it affects user's configurations. I'm working in a code base that doesn't allow r-value references outside of move constructors and assignment, but if you'd like to add handing the r-value reference case, that'd be great! I agree that with this change it make sense to handle r-value references here too.

[sbenza]

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

34 ↗

(On Diff #58093)

isSubset?

clang-tidy/utils/Matchers.cpp

16 ↗

(On Diff #58093)

Prefer AST_MATCHER_FUNCTION.
That macro will make sure to use a singleton matcher instead, since it has no arguments.

clang-tidy/utils/Matchers.h

44 ↗

(On Diff #58093)

isReferenceToConst

[flx]

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

34 ↗

(On Diff #58093)

In the caller code it seems to me more intuitive to say we're checking whether the set difference is empty. But no particularly strong feelings if you think isSubset is more clear.

[alexfh]

A few nits. Otherwise looks good, if Sam has no objections.

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

34 ↗

(On Diff #59167)

+1 to isSubset. And please add a comment explaining what should be a subset of what for this function to return true.

clang-tidy/utils/Matchers.h

50 ↗

(On Diff #59167)

How about using ast_matchers; in this function to make the statement easier to read?

clang-tidy/utils/TypeTraits.cpp

129 ↗

(On Diff #59167)

Maybe simplify this a bit?

return Record && Record->hasDefinition() && Record->hasNonTrivialMoveAssignment();

136 ↗

(On Diff #59167)

ditto

clang-tidy/utils/TypeTraits.h

32 ↗

(On Diff #59167)

Please use doxygen comments (///) and enclose inline code snippets in backquotes. The other function comments in this file should also be doxygen, I've fixed them in r272994.

[flx]

Thanks for the review. I addressed all remaining comments.

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

34 ↗

(On Diff #59167)

Omitted the comment by making the parameter names very explicit.

clang-tidy/utils/Matchers.h

50 ↗

(On Diff #59167)

Used using namespace ast_matchers;.

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h

 //===--- UnnecessaryValueParamCheck.h - clang-tidy---------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H
 
 #include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 /// \brief A check that flags value parameters of expensive to copy types that
 /// can safely be converted to const references.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html
 class UnnecessaryValueParamCheck : public ClangTidyCheck {
 public:
-  UnnecessaryValueParamCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UnnecessaryValueParamCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument,
+                     const ASTContext &Context);
+
+  std::unique_ptr<utils::IncludeInserter> Inserter;
+  const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

 //===--- UnnecessaryValueParamCheck.cpp - clang-tidy-----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "UnnecessaryValueParamCheck.h"
 
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
+#include "../utils/TypeTraits.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 namespace {
 
 std::string paramNameOrIndex(StringRef Name, size_t Index) {
   return (Name.empty() ? llvm::Twine('#') + llvm::Twine(Index + 1)
                        : llvm::Twine('\'') + Name + llvm::Twine('\''))
       .str();
 }
 
+template <typename S>
+bool isSubset(const S &SubsetCandidate, const S &SupersetCandidate) {
+  for (const auto &E : SubsetCandidate)
+    if (SupersetCandidate.count(E) == 0)
+      return false;
+  return true;
+}
+
 } // namespace
 
+UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+          Options.get("IncludeStyle", "llvm"))) {}
+
 void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) {
   const auto ExpensiveValueParamDecl =
       parmVarDecl(hasType(hasCanonicalType(allOf(matchers::isExpensiveToCopy(),
                                                  unless(referenceType())))),
                   decl().bind("param"));
   Finder->addMatcher(
       functionDecl(isDefinition(), unless(cxxMethodDecl(isOverride())),
                    unless(isInstantiated()),
void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
 
   // Skip declarations delayed by late template parsing without a body.
   if (!Function->getBody())
     return;
 
   // Do not trigger on non-const value parameters when:
   // 1. they are in a constructor definition since they can likely trigger
   //    misc-move-constructor-init which will suggest to move the argument.
-  // 2. they are not only used as const.
   if (!IsConstQualified && (llvm::isa<CXXConstructorDecl>(Function) ||
-                            !Function->doesThisDeclarationHaveABody() ||
-                            !utils::decl_ref_expr::isOnlyUsedAsConst(
-                                *Param, *Function->getBody(), *Result.Context)))
+                            !Function->doesThisDeclarationHaveABody()))
+    return;
+
+  auto AllDeclRefExprs = utils::decl_ref_expr::allDeclRefExprs(
+      *Param, *Function->getBody(), *Result.Context);
+  auto ConstDeclRefExprs = utils::decl_ref_expr::constReferenceDeclRefExprs(
+      *Param, *Function->getBody(), *Result.Context);
+  // 2. they are not only used as const.
+  if (!isSubset(AllDeclRefExprs, ConstDeclRefExprs))
     return;
+
+  // If the parameter is non-const, check if it has a move constructor and is
+  // only referenced once to copy-construct another object or whether it has a
+  // move assignment operator and is only referenced once when copy-assigned.
+  // In this case wrap DeclRefExpr with std::move() to avoid the unnecessary
+  // copy.
+  if (!IsConstQualified) {
+    auto CanonicalType = Param->getType().getCanonicalType();
+    if (AllDeclRefExprs.size() == 1 &&
+        ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
+          utils::decl_ref_expr::isCopyConstructorArgument(
+              **AllDeclRefExprs.begin(), *Function->getBody(),
+              *Result.Context)) ||
+         (utils::type_traits::hasNonTrivialMoveAssignment(CanonicalType) &&
+          utils::decl_ref_expr::isCopyAssignmentArgument(
+              **AllDeclRefExprs.begin(), *Function->getBody(),
+              *Result.Context)))) {
+      handleMoveFix(*Param, **AllDeclRefExprs.begin(), *Result.Context);
+      return;
+    }
+  }
+
   auto Diag =
       diag(Param->getLocation(),
            IsConstQualified ? "the const qualified parameter %0 is "
                               "copied for each invocation; consider "
                               "making it a reference"
                             : "the parameter %0 is copied for each "
                               "invocation but only used as a const reference; "
                               "consider making it a const reference")
       << paramNameOrIndex(Param->getName(), Index);
   // Do not propose fixes in macros since we cannot place them correctly.
   if (Param->getLocStart().isMacroID())
     return;
   for (const auto *FunctionDecl = Function; FunctionDecl != nullptr;
        FunctionDecl = FunctionDecl->getPreviousDecl()) {
     const auto &CurrentParam = *FunctionDecl->getParamDecl(Index);
     Diag << utils::fixit::changeVarDeclToReference(CurrentParam,
                                                            *Result.Context);
     if (!IsConstQualified)
       Diag << utils::fixit::changeVarDeclToConst(CurrentParam);
   }
 }
 
+void UnnecessaryValueParamCheck::registerPPCallbacks(
+    CompilerInstance &Compiler) {
+  Inserter.reset(new utils::IncludeInserter(
+      Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
+  Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
+}
+
+void UnnecessaryValueParamCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle",
+                utils::IncludeSorter::toString(IncludeStyle));
+}
+
+void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
+                                               const DeclRefExpr &CopyArgument,
+                                               const ASTContext &Context) {
+  auto Diag = diag(CopyArgument.getLocStart(),
+                   "parameter %0 is passed by value and only copied once; "
+                   "consider moving it to avoid unnecessary copies")
+              << &Var;
+  // Do not propose fixes in macros since we cannot place them correctly.
+  if (CopyArgument.getLocStart().isMacroID())
+    return;
+  const auto &SM = Context.getSourceManager();
+  auto EndLoc = Lexer::getLocForEndOfToken(CopyArgument.getLocation(), 0, SM, 
+             Context.getLangOpts());
+  Diag << FixItHint::CreateInsertion(CopyArgument.getLocStart(), "std::move(")
+       << FixItHint::CreateInsertion(EndLoc, ")");
+  if (auto IncludeFixit = Inserter->CreateIncludeInsertion(
+          SM.getFileID(CopyArgument.getLocStart()), "utility",
+          /*IsAngled=*/true))
+    Diag << *IncludeFixit;
+}
+
 } // namespace performance
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.h

 //===--- DeclRefExprUtils.h - clang-tidy-------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"
+#include "llvm/ADT/SmallPtrSet.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 namespace decl_ref_expr {
 
 /// \brief Returns true if all ``DeclRefExpr`` to the variable within ``Stmt``
 /// do not modify it.
 ///
 /// Returns ``true`` if only const methods or operators are called on the
 /// variable or the variable is a const reference or value argument to a
 /// ``callExpr()``.
 bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
                        ASTContext &Context);
 
+// Returns set of all DeclRefExprs to VarDecl in Stmt.
+llvm::SmallPtrSet<const DeclRefExpr *, 16>
+allDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context);
+
+// Returns set of all DeclRefExprs where VarDecl is guaranteed to be accessed in
+// a const fashion.
+llvm::SmallPtrSet<const DeclRefExpr *, 16>
+constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
+                           ASTContext &Context);
+
+// Returns true if DeclRefExpr is the argument of a copy-constructor call expr.
+bool isCopyConstructorArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                               ASTContext &Context);
+
+// Returns true if DeclRefExpr is the argument of a copy-assignment operator
+// call expr.
+bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                              ASTContext &Context);
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H

clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp

 //===--- DeclRefExprUtils.cpp - clang-tidy---------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "DeclRefExprUtils.h"
+#include "Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "llvm/ADT/SmallPtrSet.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 namespace decl_ref_expr {
 
 using namespace ::clang::ast_matchers;
 using llvm::SmallPtrSet;

 // Extracts all Nodes keyed by ID from Matches and inserts them into Nodes.
 template <typename Node>
 void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
                         SmallPtrSet<const Node *, 16> &Nodes) {
   for (const auto &Match : Matches)
     Nodes.insert(Match.getNodeAs<Node>(ID));
 }
 
-// Finds all DeclRefExprs to VarDecl in Stmt.
-SmallPtrSet<const DeclRefExpr *, 16>
-declRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
-  auto Matches = match(
-      findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
-      Stmt, Context);
-  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  return DeclRefs;
-}
+} // namespace
 
 // Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
 // is the a const reference or value argument to a CallExpr or CXXConstructExpr.
 SmallPtrSet<const DeclRefExpr *, 16>
 constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
                            ASTContext &Context) {
   auto DeclRefToVar =
       declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
   Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   Matches =
       match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   return DeclRefs;
 }
 
-} // namespace
-
 bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
                        ASTContext &Context) {
   // Collect all DeclRefExprs to the loop variable and all CallExprs and
   // CXXConstructExprs where the loop variable is used as argument to a const
   // reference parameter.
   // If the difference is empty it is safe for the loop variable to be a const
   // reference.
-  auto AllDeclRefs = declRefExprs(Var, Stmt, Context);
+  auto AllDeclRefs = allDeclRefExprs(Var, Stmt, Context);
   auto ConstReferenceDeclRefs = constReferenceDeclRefExprs(Var, Stmt, Context);
   return isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs);
 }
 
+SmallPtrSet<const DeclRefExpr *, 16>
+allDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
+  auto Matches = match(
+      findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
+      Stmt, Context);
+  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  return DeclRefs;
+}
+
+bool isCopyConstructorArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                               ASTContext &Context) {
+  auto UsedAsConstRefArg = forEachArgumentWithParam(
+      declRefExpr(equalsNode(&DeclRef)),
+      parmVarDecl(hasType(matchers::isReferenceToConst())));
+  auto Matches = match(
+      stmt(hasDescendant(
+          cxxConstructExpr(UsedAsConstRefArg, hasDeclaration(cxxConstructorDecl(
+                                                  isCopyConstructor())))
+              .bind("constructExpr"))),
+      Stmt, Context);
+  return !Matches.empty();
+}
+
+bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                              ASTContext &Context) {
+  auto UsedAsConstRefArg = forEachArgumentWithParam(
+      declRefExpr(equalsNode(&DeclRef)),
+      parmVarDecl(hasType(matchers::isReferenceToConst())));
+  auto Matches = match(
+      stmt(hasDescendant(
+          cxxOperatorCallExpr(UsedAsConstRefArg, hasOverloadedOperatorName("="))
+              .bind("operatorCallExpr"))),
+      Stmt, Context);
+  return !Matches.empty();
+}
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

 
 AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
   return utils::type_traits::recordIsTriviallyDefaultConstructible(
       Node, Finder->getASTContext());
 }
 
 AST_MATCHER(FieldDecl, isBitfield) { return Node.isBitField(); }
 
+// Returns QualType matcher for references to const.
+AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isReferenceToConst) {
+  using namespace ast_matchers;
+  return referenceType(pointee(qualType(isConstQualified())));
+}
+
 } // namespace matchers
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H

clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.h

 
 /// Returns `true` if `Type` is trivially default constructible.
 bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context);
 
 /// Returns `true` if `RecordDecl` is trivially default constructible.
 bool recordIsTriviallyDefaultConstructible(const RecordDecl &RecordDecl,
                                            const ASTContext &Context);
 
+/// Returns true if `Type` has a non-trivial move constructor.
+bool hasNonTrivialMoveConstructor(QualType Type);
+
+/// Return true if `Type` has a non-trivial move assignment operator.
+bool hasNonTrivialMoveAssignment(QualType Type);
+
 } // type_traits
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H

clang-tools-extra/trunk/clang-tidy/utils/TypeTraits.cpp

bool isTriviallyDefaultConstructible(QualType Type,
   if (const auto *RT = CanonicalType->getAs<RecordType>()) {
     return recordIsTriviallyDefaultConstructible(*RT->getDecl(), Context);
   }
 
   // No other types can match.
   return false;
 }
 
+bool hasNonTrivialMoveConstructor(QualType Type) {
+  auto *Record = Type->getAsCXXRecordDecl();
+  return Record && Record->hasDefinition() &&
+         Record->hasNonTrivialMoveConstructor();
+}
+
+bool hasNonTrivialMoveAssignment(QualType Type) {
+  auto *Record = Type->getAsCXXRecordDecl();
+  return Record && Record->hasDefinition() &&
+         Record->hasNonTrivialMoveAssignment();
+}
+
 } // namespace type_traits
 } // namespace utils
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst

 .. title:: clang-tidy - performance-unnecessary-value-param
 
 performance-unnecessary-value-param
 ===================================
 
 Flags value parameter declarations of expensive to copy types that are copied
 for each invocation but it would suffice to pass them by const reference.
 
 The check is only applied to parameters of types that are expensive to copy
 which means they are not trivially copyable or have a non-trivial copy
 constructor or destructor.
 
-To ensure that it is safe to replace the value paramater with a const reference
+To ensure that it is safe to replace the value parameter with a const reference
 the following heuristic is employed:
 
 1. the parameter is const qualified;
 2. the parameter is not const, but only const methods or operators are invoked
    on it, or it is used as const reference or value argument in constructors or
    function calls.
 
 Example:
 
 .. code-block:: c++
 
   void f(const string Value) {
     // The warning will suggest making Value a reference.
   }
 
   void g(ExpensiveToCopy Value) {
     // The warning will suggest making Value a const reference.
     Value.ConstMethd();
     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.
+
+Example:
+
+.. code-block:: c++
+
+  void setValue(string Value) {
+    Field = Value;
+  }
+
+Will become:
+
+.. code-block:: c++
+
+  #include <utility>
+
+  void setValue(string Value) {
+    Field = std::move(Value);
+  }

clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param.cpp

 // RUN: %check_clang_tidy %s performance-unnecessary-value-param %t
 
+// CHECK-FIXES: #include <utility>
+
 struct ExpensiveToCopyType {
   const ExpensiveToCopyType & constReference() const {
     return *this;
   }
   void nonConstMethod();
   virtual ~ExpensiveToCopyType();
 };
 

 
 struct MoveOnlyType {
   MoveOnlyType(const MoveOnlyType &) = delete;
   MoveOnlyType(MoveOnlyType &&) = default;
   ~MoveOnlyType();
   void constMethod() const;
 };
 
+struct ExpensiveMovableType {
+  ExpensiveMovableType();
+  ExpensiveMovableType(ExpensiveMovableType &&);
+  ExpensiveMovableType(const ExpensiveMovableType &) = default;
+  ExpensiveMovableType &operator=(const ExpensiveMovableType &) = default;
+  ExpensiveMovableType &operator=(ExpensiveMovableType &&);
+  ~ExpensiveMovableType();
+};
+
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj);
 // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj);
 void positiveExpensiveConstValue(const ExpensiveToCopyType Obj) {
   // CHECK-MESSAGES: [[@LINE-1]]:60: warning: the const qualified parameter 'Obj' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param]
   // CHECK-FIXES: void positiveExpensiveConstValue(const ExpensiveToCopyType& Obj) {
 }
 
 void positiveExpensiveValue(ExpensiveToCopyType Obj);
struct NegativeDeletedMethod {
   ~NegativeDeletedMethod() {}
   NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
   // CHECK-FIXES: NegativeDeletedMethod& operator=(NegativeDeletedMethod N) = delete;
 };
 
 void NegativeMoveOnlyTypePassedByValue(MoveOnlyType M) {
   M.constMethod();
 }
+
+void PositiveMoveOnCopyConstruction(ExpensiveMovableType E) {
+  auto F = E;
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: parameter 'E' is passed by value and only copied once; consider moving it to avoid unnecessary copies [performance-unnecessary-value-param]
+  // CHECK-FIXES: auto F = std::move(E);
+}
+
+void PositiveConstRefNotMoveSinceReferencedMultipleTimes(ExpensiveMovableType E) {
+  // CHECK-MESSAGES: [[@LINE-1]]:79: warning: the parameter 'E' is copied
+  // CHECK-FIXES: void PositiveConstRefNotMoveSinceReferencedMultipleTimes(const ExpensiveMovableType& E) {
+  auto F = E;
+  auto G = E;
+}
+
+void PositiveMoveOnCopyAssignment(ExpensiveMovableType E) {
+  ExpensiveMovableType F;
+  F = E;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: parameter 'E' is passed by value
+  // CHECK-FIXES: F = std::move(E);
+}
+
+void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
+  // CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
+  // CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const ExpensiveToCopyType& T) {
+  auto U = T;
+}
+
+void PositiveConstRefNotMoveAssignable(ExpensiveToCopyType A) {
+  // CHECK-MESSAGES: [[@LINE-1]]:60: warning: the parameter 'A' is copied
+  // CHECK-FIXES: void PositiveConstRefNotMoveAssignable(const ExpensiveToCopyType& A) {
+  ExpensiveToCopyType B;
+  B = A;
+}