Only invoke ForRangeCopyCheck on expensive-to-copy types.

[flx]

Summary

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

[alexfh]

LG

clang-tidy/performance/ForRangeCopyCheck.cpp

bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
                                              ASTContext &Context) {
   if (WarnOnAllAutoCopies) {
     // For aggressive check just test that loop variable has auto type.
     if (!isa<AutoType>(LoopVar.getType()))
       return false;
   } 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(),
            "the loop variable's type is not a reference type; this creates a "
            "copy in each iteration; consider making this a reference")
       << createAmpersandFix(LoopVar, Context);
   if (!LoopVar.getType().isConstQualified())
     Diagnostic << createConstFix(LoopVar);
   return true;
 }
 
 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
   // CXXConstructExprs where the loop variable is used as argument to a const
   // reference parameter.
   // If the difference is empty it is safe for the loop variable to be a const
   // reference.
   auto AllDeclRefs = declRefExprs(LoopVar, *ForRange.getBody(), Context);

test/clang-tidy/performance-for-range-copy.cpp

Mutable& operator<<(Mutable &Out, bool B) {
   return Out;
 }
 
 bool operator!=(const Mutable& M1, const Mutable& M2) {
   return false;
 }
 
 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);
 void mutate(Mutable *M);
 void mutate(Mutable &M);
 void onceConstOnceMutated(const Mutable &M1, Mutable &M2);
 
 void negativeVariableIsMutated() {

 }
 
 void negativeNonConstNonMemberOperatorInvoked() {
   for (auto NonConstOperatorInvokee : View<Iterator<Mutable>>()) {
     NonConstOperatorInvokee << true;
   }
 }
 
+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]
     // CHECK-FIXES: for (const auto& M : View<Iterator<Mutable>>()) {
     M.constMethod();
   }
 }