[PATCH] D12530: Fix several corner cases for loop-convert check.

[Manuel Klimek via cfe-commits]

klimek added a comment.

Nice!


================
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:378
@@ +377,3 @@
+  for (const auto &U : Usages) {
+    if (!U.E->isRValue())
+      return false;
----------------
(not necessarily in this CL) please rename E to Expression or similar; I'm fine with one-letter variables for small scopes, but struct scopes are basically infinite.

================
Comment at: test/clang-tidy/modernize-loop-convert-basic.cpp:437
@@ +436,3 @@
+
+  for (S::iterator it = s.begin(), e = s.end(); it != e; ++it) {
+    foo(it);
----------------
Use LLVM coding conventions for iterators (I, E) as above.

================
Comment at: test/clang-tidy/modernize-loop-convert-basic.cpp:448
@@ +447,3 @@
+    ret = it;
+  }
+}
----------------
This test seems to be missing the it.insert(0) case that was removed from the "unsupported"  comment, if I'm not missing something.

================
Comment at: test/clang-tidy/modernize-loop-convert-basic.cpp:540-541
@@ -518,4 +539,4 @@
   unsigned size() const;
-  unsigned begin() const;
-  unsigned end() const;
+  unsigned* begin() const;
+  unsigned* end() const;
 };
----------------
Isn't it important that it's a pointer to an unsigned const, not that the iterator method is const?


http://reviews.llvm.org/D12530