[PATCH] Use 'auto const&' for iterators whose deref operator returns a const var

Edwin Vane edwin.vane at intel.com
Wed May 8 12:56:34 PDT 2013


  Since I've asked you to change many of the iterator types to respect the original reasons for those tests, you now need to add more tests that explicitly exercise all the code paths you've added.


================
Comment at: test/cpp11-migrate/LoopConvert/iterator.cpp:29
@@ -28,3 +28,3 @@
   }
-  // CHECK: for (auto & elem : s)
+  // CHECK: for (auto const & elem : s)
   // CHECK-NEXT: printf("s has value %d\n", (elem).x);
----------------
Rather than updating these tests to expect const, I think it's better the iterator type is just changed to S::iterator and make the variable type the same as the initializer, thus removing an extra variable from the test. This is supposed to be a basic test.

================
Comment at: test/cpp11-migrate/LoopConvert/iterator.cpp:87
@@ -86,3 +86,3 @@
   }
-  // CHECK: for (auto & elem : v)
-  // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK: for (dependent<int>::const_iterator it = v.begin(), e = v.end();
+  // CHECK-NEXT: it != e; ++it) {
----------------
Same idea here. Should use dependent<int>::iterator. The point of this test is to test whether dependent container types work.

================
Comment at: test/cpp11-migrate/LoopConvert/iterator.cpp:179
@@ -176,3 +178,3 @@
     for (const_iterator I = begin(), E = end(); I != E; ++I) {
-      // CHECK: for (auto & elem : *this) {
+      // CHECK: for (auto const & elem : *this) {
     }
----------------
This is actually wrong. The type returned by this->begin() should be const_iterator. Types should match and therefore we should get just auto & elem. Check the code for why.

================
Comment at: test/cpp11-migrate/LoopConvert/naming-conflict.cpp:39
@@ -38,3 +38,3 @@
   }
-  // CHECK: for (auto & MAXs_it : MAXs)
+  // CHECK: for (auto const & MAXs_it : MAXs)
   // CHECK-NEXT: printf("s has value %d\n", (MAXs_it).x);
----------------
Change the iterator type to S::iterator.

================
Comment at: test/cpp11-migrate/LoopConvert/single-iterator.cpp:40
@@ -39,3 +39,3 @@
   }
-  // CHECK: for (auto & elem : s)
+  // CHECK: for (auto const & elem : s)
   // CHECK-NEXT: printf("s has value %d\n", (elem).x);
----------------
Change to S::iterator.

================
Comment at: test/cpp11-migrate/LoopConvert/single-iterator.cpp:98
@@ -97,3 +97,3 @@
   }
-  // CHECK: for (auto & elem : v)
-  // CHECK-NEXT: printf("Fibonacci number is %d\n", elem);
+  // CHECK: for (dependent<int>::const_iterator it = v.begin();
+  // CHECK-NEXT: it != v.end(); ++it) {
----------------
Change to dependent<int>::iterator.


http://llvm-reviews.chandlerc.com/D759



More information about the cfe-commits mailing list