[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...

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 3 14:54:59 PST 2016


dblaikie added a subscriber: dblaikie.

================
Comment at: include/llvm/ADT/Sequence.h:41
@@ +40,3 @@
+  template <typename U>
+  value_sequence_iterator(U &&Value) : Value(std::forward<U &&>(Value)) {}
+
----------------
that should be forward<U>, I think?

================
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,
----------------
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?)

================
Comment at: unittests/ADT/SequenceTest.cpp:21
@@ +20,3 @@
+
+TEST(SequenceTest, Basic) {
+  int x = 0;
----------------
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 +/-)?


http://reviews.llvm.org/D17870





More information about the llvm-commits mailing list