flx removed rL LLVM as the repository for this revision.
flx updated this revision to Diff 79550.

https://reviews.llvm.org/D27187

Files:
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/utils/DeclRefExprUtils.cpp
  clang-tidy/utils/DeclRefExprUtils.h
  test/clang-tidy/performance-unnecessary-value-param.cpp


Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -225,6 +225,15 @@
   // CHECK-FIXES: F = std::move(E);
 }
 
+// The argument could be moved but is not since copy statement is inside a 
loop.
+void PositiveNoMoveInsideLoop(ExpensiveMovableType E) {
+  // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the parameter 'E' is copied
+  // CHECK-FIXES: void PositiveNoMoveInsideLoop(const ExpensiveMovableType& E) 
{
+  for (;;) {
+    auto F = E;
+  }
+}
+
 void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
   // CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
   // CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const 
ExpensiveToCopyType& T) {
Index: clang-tidy/utils/DeclRefExprUtils.h
===================================================================
--- clang-tidy/utils/DeclRefExprUtils.h
+++ clang-tidy/utils/DeclRefExprUtils.h
@@ -47,6 +47,11 @@
 bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
                               ASTContext &Context);
 
+// Returns true if DeclRefExpr has a loop statement (ForStmt, CXXForRangeStmt,
+// WhileStmt, DoStmt) as ancestor.
+bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                         ASTContext &Context);
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/utils/DeclRefExprUtils.cpp
===================================================================
--- clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tidy/utils/DeclRefExprUtils.cpp
@@ -120,6 +120,17 @@
   return !Matches.empty();
 }
 
+bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                         ASTContext &Context) {
+  auto Matches =
+      match(findAll(declRefExpr(equalsNode(&DeclRef),
+                                unless(hasAncestor(stmt(anyOf(
+                                    forStmt(), cxxForRangeStmt(), whileStmt(),
+                                    doStmt())))))),
+            Stmt, Context);
+  return Matches.empty();
+}
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -104,6 +104,8 @@
   if (!IsConstQualified) {
     auto CanonicalType = Param->getType().getCanonicalType();
     if (AllDeclRefExprs.size() == 1 &&
+        !utils::decl_ref_expr::hasLoopStmtAncestor(
+            **AllDeclRefExprs.begin(), *Function->getBody(), *Result.Context) 
&&
         ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
           utils::decl_ref_expr::isCopyConstructorArgument(
               **AllDeclRefExprs.begin(), *Function->getBody(),


Index: test/clang-tidy/performance-unnecessary-value-param.cpp
===================================================================
--- test/clang-tidy/performance-unnecessary-value-param.cpp
+++ test/clang-tidy/performance-unnecessary-value-param.cpp
@@ -225,6 +225,15 @@
   // CHECK-FIXES: F = std::move(E);
 }
 
+// The argument could be moved but is not since copy statement is inside a loop.
+void PositiveNoMoveInsideLoop(ExpensiveMovableType E) {
+  // CHECK-MESSAGES: [[@LINE-1]]:52: warning: the parameter 'E' is copied
+  // CHECK-FIXES: void PositiveNoMoveInsideLoop(const ExpensiveMovableType& E) {
+  for (;;) {
+    auto F = E;
+  }
+}
+
 void PositiveConstRefNotMoveConstructible(ExpensiveToCopyType T) {
   // CHECK-MESSAGES: [[@LINE-1]]:63: warning: the parameter 'T' is copied
   // CHECK-FIXES: void PositiveConstRefNotMoveConstructible(const ExpensiveToCopyType& T) {
Index: clang-tidy/utils/DeclRefExprUtils.h
===================================================================
--- clang-tidy/utils/DeclRefExprUtils.h
+++ clang-tidy/utils/DeclRefExprUtils.h
@@ -47,6 +47,11 @@
 bool isCopyAssignmentArgument(const DeclRefExpr &DeclRef, const Stmt &Stmt,
                               ASTContext &Context);
 
+// Returns true if DeclRefExpr has a loop statement (ForStmt, CXXForRangeStmt,
+// WhileStmt, DoStmt) as ancestor.
+bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                         ASTContext &Context);
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/utils/DeclRefExprUtils.cpp
===================================================================
--- clang-tidy/utils/DeclRefExprUtils.cpp
+++ clang-tidy/utils/DeclRefExprUtils.cpp
@@ -120,6 +120,17 @@
   return !Matches.empty();
 }
 
+bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Stmt &Stmt,
+                         ASTContext &Context) {
+  auto Matches =
+      match(findAll(declRefExpr(equalsNode(&DeclRef),
+                                unless(hasAncestor(stmt(anyOf(
+                                    forStmt(), cxxForRangeStmt(), whileStmt(),
+                                    doStmt())))))),
+            Stmt, Context);
+  return Matches.empty();
+}
+
 } // namespace decl_ref_expr
 } // namespace utils
 } // namespace tidy
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -104,6 +104,8 @@
   if (!IsConstQualified) {
     auto CanonicalType = Param->getType().getCanonicalType();
     if (AllDeclRefExprs.size() == 1 &&
+        !utils::decl_ref_expr::hasLoopStmtAncestor(
+            **AllDeclRefExprs.begin(), *Function->getBody(), *Result.Context) &&
         ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
           utils::decl_ref_expr::isCopyConstructorArgument(
               **AllDeclRefExprs.begin(), *Function->getBody(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to