[clang-tidy] Extend LoopConvert on array with `!=` comparison

[njames93]

Summary

Enables transforming loops of the form:

for (int i = 0; I != container.size(); ++I) { container[I]...; }
for (int i = 0; I != N; ++I) { FixedArrSizeN[I]...; }

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp

return varDecl(
              hasInitializer(ignoringParenImpCasts(integerLiteral(equals(0)))))
       .bind(InitVarName);
 }
 
 static const StatementMatcher incrementVarMatcher() {
   return declRefExpr(to(varDecl(equalsBoundNode(InitVarName))));
 }
 
+static StatementMatcher
+arrayConditionMatcher(internal::Matcher<Expr> LimitExpr) {
+  return binaryOperator(
+      anyOf(allOf(hasOperatorName("<"), hasLHS(integerComparisonMatcher()),
+                  hasRHS(LimitExpr)),
+            allOf(hasOperatorName(">"), hasLHS(LimitExpr),
+                  hasRHS(integerComparisonMatcher())),
+            allOf(hasOperatorName("!="),
+                  hasOperands(integerComparisonMatcher(), LimitExpr))));
+}
+
 /// The matcher for loops over arrays.
 /// \code
 ///   for (int i = 0; i < 3 + 2; ++i) { ... }
 /// \endcode
 /// The following string identifiers are bound to these parts of the AST:
 ///   ConditionBoundName: '3 + 2' (as an Expr)
 ///   InitVarName: 'i' (as a VarDecl)
 ///   LoopName: The entire for loop (as a ForStmt)
 ///
 /// Client code will need to make sure that:
 ///   - The index variable is only used as an array index.
 ///   - All arrays indexed by the loop are the same.
 StatementMatcher makeArrayLoopMatcher() {
   StatementMatcher ArrayBoundMatcher =
       expr(hasType(isInteger())).bind(ConditionBoundName);
 
-  return forStmt(
-             unless(isInTemplateInstantiation()),
+  return forStmt(unless(isInTemplateInstantiation()),
                  hasLoopInit(declStmt(hasSingleDecl(initToZeroMatcher()))),
-             hasCondition(anyOf(
-                 binaryOperator(hasOperatorName("<"),
-                                hasLHS(integerComparisonMatcher()),
-                                hasRHS(ArrayBoundMatcher)),
-                 binaryOperator(hasOperatorName(">"), hasLHS(ArrayBoundMatcher),
-                                hasRHS(integerComparisonMatcher())))),
+                 hasCondition(arrayConditionMatcher(ArrayBoundMatcher)),
                  hasIncrement(
                      unaryOperator(hasOperatorName("++"),
                                    hasUnaryOperand(incrementVarMatcher()))))
       .bind(LoopNameArray);
 }
 
 /// The matcher used for iterator-based for loops.
 ///
 /// This matcher is more flexible than array-based loops. It will match
 /// catch loops of the following textual forms (regardless of whether the
 /// iterator type is actually a pointer type or a class type):
StatementMatcher makePseudoArrayLoopMatcher() {
   DeclarationMatcher EndDeclMatcher =
       varDecl(hasInitializer(EndInitMatcher)).bind(EndVarName);
 
   StatementMatcher IndexBoundMatcher =
       expr(anyOf(ignoringParenImpCasts(
                      declRefExpr(to(varDecl(equalsBoundNode(EndVarName))))),
                  EndInitMatcher));
 
-  return forStmt(
-             unless(isInTemplateInstantiation()),
+  return forStmt(unless(isInTemplateInstantiation()),
                  hasLoopInit(
                      anyOf(declStmt(declCountIs(2),
                                     containsDeclaration(0, initToZeroMatcher()),
                                     containsDeclaration(1, EndDeclMatcher)),
                            declStmt(hasSingleDecl(initToZeroMatcher())))),
-             hasCondition(anyOf(
-                 binaryOperator(hasOperatorName("<"),
-                                hasLHS(integerComparisonMatcher()),
-                                hasRHS(IndexBoundMatcher)),
-                 binaryOperator(hasOperatorName(">"), hasLHS(IndexBoundMatcher),
-                                hasRHS(integerComparisonMatcher())))),
+                 hasCondition(arrayConditionMatcher(IndexBoundMatcher)),
                  hasIncrement(
                      unaryOperator(hasOperatorName("++"),
                                    hasUnaryOperand(incrementVarMatcher()))))
       .bind(LoopNamePseudoArray);
 }
 
 /// Determine whether Init appears to be an initializing an iterator.
 ///
 /// If it is, returns the object whose begin() or end() method is called, and
 /// the output parameter isArrow is set to indicate whether the initialization
 /// is called via . or ->.

clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp

void f() {
 
   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-NEXT: Tea.g();
+
+  for (int I = 0; N > I; ++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 (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0; N != I; ++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 (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  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 (int & I : Arr)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number %d has address %p\n", I, &I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
 }
 
 const int *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 (int I : ConstArr)
void f() {
   for (int I = 0; I < Cv->size(); ++I) {
     printf("Fibonacci number is %d\n", Cv->at(I));
     Sum += Cv->at(I) + 2;
   }
   // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (int I : *Cv)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); E > I; ++I) {
+    printf("Fibonacci number is %d\n", V[I]);
+    Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); I != E; ++I) {
+    printf("Fibonacci number is %d\n", V[I]);
+    Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = V.size(); E != I; ++I) {
+    printf("Fibonacci number is %d\n", V[I]);
+    Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
 }
 
 // Ensure that 'const auto &' is used with containers of non-trivial types.
 void constness() {
   int Sum = 0;
   for (int I = 0, E = Constv.size(); I < E; ++I) {
     printf("Fibonacci number is %d\n", Constv[I].X);
     Sum += Constv[I].X + 2;