[PATCH] D30853: Improve the genericity of `llvm::enumerate()`.

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

Will address the rest of the comments in a later followup.



================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1062
+  enumerator_iter<R> end() {
+    return enumerator_iter<R>(-1, std::end(TheRange));
+  }
----------------
dblaikie wrote:
> If the index of the end is going to be -1, it could be used to quickly catch a few mistakes with iterators - assert(index != -1) in operator++, for example?
-1 is sufficient to be end, but not necessary.  For example, when ++'ing a valid iterator, you don't know if it's end without having the end of the backing container.  So in that case index will just be whatever.  Such an assert would still be fine, but it would only guarantee that you're not incrementing the result of std::end(enumerate(R)), and it wouldn't trigger when you increment an iterator past it's actual end.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1065-1067
+  template <unsigned N> SmallVector<result_pair<R>, N> toVector() const {
+    return SmallVector<result_pair<R>, N>(begin(), end());
+  }
----------------
dblaikie wrote:
> Once there's a generic utility function for all ranges as you've provided with to_vector - is this member still useful/necessary? (indeed this function seems to be untested - the test case exercises the generic to_vector instead)
This was from some early attempts that is not needed anymore.  I meant to remove it, but forgot.  Thanks for catching it.


https://reviews.llvm.org/D30853