Avoid LoopConvertCheck replacements in template instantiations.

[angelgarcia]

Summary

Prevent LoopConvertCheck from doing replacements in template instantiations, and add a test.

[alexfh]

Looks good with a comment.

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

636

Please add a test to ensure that loops over non-dependent container types in templates are still converted.

[angelgarcia]

Add test.

[alexfh]

Thanks! Still looks good.

clang-tidy/modernize/LoopConvertCheck.cpp

 ///     VarDecl.
 ///   - 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()),
              hasLoopInit(declStmt(hasSingleDecl(InitToZeroMatcher))),
              hasCondition(anyOf(
                  binaryOperator(hasOperatorName("<"),
                                 hasLHS(IntegerComparisonMatcher),
                                 hasRHS(ArrayBoundMatcher)),
                  binaryOperator(hasOperatorName(">"), hasLHS(ArrayBoundMatcher),
                                 hasRHS(IntegerComparisonMatcher)))),
              hasIncrement(unaryOperator(hasOperatorName("++"),
internal::Matcher<VarDecl> TestDerefReturnsByValue =
                           .bind(DerefByValueResultName)),
               returns(
                   // Skip loops where the iterator's operator* returns an
                   // rvalue reference. This is just weird.
                   qualType(unless(hasCanonicalType(rValueReferenceType())))
                       .bind(DerefByRefResultName)))))));
 
   return forStmt(
+             unless(isInTemplateInstantiation()),
              hasLoopInit(anyOf(declStmt(declCountIs(2),
                                         containsDeclaration(0, InitDeclMatcher),
                                         containsDeclaration(1, EndDeclMatcher)),
                                declStmt(hasSingleDecl(InitDeclMatcher)))),
              hasCondition(
                  anyOf(binaryOperator(hasOperatorName("!="),
                                       hasLHS(IteratorComparisonMatcher),
                                       hasRHS(IteratorBoundMatcher)),
DeclarationMatcher EndDeclMatcher =
       varDecl(hasInitializer(EndInitMatcher)).bind(EndVarName);
 
   StatementMatcher IndexBoundMatcher =
       expr(anyOf(ignoringParenImpCasts(declRefExpr(to(
                      varDecl(hasType(isInteger())).bind(ConditionEndVarName)))),
                  EndInitMatcher));
 
   return forStmt(
+             unless(isInTemplateInstantiation()),
              hasLoopInit(
                  anyOf(declStmt(declCountIs(2),
                                 containsDeclaration(0, InitToZeroMatcher),
                                 containsDeclaration(1, EndDeclMatcher)),
                        declStmt(hasSingleDecl(InitToZeroMatcher)))),
              hasCondition(anyOf(
                  binaryOperator(hasOperatorName("<"),
                                 hasLHS(IntegerComparisonMatcher),

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

void messing_with_macros() {
   // CHECK-FIXES-NEXT:  printf("Value: %d\n", elem);
 
   for (int i = 0; i < N; ++i) {
     printf("Value: %d\n", CONT arr[i]);
   }
 }
 
 } // 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) {
+  }
+  S s;
+  for (S::iterator SI = s.begin(), SE = s.end(); SI != SE; ++SI) {
+  }
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : s) {
+}
+
+void template_instantiation() {
+  S a;
+  set_union(a);
+}
+
+} // namespace Templates