From 0ed0febb3e58081edf1c69b62d28f72703a417f8 Mon Sep 17 00:00:00 2001
From: Shuai Wang <shuaiwang@google.com>
Date: Tue, 10 Jul 2018 22:51:06 +0000
Subject: [PATCH] Use ExprMutationAnalyzer in performance-for-range-copy

Summary:
This gives better coverage to the check as ExprMutationAnalyzer is more
accurate comparing to isOnlyUsedAsConst.

Majority of wins come from const usage of member field, e.g.:
for (auto widget : container) { // copy of loop variable
  if (widget.type == BUTTON) { // const usage only recognized by ExprMutationAnalyzer
    // ...
  }
}

Reviewers: george.karpenkov

Subscribers: a.sidorin, cfe-commits

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

llvm-svn: 336737
---
 .../clang-tidy/performance/ForRangeCopyCheck.cpp           |  6 +++---
 .../test/clang-tidy/performance-for-range-copy.cpp         | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
index 0b9dd23..2358aac 100644
--- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -8,7 +8,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "ForRangeCopyCheck.h"
-#include "../utils/DeclRefExprUtils.h"
+#include "../utils/ExprMutationAnalyzer.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/TypeTraits.h"
 
@@ -79,8 +79,8 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
       utils::type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
   if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive)
     return false;
-  if (!utils::decl_ref_expr::isOnlyUsedAsConst(LoopVar, *ForRange.getBody(),
-                                               Context))
+  if (utils::ExprMutationAnalyzer(ForRange.getBody(), &Context)
+          .isMutated(&LoopVar))
     return false;
   diag(LoopVar.getLocation(),
        "loop variable is copied but only used as const reference; consider "
diff --git a/clang-tools-extra/test/clang-tidy/performance-for-range-copy.cpp b/clang-tools-extra/test/clang-tidy/performance-for-range-copy.cpp
index 88ec45b..1c77996 100644
--- a/clang-tools-extra/test/clang-tidy/performance-for-range-copy.cpp
+++ b/clang-tools-extra/test/clang-tidy/performance-for-range-copy.cpp
@@ -117,6 +117,11 @@ struct Mutable {
   ~Mutable() {}
 };
 
+struct Point {
+  ~Point() {}
+  int x, y;
+};
+
 Mutable& operator<<(Mutable &Out, bool B) {
   Out.setBool(B);
   return Out;
@@ -214,6 +219,15 @@ void positiveOnlyUsedAsConstArguments() {
   }
 }
 
+void positiveOnlyAccessedFieldAsConst() {
+  for (auto UsedAsConst : View<Iterator<Point>>()) {
+    // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
+    // CHECK-FIXES: for (const auto& UsedAsConst : View<Iterator<Point>>()) {
+    use(UsedAsConst.x);
+    use(UsedAsConst.y);
+  }
+}
+
 void positiveOnlyUsedInCopyConstructor() {
   for (auto A : View<Iterator<Mutable>>()) {
     // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
-- 
2.7.4