[clang-tidy] Do not move parameter if only DeclRefExpr occurs inside of a loop

Summary: This fixes a bug where the performance-unnecessary-value-param check suggests a fix to move the parameter inside of a loop which could be invoked multiple times.

Reviewers: sbenza, aaron.ballman, alexfh

Subscribers: JDevlieghere, cfe-commits

Differential Revision: https://reviews.llvm.org/D27187

llvm-svn: 289912

1 files changed, 13 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
index 5d746d6..0b58384 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -47,6 +47,17 @@ bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
   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
 
 UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
@@ -105,6 +116,8 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
   if (!IsConstQualified) {
     auto CanonicalType = Param->getType().getCanonicalType();
     if (AllDeclRefExprs.size() == 1 &&
+        !hasLoopStmtAncestor(**AllDeclRefExprs.begin(), *Function->getBody(),
+                             *Result.Context) &&
         ((utils::type_traits::hasNonTrivialMoveConstructor(CanonicalType) &&
           utils::decl_ref_expr::isCopyConstructorArgument(
               **AllDeclRefExprs.begin(), *Function->getBody(),