Avoid naming conflicts with the old index in modernize-loop-convert.

Summary: The old index declaration is going to be removed anyway, so we can reuse its name if it is the best candidate for the new index.

Reviewers: klimek

Subscribers: cfe-commits, alexfh

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

llvm-svn: 252303

2 files changed, 23 insertions, 5 deletions

diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index f1fd078fa01..1f2b40da977 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -820,14 +820,14 @@ std::string VariableNamer::createIndexName() {
   if (Len > 1 && ContainerName.endswith(Style == NS_UpperCase ? "S" : "s")) {
     IteratorName = ContainerName.substr(0, Len - 1);
     // E.g.: (auto thing : things)
-    if (!declarationExists(IteratorName))
+    if (!declarationExists(IteratorName) || IteratorName == OldIndex->getName())
       return IteratorName;
   }
 
   if (Len > 2 && ContainerName.endswith(Style == NS_UpperCase ? "S_" : "s_")) {
     IteratorName = ContainerName.substr(0, Len - 2);
     // E.g.: (auto thing : things_)
-    if (!declarationExists(IteratorName))
+    if (!declarationExists(IteratorName) || IteratorName == OldIndex->getName())
       return IteratorName;
   }
 
@@ -849,12 +849,12 @@ std::string VariableNamer::createIndexName() {
 
   IteratorName = AppendWithStyle(ContainerName, OldIndex->getName());
   // E.g.: (auto container_i : container)
-  if (!declarationExists(IteratorName))
+  if (!declarationExists(IteratorName) || IteratorName == OldIndex->getName())
     return IteratorName;
 
   IteratorName = AppendWithStyle(ContainerName, Elem);
   // E.g.: (auto container_elem : container)
-  if (!declarationExists(IteratorName))
+  if (!declarationExists(IteratorName) || IteratorName == OldIndex->getName())
     return IteratorName;
 
   // Someone defeated my naming scheme...
@@ -875,7 +875,8 @@ std::string VariableNamer::createIndexName() {
   int Attempt = 0;
   do {
     IteratorName = GiveMeName + std::to_string(Attempt++);
-  } while (declarationExists(IteratorName));
+  } while (declarationExists(IteratorName) ||
+           IteratorName == OldIndex->getName());
 
   return IteratorName;
 }
diff --git a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
index 576dedcdafa..254c86177c6 100644
--- a/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
+++ b/clang-tools-extra/test/clang-tidy/modernize-loop-convert-extra.cpp
@@ -325,6 +325,23 @@ void sameNames() {
   // CHECK-FIXES-NEXT: (void)NumsI;
 }
 
+void oldIndexConflict() {
+  for (int Num = 0; Num < N; ++Num) {
+    printf("Num: %d\n", Nums[Num]);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int Num : Nums)
+  // CHECK-FIXES-NEXT: printf("Num: %d\n", Num);
+
+  S Things;
+  for (S::iterator Thing = Things.begin(), End = Things.end(); Thing != End; ++Thing) {
+    printf("Thing: %d %d\n", Thing->X, (*Thing).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & Thing : Things)
+  // CHECK-FIXES-NEXT: printf("Thing: %d %d\n", Thing.X, Thing.X);
+}
+
 void macroConflict() {
   S MAXs;
   for (S::iterator It = MAXs.begin(), E = MAXs.end(); It != E; ++It) {