Another patch for modernize-loop-convert.

[angelgarcia]

Summary

1. Avoid converting loops that iterate over the size of a container and don't use its elements, as this would result in an unused-result warning.
2. Never capture the elements by value on lambdas, thus avoiding doing unnecessary copies and errors with non-copyable types.
3. The 'const auto &' instead of 'auto &' substitution on const containers now works on arrays and pseudoarrays as well.
4. The error about multiple replacements in the same macro call is now documented in the tests (not solved though).
5. Due to [1], I had to add a dummy usage of the range element (like "(void) *It;" or similars) on the tests that had empty loops.
6. I removed the braces from the CHECK comments. I think that there is no need for them, and they confuse vim.

[klimek]

clang-tidy/modernize/LoopConvertCheck.cpp

457–458

I'd call that 'Usage' here, as it's not an iterator.

459–464

We need to document the members of Usage better: It seems unclear why the 'else' part goes into a lambda capture, or why in the if() path adding a '.' will ever help. Examples would help in comments here, I think.

clang-tidy/modernize/LoopConvertUtils.cpp

765–769

This needs to be a comment at the IsArrow field of Usage.

test/clang-tidy/modernize-loop-convert-basic.cpp

504

why Pseudo?

[angelgarcia]

Document Usage's fields and other comments.

[klimek]

clang-tidy/modernize/LoopConvertUtils.cpp

765–769

I think when the comment is on the Usage member, it's fine to delete it here, as it'll only get out of sync.

clang-tidy/modernize/LoopConvertUtils.h

214–217

I think this now really needs a different name, or we should put it into an enum if we can't find a good bool name that fits both the lambda capture and member expression case.

[angelgarcia]

Replace the IsArrow field of Usage with an enum.

[klimek]

clang-tidy/modernize/LoopConvertCheck.cpp

468–470

Isn't it the other way round?

clang-tidy/modernize/LoopConvertUtils.h

200–205

Please comment the enumerators. Do we need default? Is that always a member expression or sometimes also something else?

[angelgarcia]

Comment the enumerators.

Do we need default?

I think so. We need to set the cases that do not fall in any of these categories to something, and I think that using one of the other three as the default kind would be confusing. But maybe there is a better name than just UK_Default. Any ideas?

[klimek]

In D12734#243157, @angelgarcia wrote:

Comment the enumerators.

Do we need default?

I think so. We need to set the cases that do not fall in any of these categories to something, and I think that using one of the other three as the default kind would be confusing. But maybe there is a better name than just UK_Default. Any ideas?

Can we perhaps just add a couple of example of other cases that are not member expressions? With that, LG.

[angelgarcia]

Add examples of 'default' usages.

clang-tidy/modernize/LoopConvertCheck.h

public:
   LoopConvertCheck(StringRef Name, ClangTidyContext *Context);
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
   struct RangeDescriptor {
     bool ContainerNeedsDereference;
+    bool DerefByConstRef;
     bool DerefByValue;
     bool IsTriviallyCopyable;
-    bool DerefByConstRef;
   };
 
   void doConversion(ASTContext *Context, const VarDecl *IndexVar,
                     const VarDecl *MaybeContainer, StringRef ContainerString,
                     const UsageResult &Usages, const DeclStmt *AliasDecl,
                     bool AliasUseRequired, bool AliasFromForInit,
                     const ForStmt *TheLoop, RangeDescriptor Descriptor);
 

clang-tidy/modernize/LoopConvertCheck.cpp

 static bool usagesReturnRValues(const UsageResult &Usages) {
   for (const auto &U : Usages) {
     if (U.Expression && !U.Expression->isRValue())
       return false;
   }
   return true;
 }
 
+/// \brief Returns true if the container is const-qualified.
+static bool containerIsConst(const Expr *ContainerExpr, bool Dereference) {
+  if (const auto *VDec = getReferencedVariable(ContainerExpr)) {
+    QualType CType = VDec->getType();
+    if (Dereference) {
+      if (!CType->isPointerType())
+        return false;
+      CType = CType->getPointeeType();
+    }
+    return CType.isConstQualified();
+  }
+  return false;
+}
+
 LoopConvertCheck::LoopConvertCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), TUInfo(new TUTrackingInfo),
       MinConfidence(StringSwitch<Confidence::Level>(
                         Options.get("MinConfidence", "reasonable"))
                         .Case("safe", Confidence::CL_Safe)
                         .Case("risky", Confidence::CL_Risky)
                         .Default(Confidence::CL_Reasonable)) {}
 
 void LoopConvertCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   SmallVector<std::string, 3> Confs{"risky", "reasonable", "safe"};
   Options.store(Opts, "MinConfidence", Confs[static_cast<int>(MinConfidence)]);
 }
 
 /// \brief Computes the changes needed to convert a given for loop, and
 /// applies it.
 void LoopConvertCheck::doConversion(
     ASTContext *Context, const VarDecl *IndexVar, const VarDecl *MaybeContainer,
     StringRef ContainerString, const UsageResult &Usages,
     const DeclStmt *AliasDecl, bool AliasUseRequired, bool AliasFromForInit,
     const ForStmt *TheLoop, RangeDescriptor Descriptor) {
+  // If there aren't any usages, converting the loop would generate an unused
+  // variable warning.
+  if (Usages.size() == 0)
+    return;
+
   auto Diag = diag(TheLoop->getForLoc(), "use range-based for loop instead");
 
   std::string VarName;
   bool VarNameFromAlias = (Usages.size() == 1) && AliasDecl;
   bool AliasVarIsRef = false;
 
   if (VarNameFromAlias) {
     const auto *AliasVar = cast<VarDecl>(AliasDecl->getSingleDecl());
if (VarNameFromAlias) {
     // No further replacements are made to the loop, since the iterator or index
     // was used exactly once - in the initialization of AliasVar.
   } else {
     VariableNamer Namer(&TUInfo->getGeneratedDecls(),
                         &TUInfo->getParentFinder().getStmtToParentStmtMap(),
                         TheLoop, IndexVar, MaybeContainer, Context);
     VarName = Namer.createIndexName();
     // First, replace all usages of the array subscript expression with our new
     // variable.
-    for (const auto &I : Usages) {
-      std::string ReplaceText = I.IsArrow ? VarName + "." : VarName;
+    for (const auto &Usage : Usages) {
+      std::string ReplaceText;
+      if (Usage.Expression) {
+        // If this is an access to a member through the arrow operator, after
+        // the replacement it must be accessed through the '.' operator.
+        ReplaceText = Usage.Kind == Usage::UK_MemberThroughArrow ? VarName + "."
+                                                                 : VarName;
+      } else {
+        // The Usage expression is only null in case of lambda captures (which
+        // are VarDecl). If the index is captured by value, add '&' to capture
+        // by reference instead.
+        ReplaceText =
+            Usage.Kind == Usage::UK_CaptureByCopy ? "&" + VarName : VarName;
+      }
       TUInfo->getReplacedVars().insert(std::make_pair(TheLoop, IndexVar));
       Diag << FixItHint::CreateReplacement(
-          CharSourceRange::getTokenRange(I.Range), ReplaceText);
+          CharSourceRange::getTokenRange(Usage.Range), ReplaceText);
     }
   }
 
   // Now, we need to construct the new range expression.
   SourceRange ParenRange(TheLoop->getLParenLoc(), TheLoop->getRParenLoc());
 
   QualType AutoRefType = Context->getAutoDeductType();
 
void LoopConvertCheck::findAndVerifyUsages(
 
   if (!Finder.findAndVerifyUsages(TheLoop->getBody()))
     return;
 
   Confidence ConfidenceLevel(Finder.getConfidenceLevel());
   if (FixerKind == LFK_Array) {
     // The array being indexed by IndexVar was discovered during traversal.
     ContainerExpr = Finder.getContainerIndexed()->IgnoreParenImpCasts();
+
     // Very few loops are over expressions that generate arrays rather than
     // array variables. Consider loops over arrays that aren't just represented
     // by a variable to be risky conversions.
     if (!getReferencedVariable(ContainerExpr) &&
         !isDirectMemberExpr(ContainerExpr))
       ConfidenceLevel.lowerTo(Confidence::CL_Risky);
+
+    // Use 'const' if the array is const.
+    if (containerIsConst(ContainerExpr, Descriptor.ContainerNeedsDereference))
+      Descriptor.DerefByConstRef = true;
+
   } else if (FixerKind == LFK_PseudoArray) {
     if (!Descriptor.DerefByValue && !Descriptor.DerefByConstRef) {
       const UsageResult &Usages = Finder.getUsages();
-      if (usagesAreConst(Usages)) {
+      if (usagesAreConst(Usages) ||
+          containerIsConst(ContainerExpr,
+                           Descriptor.ContainerNeedsDereference)) {
         Descriptor.DerefByConstRef = true;
       } else if (usagesReturnRValues(Usages)) {
         // If the index usages (dereference, subscript, at) return RValues,
         // then we should not use a non-const reference.
         Descriptor.DerefByValue = true;
         // Try to find the type of the elements on the container from the
         // usages.
         for (const Usage &U : Usages) {
           if (!U.Expression || U.Expression->getType().isNull())
             continue;
           QualType Type = U.Expression->getType().getCanonicalType();
-          if (U.IsArrow) {
+          if (U.Kind == Usage::UK_MemberThroughArrow) {
             if (!Type->isPointerType())
               continue;
             Type = Type->getPointeeType();
           }
           Descriptor.IsTriviallyCopyable =
               Type.isTriviallyCopyableType(*Context);
         }
       }
void LoopConvertCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *ConditionEndVar = Nodes.getDeclAs<VarDecl>(ConditionEndVarName);
   if (EndVar && !areSameVariable(EndVar, ConditionEndVar))
     return;
 
   // If the end comparison isn't a variable, we can try to work with the
   // expression the loop variable is being tested against instead.
   const auto *EndCall = Nodes.getStmtAs<CXXMemberCallExpr>(EndCallName);
   const auto *BoundExpr = Nodes.getStmtAs<Expr>(ConditionBoundName);
+
   // If the loop calls end()/size() after each iteration, lower our confidence
   // level.
   if (FixerKind != LFK_Array && !EndVar)
     ConfidenceLevel.lowerTo(Confidence::CL_Reasonable);
 
   const Expr *ContainerExpr = nullptr;
   RangeDescriptor Descriptor{false, false, false, false};
   // FIXME: Try to put most of this logic inside a matcher. Currently, matchers

clang-tidy/modernize/LoopConvertUtils.h

private:
   bool VisitNamedDecl(clang::NamedDecl *D);
   bool VisitDeclRefExpr(clang::DeclRefExpr *D);
   bool VisitTypeLoc(clang::TypeLoc TL);
 };
 
 /// \brief The information needed to describe a valid convertible usage
 /// of an array index or iterator.
 struct Usage {
+  enum UsageKind {
+    // Regular usages of the loop index (the ones not specified below).
+    UK_Default,
+    // Indicates whether this is an access to a member through the arrow
+    // operator on pointers or iterators.
+    UK_MemberThroughArrow,
+    // If the variable is being captured by a lambda, indicates whether the
+    // capture was done by value or by reference.
+    UK_CaptureByCopy,
+    UK_CaptureByRef
+  };
+  // The expression that is going to be converted. Null in case of lambda
+  // captures.
   const Expr *Expression;
-  bool IsArrow;
+
+  UsageKind Kind;
+
+  // Range that covers this usage.
   SourceRange Range;
 
   explicit Usage(const Expr *E)
-      : Expression(E), IsArrow(false), Range(Expression->getSourceRange()) {}
-  Usage(const Expr *E, bool IsArrow, SourceRange Range)
-      : Expression(E), IsArrow(IsArrow), Range(std::move(Range)) {}
+      : Expression(E), Kind(UK_Default), Range(Expression->getSourceRange()) {}
+  Usage(const Expr *E, UsageKind Kind, SourceRange Range)
+      : Expression(E), Kind(Kind), Range(std::move(Range)) {}
 };
 
 /// \brief A class to encapsulate lowering of the tool's confidence level.
 class Confidence {
 public:
   enum Level {
     // Transformations that are likely to change semantics.
     CL_Risky,

clang-tidy/modernize/LoopConvertUtils.cpp

if (Obj && exprReferencesVariable(IndexVar, Obj)) {
     // FIXME: This works around not having the location of the arrow operator.
     // Consider adding OperatorLoc to MemberExpr?
     SourceLocation ArrowLoc = Lexer::getLocForEndOfToken(
         Base->getExprLoc(), 0, Context->getSourceManager(),
         Context->getLangOpts());
     // If something complicated is happening (i.e. the next token isn't an
     // arrow), give up on making this work.
     if (!ArrowLoc.isInvalid()) {
-      addUsage(Usage(ResultExpr, /*IsArrow=*/true,
+      addUsage(Usage(ResultExpr, Usage::UK_MemberThroughArrow,
                      SourceRange(Base->getExprLoc(), ArrowLoc)));
       return true;
     }
   }
   return VisitorBase::TraverseMemberExpr(Member);
 }
 
 /// \brief If a member function call is the at() accessor on the container with

 bool ForLoopIndexUseVisitor::TraverseLambdaCapture(LambdaExpr *LE,
                                                    const LambdaCapture *C) {
   if (C->capturesVariable()) {
     const VarDecl *VDecl = C->getCapturedVar();
     if (areSameVariable(IndexVar, cast<ValueDecl>(VDecl))) {
       // FIXME: if the index is captured, it will count as an usage and the
       // alias (if any) won't work, because it is only used in case of having
       // exactly one usage.
-      addUsage(Usage(nullptr, false, C->getLocation()));
+      addUsage(Usage(nullptr,
+                     C->getCaptureKind() == LCK_ByCopy ? Usage::UK_CaptureByCopy
+                                                       : Usage::UK_CaptureByRef,
+                     C->getLocation()));
     }
   }
   return VisitorBase::TraverseLambdaCapture(LE, C);
 }
 
 /// \brief If we find that another variable is created just to refer to the loop
 /// element, note it for reuse as the loop variable.
 ///
 /// See the comments for isAliasDecl.

test/clang-tidy/Inputs/modernize-loop-convert/structures.h

 struct X {
   S s;
   T t;
   U u;
   S getS();
 };
 
 template<typename ElemType>
-class dependent{
+class dependent {
  public:
+  dependent<ElemType>();
   struct iterator_base {
     const ElemType& operator*()const;
     iterator_base& operator ++();
     bool operator!=(const iterator_base &other) const;
     const ElemType *operator->() const;
   };
 
   struct iterator : iterator_base {

test/clang-tidy/modernize-loop-convert-basic.cpp

 // RUN: %python %S/check_clang_tidy.py %s modernize-loop-convert %t -- -std=c++11 -I %S/Inputs/modernize-loop-convert
 
 #include "structures.h"
 
 namespace Array {
 
 const int N = 6;
 const int NMinusOne = N - 1;
 int arr[N] = {1, 2, 3, 4, 5, 6};
+const int constArr[N] = {1, 2, 3, 4, 5, 6};
 int (*pArr)[N] = &arr;
 
 void f() {
   int sum = 0;
 
   for (int i = 0; i < N; ++i) {
     sum += arr[i];
     int k;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead [modernize-loop-convert]
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: sum += elem;
   // CHECK-FIXES-NEXT: int k;
-  // CHECK-FIXES-NEXT: }
 
   for (int i = 0; i < N; ++i) {
     printf("Fibonacci number is %d\n", arr[i]);
     sum += arr[i] + 2;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
void f() {
   // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: int x = N;
   // CHECK-FIXES-NEXT: x = elem;
 
   for (int i = 0; i < N; ++i) {
     arr[i] += 1;
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: elem += 1;
-  // CHECK-FIXES-NEXT: }
 
   for (int i = 0; i < N; ++i) {
     int x = arr[i] + 2;
     arr[i]++;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: int x = elem + 2;
   // CHECK-FIXES-NEXT: elem++;
 
   for (int i = 0; i < N; ++i) {
     arr[i] = 4 + arr[i];
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: elem = 4 + elem;
 
   for (int i = 0; i < NMinusOne + 1; ++i) {
     sum += arr[i];
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: sum += elem;
-  // CHECK-FIXES-NEXT: }
 
   for (int i = 0; i < N; ++i) {
     printf("Fibonacci number %d has address %p\n", arr[i], &arr[i]);
     sum += arr[i] + 2;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", elem, &elem);
   // CHECK-FIXES-NEXT: sum += elem + 2;
 
   Val teas[N];
   for (int i = 0; i < N; ++i) {
     teas[i].g();
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & tea : teas) {
+  // CHECK-FIXES: for (auto & tea : teas)
   // CHECK-FIXES-NEXT: tea.g();
-  // CHECK-FIXES-NEXT: }
+}
+
+void constArray() {
+  for (int i = 0; i < N; ++i) {
+    printf("2 * %d = %d\n", constArr[i], constArr[i] + constArr[i]);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & elem : constArr)
+  // CHECK-FIXES-NEXT: printf("2 * %d = %d\n", elem, elem + elem);
 }
 
 struct HasArr {
   int Arr[N];
   Val ValArr[N];
   void implicitThis() {
     for (int i = 0; i < N; ++i) {
       printf("%d", Arr[i]);
     }
     // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : Arr) {
+    // CHECK-FIXES: for (auto & elem : Arr)
     // CHECK-FIXES-NEXT: printf("%d", elem);
-    // CHECK-FIXES-NEXT: }
 
     for (int i = 0; i < N; ++i) {
       printf("%d", ValArr[i].x);
     }
     // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : ValArr) {
+    // CHECK-FIXES: for (auto & elem : ValArr)
     // CHECK-FIXES-NEXT: printf("%d", elem.x);
-    // CHECK-FIXES-NEXT: }
   }
 
   void explicitThis() {
     for (int i = 0; i < N; ++i) {
       printf("%d", this->Arr[i]);
     }
     // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : this->Arr) {
+    // CHECK-FIXES: for (auto & elem : this->Arr)
     // CHECK-FIXES-NEXT: printf("%d", elem);
-    // CHECK-FIXES-NEXT: }
 
     for (int i = 0; i < N; ++i) {
       printf("%d", this->ValArr[i].x);
     }
     // CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : this->ValArr) {
+    // CHECK-FIXES: for (auto & elem : this->ValArr)
     // CHECK-FIXES-NEXT: printf("%d", elem.x);
-    // CHECK-FIXES-NEXT: }
   }
 };
 
-// Loops whose bounds are value-dependent shold not be converted.
+// Loops whose bounds are value-dependent should not be converted.
 template <int N>
 void dependentExprBound() {
   for (int i = 0; i < N; ++i)
     arr[i] = 0;
 }
 template void dependentExprBound<20>();
 
 void memberFunctionPointer() {
void f() {
   // CHECK-FIXES-NEXT: int k = A->x + elem.x;
 
   dependent<int> v;
   for (dependent<int>::iterator it = v.begin(), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : v) {
+  // CHECK-FIXES: for (auto & elem : v)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
 
   for (dependent<int>::iterator it(v.begin()), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : v) {
+  // CHECK-FIXES: for (auto & elem : v)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
 
   doublyDependent<int, int> intmap;
   for (doublyDependent<int, int>::iterator it = intmap.begin(), e = intmap.end();
        it != e; ++it) {
     printf("intmap[%d] = %d", it->first, it->second);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : intmap)
   // CHECK-FIXES: printf("intmap[%d] = %d", elem.first, elem.second);
 
   // PtrSet's iterator dereferences by value so auto & can't be used.
   {
-    PtrSet<int *> int_ptrs;
-    for (PtrSet<int *>::iterator I = int_ptrs.begin(),
-                                 E = int_ptrs.end();
+    PtrSet<int *> val_int_ptrs;
+    for (PtrSet<int *>::iterator I = val_int_ptrs.begin(),
+                                 E = val_int_ptrs.end();
          I != E; ++I) {
+      (void) *I;
     }
-    // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto int_ptr : int_ptrs) {
+    // CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
+    // CHECK-FIXES: for (auto val_int_ptr : val_int_ptrs)
   }
 
   // This container uses an iterator where the derefence type is a typedef of
   // a reference type. Make sure non-const auto & is still used. A failure here
   // means canonical types aren't being tested.
   {
     TypedefDerefContainer<int> int_ptrs;
     for (TypedefDerefContainer<int>::iterator I = int_ptrs.begin(),
                                               E = int_ptrs.end();
          I != E; ++I) {
+      (void) *I;
     }
-    // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & int_ptr : int_ptrs) {
+    // CHECK-MESSAGES: :[[@LINE-5]]:5: warning: use range-based for loop instead
+    // CHECK-FIXES: for (auto & int_ptr : int_ptrs)
   }
 
   {
     // Iterators returning an rvalue reference should disqualify the loop from
     // transformation.
     RValueDerefContainer<int> container;
     for (RValueDerefContainer<int>::iterator I = container.begin(),
                                              E = container.end();
          I != E; ++I) {
+      (void) *I;
     }
-    // CHECK-FIXES: for (RValueDerefContainer<int>::iterator I = container.begin(),
-    // CHECK-FIXES-NEXT: E = container.end();
-    // CHECK-FIXES-NEXT: I != E; ++I) {
   }
 }
 
 // Tests to verify the proper use of auto where the init variable type and the
 // initializer type differ or are mostly the same except for const qualifiers.
 void different_type() {
   // s.begin() returns a type 'iterator' which is just a non-const pointer and
   // differs from const_iterator only on the const qualification.
void different_type() {
   // v.begin() returns a user-defined type 'iterator' which, since it's
   // different from const_iterator, disqualifies these loops from
   // transformation.
   dependent<int> v;
   for (dependent<int>::const_iterator it = v.begin(), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK-FIXES: for (dependent<int>::const_iterator it = v.begin(), e = v.end();
-  // CHECK-FIXES-NEXT: it != e; ++it) {
-  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
 
   for (dependent<int>::const_iterator it(v.begin()), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK-FIXES: for (dependent<int>::const_iterator it(v.begin()), e = v.end();
-  // CHECK-FIXES-NEXT: it != e; ++it) {
-  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", *it);
 }
 
 // Tests to ensure that an implicit 'this' is picked up as the container.
 // If member calls are made to 'this' within the loop, the transform becomes
 // risky as these calls may affect state that affects the loop.
 class C {
 public:
   typedef MutableVal *iterator;
   typedef const MutableVal *const_iterator;
 
   iterator begin();
   iterator end();
   const_iterator begin() const;
   const_iterator end() const;
 
   void doSomething();
   void doSomething() const;
 
   void doLoop() {
-    for (iterator I = begin(), E = end(); I != E; ++I) {
-    }
+    for (iterator I = begin(), E = end(); I != E; ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
 
-    for (iterator I = C::begin(), E = C::end(); I != E; ++I) {
-    }
+    for (iterator I = C::begin(), E = C::end(); I != E; ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
 
     for (iterator I = begin(), E = end(); I != E; ++I) {
+      (void) *I;
       doSomething();
     }
 
-    for (iterator I = begin(); I != end(); ++I) {
-    }
+    for (iterator I = begin(); I != end(); ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
 
     for (iterator I = begin(); I != end(); ++I) {
+      (void) *I;
       doSomething();
     }
   }
 
   void doLoop() const {
-    for (const_iterator I = begin(), E = end(); I != E; ++I) {
-    }
+    for (const_iterator I = begin(), E = end(); I != E; ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
 
-    for (const_iterator I = C::begin(), E = C::end(); I != E; ++I) {
-    }
+    for (const_iterator I = C::begin(), E = C::end(); I != E; ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
 
     for (const_iterator I = begin(), E = end(); I != E; ++I) {
+      (void) *I;
       doSomething();
     }
   }
 };
 
 class C2 {
 public:
   typedef MutableVal *iterator;
 
   iterator begin() const;
   iterator end() const;
 
   void doLoop() {
     // The implicit 'this' will have an Implicit cast to const C2* wrapped
     // around it. Make sure the replacement still happens.
-    for (iterator I = begin(), E = end(); I != E; ++I) {
-    }
+    for (iterator I = begin(), E = end(); I != E; ++I)
+      (void) *I;
     // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use range-based for loop instead
-    // CHECK-FIXES: for (auto & elem : *this) {
+    // CHECK-FIXES: for (auto & elem : *this)
   }
 };
 
 } // namespace Iterator
 
 namespace PseudoArray {
 
 const int N = 6;
 dependent<int> v;
 dependent<int> *pv;
+const dependent<int> constv;
+const dependent<int> *pconstv;
 
 transparent<dependent<int>> cv;
 
 void f() {
   int sum = 0;
   for (int i = 0, e = v.size(); i < e; ++i) {
     printf("Fibonacci number is %d\n", v[i]);
     sum += v[i] + 2;
for (int i = 0; i < cv->size(); ++i) {
     sum += cv->at(i) + 2;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : *cv)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
   // CHECK-FIXES-NEXT: sum += elem + 2;
 }
 
+void constness() {
+  int sum = 0;
+  for (int i = 0, e = constv.size(); i < e; ++i) {
+    printf("Fibonacci number is %d\n", constv[i]);
+    sum += constv[i] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & elem : constv)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK-FIXES-NEXT: sum += elem + 2;
+
+  for (int i = 0, e = constv.size(); i < e; ++i) {
+    printf("Fibonacci number is %d\n", constv.at(i));
+    sum += constv.at(i) + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & elem : constv)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK-FIXES-NEXT: sum += elem + 2;
+
+  for (int i = 0, e = pconstv->size(); i < e; ++i) {
+    printf("Fibonacci number is %d\n", pconstv->at(i));
+    sum += pconstv->at(i) + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & elem : *pconstv)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK-FIXES-NEXT: sum += elem + 2;
+
+  // This test will fail if size() isn't called repeatedly, since it
+  // returns unsigned int, and 0 is deduced to be signed int.
+  // FIXME: Insert the necessary explicit conversion, or write out the types
+  // explicitly.
+  for (int i = 0; i < pconstv->size(); ++i) {
+    printf("Fibonacci number is %d\n", (*pconstv).at(i));
+    sum += (*pconstv)[i] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (const auto & elem : *pconstv)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK-FIXES-NEXT: sum += elem + 2;
+}
+
 // Check for loops that don't mention containers.
 void noContainer() {
   for (auto i = 0; i < v.size(); ++i) {
   }
-  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & elem : v) {
 
   for (auto i = 0; i < v.size(); ++i)
     ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & elem : v)
 }
 
 struct NoBeginEnd {
   unsigned size() const;
+  unsigned& operator[](int);
+  const unsigned& operator[](int) const;
 };
 
 struct NoConstBeginEnd {
   NoConstBeginEnd();
   unsigned size() const;
   unsigned* begin();
   unsigned* end();
+  unsigned& operator[](int);
+  const unsigned& operator[](int) const;
 };
 
 struct ConstBeginEnd {
   ConstBeginEnd();
   unsigned size() const;
   unsigned* begin() const;
   unsigned* end() const;
+  unsigned& operator[](int);
+  const unsigned& operator[](int) const;
 };
 
 // Shouldn't transform pseudo-array uses if the container doesn't provide
 // begin() and end() of the right const-ness.
 void NoBeginEndTest() {
   NoBeginEnd NBE;
-  for (unsigned i = 0, e = NBE.size(); i < e; ++i) {
-  }
+  for (unsigned i = 0, e = NBE.size(); i < e; ++i)
+    printf("%d\n", NBE[i]);
 
   const NoConstBeginEnd const_NCBE;
-  for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i) {
-  }
+  for (unsigned i = 0, e = const_NCBE.size(); i < e; ++i)
+    printf("%d\n", const_NCBE[i]);
 
   ConstBeginEnd CBE;
-  for (unsigned i = 0, e = CBE.size(); i < e; ++i) {
-  }
+  for (unsigned i = 0, e = CBE.size(); i < e; ++i)
+    printf("%d\n", CBE[i]);
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & elem : CBE) {
+  // CHECK-FIXES: for (auto & elem : CBE)
+  // CHECK-FIXES-NEXT: printf("%d\n", elem);
 
   const ConstBeginEnd const_CBE;
-  for (unsigned i = 0, e = const_CBE.size(); i < e; ++i) {
-  }
+  for (unsigned i = 0, e = const_CBE.size(); i < e; ++i)
+    printf("%d\n", const_CBE[i]);
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & elem : const_CBE) {
+  // CHECK-FIXES: for (const auto & elem : const_CBE)
+  // CHECK-FIXES-NEXT: printf("%d\n", elem);
 }
 
 struct DerefByValue {
   DerefByValue();
   struct iter { unsigned operator*(); };
   unsigned size() const;
   iter begin();
   iter end();
   unsigned operator[](int);
 };
 
 void derefByValueTest() {
   DerefByValue DBV;
   for (unsigned i = 0, e = DBV.size(); i < e; ++i) {
     printf("%d\n", DBV[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto elem : DBV) {
+  // CHECK-FIXES: for (auto elem : DBV)
   // CHECK-FIXES-NEXT: printf("%d\n", elem);
 
   for (unsigned i = 0, e = DBV.size(); i < e; ++i) {
     auto f = [DBV, i]() {};
     printf("%d\n", DBV[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto elem : DBV) {
-  // CHECK-FIXES-NEXT: auto f = [DBV, elem]() {};
+  // CHECK-FIXES: for (auto elem : DBV)
+  // CHECK-FIXES-NEXT: auto f = [DBV, &elem]() {};
   // CHECK-FIXES-NEXT: printf("%d\n", elem);
 }
 
 } // namespace PseudoArray

test/clang-tidy/modernize-loop-convert-extra.cpp

 // RUN: %python %S/check_clang_tidy.py %s modernize-loop-convert %t -- -std=c++11 -I %S/Inputs/modernize-loop-convert
 
 #include "structures.h"
 
 namespace Dependency {
 
 void f() {
   const int N = 6;
   const int M = 8;
   int arr[N][M];
 
   for (int i = 0; i < N; ++i) {
     int a = 0;
     int b = arr[i][a];
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT: int a = 0;
   // CHECK-FIXES-NEXT: int b = elem[a];
-  // CHECK-FIXES-NEXT: }
 
   for (int j = 0; j < M; ++j) {
     int a = 0;
     int b = arr[a][j];
   }
 }
 
 } // namespace Dependency
void aliasing() {
   int IntArr[N];
   for (unsigned i = 0; i < N; ++i) {
     if (int alias = IntArr[i]) {
       sideEffect(alias);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: if (alias) {
+  // CHECK-FIXES-NEXT: if (alias)
 
   for (unsigned i = 0; i < N; ++i) {
     while (int alias = IntArr[i]) {
       sideEffect(alias);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: while (alias) {
+  // CHECK-FIXES-NEXT: while (alias)
 
   for (unsigned i = 0; i < N; ++i) {
     switch (int alias = IntArr[i]) {
     default:
       sideEffect(alias);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: switch (alias) {
+  // CHECK-FIXES-NEXT: switch (alias)
 
   for (unsigned i = 0; i < N; ++i) {
     for (int alias = IntArr[i]; alias < N; ++alias) {
       sideEffect(alias);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (; alias < N; ++alias) {
+  // CHECK-FIXES-NEXT: for (; alias < N; ++alias)
 
   for (unsigned i = 0; i < N; ++i) {
     for (unsigned j = 0; int alias = IntArr[i]; ++j) {
       sideEffect(alias);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto alias : IntArr)
-  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j) {
+  // CHECK-FIXES-NEXT: for (unsigned j = 0; alias; ++j)
 
   struct IntRef { IntRef(const int& i); };
   for (int i = 0; i < N; ++i) {
     IntRef Int(IntArr[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : IntArr) {
+  // CHECK-FIXES: for (auto & elem : IntArr)
   // CHECK-FIXES-NEXT: IntRef Int(elem);
 }
 
 
 void refs_and_vals() {
   // The following tests check that the transform correctly preserves the
   // reference or value qualifiers of the aliased variable. That is, if the
   // variable was declared as a value, the loop variable will be declared as a
void macroConflict() {
   T DEFs;
   for (T::iterator it = DEFs.begin(), e = DEFs.end(); it != e; ++it) {
     if (*it == DEF) {
       printf("I found %d\n", *it);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & DEFs_it : DEFs)
-  // CHECK-FIXES-NEXT: if (DEFs_it == DEF) {
+  // CHECK-FIXES-NEXT: if (DEFs_it == DEF)
   // CHECK-FIXES-NEXT: printf("I found %d\n", DEFs_it);
 }
 
 void keywordConflict() {
   T ints;
   for (T::iterator it = ints.begin(), e = ints.end(); it != e; ++it) {
     *it = 5;
   }
void keywordConflict() {
   // CHECK-FIXES: for (auto & __FUNCTION__s_elem : __FUNCTION__s)
   // CHECK-FIXES-NEXT: int __FUNCTION__s_it = (__FUNCTION__s_elem).x + 2;
 }
 
 void typeConflict() {
   T Vals;
   // Using the name "Val", although it is the name of an existing struct, is
   // safe in this loop since it will only exist within this scope.
-  for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it) {
-  }
+  for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it)
+    (void) *it;
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & Val : Vals)
 
   // We cannot use the name "Val" in this loop since there is a reference to
   // it in the body of the loop.
   for (T::iterator it = Vals.begin(), e = Vals.end(); it != e; ++it) {
     *it = sizeof(Val);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & Vals_it : Vals)
   // CHECK-FIXES-NEXT: Vals_it = sizeof(Val);
 
   typedef struct Val TD;
   U TDs;
   // Naming the variable "TD" within this loop is safe because the typedef
   // was never used within the loop.
-  for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
-  }
+  for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it)
+    (void) *it;
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & TD : TDs)
 
   // "TD" cannot be used in this loop since the typedef is being used.
   for (U::iterator it = TDs.begin(), e = TDs.end(); it != e; ++it) {
     TD V;
     V.x = 5;
+    (void) *it;
   }
-  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & TDs_it : TDs)
   // CHECK-FIXES-NEXT: TD V;
   // CHECK-FIXES-NEXT: V.x = 5;
 
   using ns::st;
   T sts;
   for (T::iterator it = sts.begin(), e = sts.end(); it != e; ++it) {
     *it = sizeof(st);
void f() {
   // The inner loop is also convertible.
   Nested<T> NestT;
   for (Nested<T>::iterator I = NestT.begin(), E = NestT.end(); I != E; ++I) {
     for (T::iterator TI = (*I).begin(), TE = (*I).end(); TI != TE; ++TI) {
       printf("%d", *TI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : NestT) {
-  // CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI) {
+  // CHECK-FIXES: for (auto & elem : NestT)
+  // CHECK-FIXES-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI)
   // CHECK-FIXES-NEXT: printf("%d", *TI);
 
   // The inner loop is also convertible.
   Nested<S> NestS;
   for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
     for (S::const_iterator SI = (*I).begin(), SE = (*I).end(); SI != SE; ++SI) {
       printf("%d", *SI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & elem : NestS) {
-  // CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI) {
+  // CHECK-FIXES: for (const auto & elem : NestS)
+  // CHECK-FIXES-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI)
   // CHECK-FIXES-NEXT: printf("%d", *SI);
 
   for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
     const S &s = *I;
     for (S::const_iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
       printf("%d", *SI);
       const_g(SI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & s : NestS) {
+  // CHECK-FIXES: for (const auto & s : NestS)
 
   for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
     S &s = *I;
     for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
       printf("%d", *SI);
       g(SI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & s : NestS) {
+  // CHECK-FIXES: for (auto & s : NestS)
 
   Foo foo;
   for (Nested<S>::const_iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
     const S &s = *I;
     for (S::const_iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
       printf("%d", *SI);
       foo.const_g(SI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (const auto & s : NestS) {
+  // CHECK-FIXES: for (const auto & s : NestS)
 
   for (Nested<S>::iterator I = NestS.begin(), E = NestS.end(); I != E; ++I) {
     S &s = *I;
     for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
       printf("%d", *SI);
       foo.g(SI);
     }
   }
   // CHECK-MESSAGES: :[[@LINE-7]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & s : NestS) {
+  // CHECK-FIXES: for (auto & s : NestS)
 
 }
 
 } // namespace Nesting
 
 namespace SingleIterator {
 
 void complexContainer() {
void f() {
   // CHECK-FIXES-NEXT: int k = A->x + elem.x;
 
   dependent<int> v;
   for (dependent<int>::iterator it = v.begin();
        it != v.end(); ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : v) {
+  // CHECK-FIXES: for (auto & elem : v)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
 
   for (dependent<int>::iterator it(v.begin());
        it != v.end(); ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : v) {
+  // CHECK-FIXES: for (auto & elem : v)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", elem);
 
   doublyDependent<int, int> intmap;
   for (doublyDependent<int, int>::iterator it = intmap.begin();
        it != intmap.end(); ++it) {
     printf("intmap[%d] = %d", it->first, it->second);
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
void different_type() {
   }
 }
 
 } // namespace SingleIterator
 
 
 namespace Macros {
 
+#define TWO_PARAM(x, y) if (x == y) {}
+#define THREE_PARAM(x, y, z) if (x == y) {z;}
+
 const int N = 10;
 int arr[N];
 
 void messing_with_macros() {
   for (int i = 0; i < N; ++i) {
     printf("Value: %d\n", arr[i]);
   }
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : arr) {
+  // CHECK-FIXES: for (auto & elem : arr)
   // CHECK-FIXES-NEXT:  printf("Value: %d\n", elem);
 
   for (int i = 0; i < N; ++i) {
     printf("Value: %d\n", CONT arr[i]);
   }
+
+  // FIXME: Right now, clang-tidy does not allow to make insertions in several
+  // arguments of the same macro call. The following code:
+  // \code
+  //   for (int i = 0; i < N; ++i) {
+  //    TWO_PARAM(arr[i], arr[i]);
+  //    THREE_PARAM(arr[i], arr[i], arr[i]);
+  //   }
+  // \endcode
+  // Should be converted to this:
+  // \code
+  //   for (auto & elem : arr) {
+  //    TWO_PARAM(elem, elem);
+  //    THREE_PARAM(elem, elem, elem);
+  //   }
+  // \endcode
 }
 
 } // namespace Macros
 
 namespace Templates {
 
 template <class Container>
 void set_union(Container &container) {
   for (typename Container::const_iterator SI = container.begin(),
        SE = container.end(); SI != SE; ++SI) {
+    (void) *SI;
   }
+
   S s;
-  for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
-  }
+  for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI)
+    (void) *SI;
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
-  // CHECK-FIXES: for (auto & elem : s) {
+  // CHECK-FIXES: for (auto & elem : s)
 }
 
 void template_instantiation() {
   S a;
   set_union(a);
 }
 
 } // namespace Templates
 
 namespace Lambdas {
 
 void capturesIndex() {
   const int N = 10;
   int Arr[N];
   // FIXME: the next four loops could be convertible, if the capture list is
   // also changed.
 
   for (int I = 0; I < N; ++I)
     auto F1 = [Arr, I]() { int R1 = Arr[I] + 1; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto F1 = [Arr, elem]() { int R1 = elem + 1; };
+  // CHECK-FIXES-NEXT: auto F1 = [Arr, &elem]() { int R1 = elem + 1; };
 
   for (int I = 0; I < N; ++I)
     auto F2 = [Arr, &I]() { int R2 = Arr[I] + 3; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
   // CHECK-FIXES-NEXT: auto F2 = [Arr, &elem]() { int R2 = elem + 3; };
 
   // FIXME: alias don't work if the index is captured.
   // Alias declared inside lambda (by value).
   for (int I = 0; I < N; ++I)
     auto F3 = [&Arr, I]() { int R3 = Arr[I]; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto F3 = [&Arr, elem]() { int R3 = elem; };
-  // FIXME: this does two copies instead of one. Capture elem by ref?
+  // CHECK-FIXES-NEXT: auto F3 = [&Arr, &elem]() { int R3 = elem; };
 
 
   for (int I = 0; I < N; ++I)
     auto F4 = [&Arr, &I]() { int R4 = Arr[I]; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
   // CHECK-FIXES-NEXT: auto F4 = [&Arr, &elem]() { int R4 = elem; };
 
   // Alias declared inside lambda (by reference).
   for (int I = 0; I < N; ++I)
     auto F5 = [&Arr, I]() { int &R5 = Arr[I]; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto F5 = [&Arr, elem]() { int &R5 = elem; };
-  // FIXME: this does one copy instead of none. Capture elem by ref?
+  // CHECK-FIXES-NEXT: auto F5 = [&Arr, &elem]() { int &R5 = elem; };
 
 
   for (int I = 0; I < N; ++I)
     auto F6 = [&Arr, &I]() { int &R6 = Arr[I]; };
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
   // CHECK-FIXES-NEXT: auto F6 = [&Arr, &elem]() { int &R6 = elem; };
 
   for (int I = 0; I < N; ++I) {
     auto F = [Arr, I](int k) {
       printf("%d\n", Arr[I] + k);
     };
     F(Arr[I]);
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto F = [Arr, elem](int k) {
+  // CHECK-FIXES-NEXT: auto F = [Arr, &elem](int k)
   // CHECK-FIXES-NEXT: printf("%d\n", elem + k);
-  // CHECK-FIXES-NEXT: };
-  // CHECK-FIXES-NEXT: F(elem);
+  // CHECK-FIXES: F(elem);
 }
 
 void implicitCapture() {
   const int N = 10;
   int Arr[N];
   // Index is used, not convertible.
   for (int I = 0; I < N; ++I) {
     auto G1 = [&]() {
void implicitCapture() {
   for (int I = 0; I < N; ++I) {
     auto G3 = [&]() {
       int R3 = Arr[I];
       int J3 = Arr[I] + R3;
     };
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto G3 = [&]() {
+  // CHECK-FIXES-NEXT: auto G3 = [&]()
   // CHECK-FIXES-NEXT: int R3 = elem;
   // CHECK-FIXES-NEXT: int J3 = elem + R3;
 
   for (int I = 0; I < N; ++I) {
     auto G4 = [=]() {
       int R4 = Arr[I] + 5;
     };
   }
   // CHECK-MESSAGES: :[[@LINE-5]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : Arr)
-  // CHECK-FIXES-NEXT: auto G4 = [=]() {
+  // CHECK-FIXES-NEXT: auto G4 = [=]()
   // CHECK-FIXES-NEXT: int R4 = elem + 5;
 
   // Alias by value.
   for (int I = 0; I < N; ++I) {
     auto G5 = [&]() {
       int R5 = Arr[I];
       int J5 = 8 + R5;
     };
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto R5 : Arr)
-  // CHECK-FIXES-NEXT: auto G5 = [&]() {
+  // CHECK-FIXES-NEXT: auto G5 = [&]()
   // CHECK-FIXES: int J5 = 8 + R5;
 
   // Alias by reference.
   for (int I = 0; I < N; ++I) {
     auto G6 = [&]() {
       int &R6 = Arr[I];
       int J6 = -1 + R6;
     };
   }
   // CHECK-MESSAGES: :[[@LINE-6]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & R6 : Arr)
-  // CHECK-FIXES-NEXT: auto G6 = [&]() {
+  // CHECK-FIXES-NEXT: auto G6 = [&]()
   // CHECK-FIXES: int J6 = -1 + R6;
 }
 
 void iterators() {
   dependent<int> dep;
 
   for (dependent<int>::iterator I = dep.begin(), E = dep.end(); I != E; ++I)
     auto H1 = [&I]() { int R = *I; };
for (dependent<int>::const_iterator I = dep.begin(), E = dep.end();
        I != E; ++I)
     auto H4 = [&]() { int R = *I + 1; };
 
   for (dependent<int>::const_iterator I = dep.begin(), E = dep.end();
        I != E; ++I)
     auto H5 = [=]() { int R = *I; };
 }
 
+void captureByValue() {
+  // When the index is captured by value, we should replace this by a capture
+  // by reference. This avoids extra copies.
+  // FIXME: this could change semantics on array or pseudoarray loops if the
+  // container is captured by copy.
+  const int N = 10;
+  int Arr[N];
+  dependent<int> Dep;
+
+  for (int I = 0; I < N; ++I) {
+    auto C1 = [&Arr, I]() { if (Arr[I] == 1); };
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : Arr)
+  // CHECK-FIXES-NEXT: auto C1 = [&Arr, &elem]() { if (elem == 1); };
+
+  for (unsigned I = 0; I < Dep.size(); ++I) {
+    auto C2 = [&Dep, I]() { if (Dep[I] == 2); };
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : Dep)
+  // CHECK-FIXES-NEXT: auto C2 = [&Dep, &elem]() { if (elem == 2); };
+}
+
 } // namespace Lambdas
 
 namespace InitLists {
 
 struct D { int i; };
 struct E { D d; };
 int g(int b);
 

test/clang-tidy/modernize-loop-convert-negative.cpp

 } // namespace NegativeIterator
 
 namespace NegativePseudoArray {
 
 const int N = 6;
 dependent<int> v;
 dependent<int> *pv;
 
-transparent<dependent<int>> cv;
 int Sum = 0;
 
 // Checks for the Index start and end:
 void IndexStartAndEnd() {
   for (int I = 0; I < v.size() + 1; ++I)
     Sum += v[I];
 
   for (int I = 0; I < v.size() - 1; ++I)
for (S::iterator I = Exes[Index].getS().begin(),
                    E = Exes[Index].getS().end();
         I != E; ++I) {
     Index++;
     MutableVal K = *I;
   }
 }
 
 } // namespace NegativeMultiEndCall
+
+namespace NoUsages {
+
+const int N = 6;
+int arr[N] = {1, 2, 3, 4, 5, 6};
+S s;
+dependent<int> v;
+int Count = 0;
+
+void foo();
+
+void f() {
+  for (int I = 0; I < N; ++I) {}
+  for (int I = 0; I < N; ++I)
+    printf("Hello world\n");
+  for (int I = 0; I < N; ++I)
+    ++Count;
+  for (int I = 0; I < N; ++I)
+    foo();
+
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I) {}
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+    printf("Hello world\n");
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+    ++Count;
+  for (S::iterator I = s.begin(), E = s.end(); I != E; ++I)
+    foo();
+
+  for (int I = 0; I < v.size(); ++I) {}
+  for (int I = 0; I < v.size(); ++I)
+    printf("Hello world\n");
+  for (int I = 0; I < v.size(); ++I)
+    ++Count;
+  for (int I = 0; I < v.size(); ++I)
+    foo();
+}
+
+} // namespace NoUsages