[PATCH] D17870: [ADT] Add an 'llvm::seq' function which produces an iterator range over a sequence of values. It increments through the values in the half-open range: [Begin, End), producing those values when indirecting the iterator. It should support integers...

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Updated and with answers below...

In http://reviews.llvm.org/D17870#367653, @llvm-commits wrote:

> Hi Chandler
>
> Haven’t looked at the code yet, but I like the idea and the use of it you gave.
>
> Seems like most integer cases are going to start at 0.  Could we have a seq(n) to handle [0, n) ranges?


We could, but I'm inclined to write the '0, '. It's pretty short, and I'm a bit hesitant to add too many overloads with different arity unless there is a significant inconvenience.


================
Comment at: include/llvm/ADT/iterator.h:139
@@ -122,3 +138,3 @@
   }
-  ReferenceT operator[](DifferenceTypeT n) const {
+  ReferenceProxy operator[](DifferenceTypeT n) const {
     static_assert(IsRandomAccess,
----------------
dblaikie wrote:
> Not entirely following here, so I might be completely in the weeds, but is this any better than simply returning by value? (I suppose this implementation supports both cases where the value isn't copyable/must be referenced and where the iterator/value is transient and must be copied, then?)
The spec says convertible to a reference. In part, if the iterator is mutable, you're required to be able to write through the reference. =/

================
Comment at: unittests/ADT/SequenceTest.cpp:21
@@ +20,3 @@
+
+TEST(SequenceTest, Basic) {
+  int x = 0;
----------------
dblaikie wrote:
> Test with a type that supports unary ++/-- but not binary +/- (you mention this in comments in the implementation about why we can't implement unary ++/-- in terms of binary +/-)?
> 
> Possibly test that a sequence iterator over such a non-random-access sequence is itself, not random access (so we don't get quiet O(N) binary +/-)?
Bleh, the non-random-access thing just doesn't work. I have to choose a tag for the iterator category, and that ends up dictating everything. I've removed the code trying to support non-random-access stuff.

In order to make that work if we ever want too, we'd need to build a type function to select a iterator category based on what operators are supported.... But that seems like *waaay* too much complexity at this juncture. For now, I've reduced it to the simple form.


http://reviews.llvm.org/D17870