[clang-tidy] Only invoke ForRangeCopyCheck on expensive-to-copy types.

Summary:
Fix oversight not checking the value of the Optional<bool> returned by
isExpensiveToCopy().

Reviewers: alexfh

Subscribers: cfe-commits

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

llvm-svn: 260870

2 files changed, 31 insertions, 3 deletions

diff --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
index 88e277720fb..a293cfee760 100644
--- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -135,7 +135,9 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
   } else if (!LoopVar.getType().isConstQualified()) {
     return false;
   }
-  if (!type_traits::isExpensiveToCopy(LoopVar.getType(), Context))
+  llvm::Optional<bool> Expensive =
+      type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
+  if (!Expensive || !*Expensive)
     return false;
   auto Diagnostic =
       diag(LoopVar.getLocation(),
@@ -150,8 +152,9 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
 bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
     const VarDecl &LoopVar, const CXXForRangeStmt &ForRange,
     ASTContext &Context) {
-  if (LoopVar.getType().isConstQualified() ||
-      !type_traits::isExpensiveToCopy(LoopVar.getType(), Context)) {
+  llvm::Optional<bool> Expensive =
+      type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
+  if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive) {
     return false;
   }
   // Collect all DeclRefExprs to the loop variable and all CallExprs and
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 0038434f3a7..88ec45b2ddf 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
@@ -127,6 +127,7 @@ bool operator!=(const Mutable& M1, const Mutable& M2) {
 }
 
 void use(const Mutable &M);
+void use(int I);
 void useTwice(const Mutable &M1, const Mutable &M2);
 void useByValue(Mutable M);
 void useByConstValue(const Mutable M);
@@ -170,6 +171,30 @@ void negativeNonConstNonMemberOperatorInvoked() {
   }
 }
 
+void negativeConstCheapToCopy() {
+  for (const int I : View<Iterator<int>>()) {
+  }
+}
+
+void negativeConstCheapToCopyTypedef() {
+  typedef const int ConstInt;
+  for (ConstInt C  : View<Iterator<ConstInt>>()) {
+  }
+}
+
+void negativeCheapToCopy() {
+  for (int I : View<Iterator<int>>()) {
+    use(I);
+  }
+}
+
+void negativeCheapToCopyTypedef() {
+  typedef int Int;
+  for (Int I : View<Iterator<Int>>()) {
+    use(I);
+  }
+}
+
 void positiveOnlyConstMethodInvoked() {
   for (auto M : 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]