Use 'auto const&' for iterators whose deref operator return a const var

This patch fixes PR15601.
- Added check for whether the loop variable and the initializer have the same
type.
- Added tests.

llvm-svn: 181528

5 files changed, 115 insertions, 15 deletions

diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h b/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h
index e37f2b54794..9dd04f42b5a 100644
--- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h
+++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/Inputs/structures.h
@@ -131,10 +131,13 @@ class transparent {
 template<typename IteratorType>
 struct Nested {
   typedef IteratorType* iterator;
+  typedef const IteratorType* const_iterator;
   IteratorType *operator->();
   IteratorType operator*();
   iterator begin();
   iterator end();
+  const_iterator begin() const;
+  const_iterator end() const;
 };
 
 // Like llvm::SmallPtrSet, the iterator has a dereference operator that returns
diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp
index 53030b50f03..3d98c0bca24 100644
--- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp
+++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/iterator.cpp
@@ -23,20 +23,20 @@ void f() {
   // CHECK-NEXT: printf("I found %d\n", elem);
 
   S s;
-  for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) {
+  for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
     printf("s has value %d\n", (*it).x);
   }
   // CHECK: for (auto & elem : s)
   // CHECK-NEXT: printf("s has value %d\n", (elem).x);
 
   S *ps;
-  for (S::const_iterator it = ps->begin(), e = ps->end(); it != e; ++it) {
+  for (S::iterator it = ps->begin(), e = ps->end(); it != e; ++it) {
     printf("s has value %d\n", (*it).x);
   }
   // CHECK: for (auto & p : *ps)
   // CHECK-NEXT: printf("s has value %d\n", (p).x);
 
-  for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) {
+  for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
     printf("s has value %d\n", it->x);
   }
   // CHECK: for (auto & elem : s)
@@ -80,18 +80,18 @@ void f() {
   // CHECK-NEXT: int k = A->x + elem.x;
 
   dependent<int> v;
-  for (dependent<int>::const_iterator it = v.begin(), e = v.end();
+  for (dependent<int>::iterator it = v.begin(), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK: for (auto & elem : v)
+  // CHECK: for (auto & elem : v) {
   // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
 
-  for (dependent<int>::const_iterator it(v.begin()), e = v.end();
+  for (dependent<int>::iterator it(v.begin()), e = v.end();
        it != e; ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK: for (auto & elem : v)
+  // CHECK: for (auto & elem : v) {
   // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
 
   doublyDependent<int,int> intmap;
@@ -134,6 +134,46 @@ void f() {
   }
 }
 
+// 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.
+  S s;
+  for (S::const_iterator it = s.begin(), e = s.end(); it != e; ++it) {
+    printf("s has value %d\n", (*it).x);
+  }
+  // CHECK: for (auto const & elem : s)
+  // CHECK-NEXT: printf("s has value %d\n", (elem).x);
+
+  S *ps;
+  for (S::const_iterator it = ps->begin(), e = ps->end(); it != e; ++it) {
+    printf("s has value %d\n", (*it).x);
+  }
+  // CHECK: for (auto const & p : *ps)
+  // CHECK-NEXT: printf("s has value %d\n", (p).x);
+
+  // 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: for (dependent<int>::const_iterator it = v.begin(), e = v.end();
+  // CHECK-NEXT: it != e; ++it) {
+  // CHECK-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: for (dependent<int>::const_iterator it(v.begin()), e = v.end();
+  // CHECK-NEXT: it != e; ++it) {
+  // CHECK-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.
diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp
index 050f44ef900..9597c63a6f2 100644
--- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp
+++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/naming-conflict.cpp
@@ -32,7 +32,7 @@ void sameNames() {
 
 void macroConflict() {
   S MAXs;
-  for (S::const_iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) {
+  for (S::iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) {
     printf("s has value %d\n", (*it).x);
     printf("Max of 3 and 5: %d\n", MAX(3,5));
   }
@@ -40,6 +40,14 @@ void macroConflict() {
   // CHECK-NEXT: printf("s has value %d\n", (MAXs_it).x);
   // CHECK-NEXT: printf("Max of 3 and 5: %d\n", MAX(3,5));
 
+  for (S::const_iterator it = MAXs.begin(), e = MAXs.end(); it != e; ++it) {
+    printf("s has value %d\n", (*it).x);
+    printf("Max of 3 and 5: %d\n", MAX(3,5));
+  }
+  // CHECK: for (auto const & MAXs_it : MAXs)
+  // CHECK-NEXT: printf("s has value %d\n", (MAXs_it).x);
+  // CHECK-NEXT: printf("Max of 3 and 5: %d\n", MAX(3,5));
+
   T DEFs;
   for (T::iterator it = DEFs.begin(), e = DEFs.end(); it != e; ++it) {
     if (*it == DEF) {
diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp
index 729b5ff21a6..b26cf218d77 100644
--- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp
+++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/nesting.cpp
@@ -54,4 +54,16 @@ void f() {
   // CHECK: for (auto & elem : NestT) {
   // CHECK-NEXT: for (T::iterator TI = (elem).begin(), TE = (elem).end(); TI != TE; ++TI) {
   // CHECK-NEXT: printf("%d", *TI);
+
+  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);
+    }
+  }
+  // The inner loop is also convertible, but doesn't need to be converted
+  // immediately. Update this test when that changes!
+  // CHECK: for (auto const & elem : NestS) {
+  // CHECK-NEXT: for (S::const_iterator SI = (elem).begin(), SE = (elem).end(); SI != SE; ++SI) {
+  // CHECK-NEXT: printf("%d", *SI);
 }
diff --git a/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp b/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp
index 91b6ea5b2f7..c717ed6c5e2 100644
--- a/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp
+++ b/clang-tools-extra/test/cpp11-migrate/LoopConvert/single-iterator.cpp
@@ -34,20 +34,20 @@ void f() {
   // CHECK-NEXT: printf("I found %d\n", elem);
 
   S s;
-  for (S::const_iterator it = s.begin(); it != s.end(); ++it) {
+  for (S::iterator it = s.begin(); it != s.end(); ++it) {
     printf("s has value %d\n", (*it).x);
   }
   // CHECK: for (auto & elem : s)
   // CHECK-NEXT: printf("s has value %d\n", (elem).x);
 
   S *ps;
-  for (S::const_iterator it = ps->begin(); it != ps->end(); ++it) {
+  for (S::iterator it = ps->begin(); it != ps->end(); ++it) {
     printf("s has value %d\n", (*it).x);
   }
   // CHECK: for (auto & p : *ps)
   // CHECK-NEXT: printf("s has value %d\n", (p).x);
 
-  for (S::const_iterator it = s.begin(); it != s.end(); ++it) {
+  for (S::iterator it = s.begin(); it != s.end(); ++it) {
     printf("s has value %d\n", it->x);
   }
   // CHECK: for (auto & elem : s)
@@ -91,18 +91,18 @@ void f() {
   // CHECK-NEXT: int k = A->x + elem.x;
 
   dependent<int> v;
-  for (dependent<int>::const_iterator it = v.begin();
+  for (dependent<int>::iterator it = v.begin();
        it != v.end(); ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK: for (auto & elem : v)
+  // CHECK: for (auto & elem : v) {
   // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
 
-  for (dependent<int>::const_iterator it(v.begin());
+  for (dependent<int>::iterator it(v.begin());
        it != v.end(); ++it) {
     printf("Fibonacci number is %d\n", *it);
   }
-  // CHECK: for (auto & elem : v)
+  // CHECK: for (auto & elem : v) {
   // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
 
   doublyDependent<int,int> intmap;
@@ -113,3 +113,40 @@ void f() {
   // CHECK: for (auto & elem : intmap)
   // CHECK-NEXT: printf("intmap[%d] = %d", elem.first, elem.second);
 }
+
+void different_type() {
+  // 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.
+
+  // s.begin() returns a type 'iterator' which is just a non-const pointer and
+  // differs from const_iterator only on the const qualification.
+  S s;
+  for (S::const_iterator it = s.begin(); it != s.end(); ++it) {
+    printf("s has value %d\n", (*it).x);
+  }
+  // CHECK: for (auto const & elem : s)
+  // CHECK-NEXT: printf("s has value %d\n", (elem).x);
+
+  S *ps;
+  for (S::const_iterator it = ps->begin(); it != ps->end(); ++it) {
+    printf("s has value %d\n", (*it).x);
+  }
+  // CHECK: for (auto const & p : *ps)
+  // CHECK-NEXT: printf("s has value %d\n", (p).x);
+
+  // 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(); it != v.end(); ++it) {
+    printf("Fibonacci number is %d\n", *it);
+  }
+  // CHECK: for (dependent<int>::const_iterator it = v.begin(); it != v.end(); ++it) {
+  // CHECK-NEXT: printf("Fibonacci number is %d\n", *it);
+
+  for (dependent<int>::const_iterator it(v.begin()); it != v.end(); ++it) {
+    printf("Fibonacci number is %d\n", *it);
+  }
+  // CHECK: for (dependent<int>::const_iterator it(v.begin()); it != v.end(); ++it) {
+  // CHECK-NEXT: printf("Fibonacci number is %d\n", *it);
+}