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

Summary:

Make check more useful in the following two cases:

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.
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.

Reviewers: alexfh, sbenza

Subscribers: cfe-commits, Prazek

Differential Revision: http://reviews.llvm.org/D20277

llvm-svn: 274380

5 files changed, 84 insertions, 14 deletions

diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index 78291ddc9f7..11d0c311f14 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -8,10 +8,10 @@
 //===----------------------------------------------------------------------===//
 
 #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 {
@@ -38,16 +38,7 @@ void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
     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.
@@ -80,8 +71,6 @@ constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
   return DeclRefs;
 }
 
-} // namespace
-
 bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
                        ASTContext &Context) {
   // Collect all DeclRefExprs to the loop variable and all CallExprs and
@@ -89,11 +78,48 @@ bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
   // 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
diff --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
index 15595c193c1..5d56f7f05d9 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.h
@@ -12,6 +12,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Type.h"
+#include "llvm/ADT/SmallPtrSet.h"
 
 namespace clang {
 namespace tidy {
@@ -27,6 +28,25 @@ namespace decl_ref_expr {
 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
diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h
index d39a412bc64..0699196b101 100644
--- a/clang-tools-extra/clang-tidy/utils/Matchers.h
+++ b/clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -42,6 +42,12 @@ AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
 
 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
diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
index 2fb4005d7a8..ea379f6e12b 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp
@@ -124,6 +124,18 @@ bool isTriviallyDefaultConstructible(QualType Type,
   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
diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.h b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
index 096c65bac99..ae0b3f0f483 100644
--- a/clang-tools-extra/clang-tidy/utils/TypeTraits.h
+++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.h
@@ -29,6 +29,12 @@ bool isTriviallyDefaultConstructible(QualType Type, const ASTContext &Context);
 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