[llvm] Make Sequence reverse-iterable

This patch simplifies the implementation of Sequence and makes it compatible with llvm::reverse.
It exposes the reverse iterators through rbegin/rend which prevents a dangling reference in std::reverse_iterator::operator++().

Differential Revision: https://reviews.llvm.org/D102679

include/llvm/ADT/Sequence.h

@@ -15,71 +15,167 @@
 #ifndef LLVM_ADT_SEQUENCE_H
 #define LLVM_ADT_SEQUENCE_H
 
-#include "llvm/ADT/iterator.h"
-#include "llvm/ADT/iterator_range.h"
-#include <algorithm>
 #include <iterator>
-#include <utility>
 
 namespace llvm {
 
 namespace detail {
 
-template <typename ValueT>
-class value_sequence_iterator
-    : public iterator_facade_base<value_sequence_iterator<ValueT>,
-                                  std::random_access_iterator_tag,
-                                  const ValueT> {
-  using BaseT = typename value_sequence_iterator::iterator_facade_base;
+template <typename T, bool IsReversed> struct iota_range_iterator {
+  using iterator_category = std::random_access_iterator_tag;
+  using value_type = T;
+  using difference_type = ptrdiff_t;
+  using pointer = T *;
+  using reference = T &;
 
-  ValueT Value;
+private:
+  struct Forward {
+    static void increment(T &V) { ++V; }
+    static void decrement(T &V) { --V; }
+    static void offset(T &V, difference_type Offset) { V += Offset; }
+    static T add(const T &V, difference_type Offset) { return V + Offset; }
+    static difference_type difference(const T &A, const T &B) { return A - B; }
+  };
+
+  struct Reverse {
+    static void increment(T &V) { --V; }
+    static void decrement(T &V) { ++V; }
+    static void offset(T &V, difference_type Offset) { V -= Offset; }
+    static T add(const T &V, difference_type Offset) { return V - Offset; }
+    static difference_type difference(const T &A, const T &B) { return B - A; }
+  };
+
+  using Op = std::conditional_t<!IsReversed, Forward, Reverse>;
 
 public:
-  using difference_type = typename BaseT::difference_type;
-  using reference = typename BaseT::reference;
+  // default-constructible
+  iota_range_iterator() = default;
+  // copy-constructible
+  iota_range_iterator(const iota_range_iterator &) = default;
+  // value constructor
+  explicit iota_range_iterator(T Value) : Value(Value) {}
+  // copy-assignable
+  iota_range_iterator &operator=(const iota_range_iterator &) = default;
+  // destructible
+  ~iota_range_iterator() = default;
 
-  value_sequence_iterator() = default;
-  value_sequence_iterator(const value_sequence_iterator &) = default;
-  value_sequence_iterator(value_sequence_iterator &&Arg)
-      : Value(std::move(Arg.Value)) {}
-  value_sequence_iterator &operator=(const value_sequence_iterator &Arg) {
-    Value = Arg.Value;
-    return *this;
+  // Can be compared for equivalence using the equality/inequality operators,
+  bool operator!=(const iota_range_iterator &RHS) const {
+    return Value != RHS.Value;
   }
-
-  template <typename U, typename Enabler = decltype(ValueT(std::declval<U>()))>
-  value_sequence_iterator(U &&Value) : Value(std::forward<U>(Value)) {}
-
-  value_sequence_iterator &operator+=(difference_type N) {
-    Value += N;
-    return *this;
-  }
-  value_sequence_iterator &operator-=(difference_type N) {
-    Value -= N;
-    return *this;
-  }
-  using BaseT::operator-;
-  difference_type operator-(const value_sequence_iterator &RHS) const {
-    return Value - RHS.Value;
-  }
-
-  bool operator==(const value_sequence_iterator &RHS) const {
+  bool operator==(const iota_range_iterator &RHS) const {
     return Value == RHS.Value;
   }
-  bool operator<(const value_sequence_iterator &RHS) const {
-    return Value < RHS.Value;
+
+  // Comparison
+  bool operator<(const iota_range_iterator &Other) const {
+    return Op::difference(Value, Other.Value) < 0;
+  }
+  bool operator<=(const iota_range_iterator &Other) const {
+    return Op::difference(Value, Other.Value) <= 0;
+  }
+  bool operator>(const iota_range_iterator &Other) const {
+    return Op::difference(Value, Other.Value) > 0;
+  }
+  bool operator>=(const iota_range_iterator &Other) const {
+    return Op::difference(Value, Other.Value) >= 0;
   }
 
-  reference operator*() const { return Value; }
+  // Dereference
+  T operator*() const { return Value; }
+  T operator[](difference_type Offset) const { return Op::add(Value, Offset); }
+
+  // Arithmetic
+  iota_range_iterator operator+(difference_type Offset) const {
+    return {Op::add(Value, Offset)};
+  }
+  iota_range_iterator operator-(difference_type Offset) const {
+    return {Op::add(Value, -Offset)};
+  }
+
+  // Iterator difference
+  difference_type operator-(const iota_range_iterator &Other) const {
+    return Op::difference(Value, Other.Value);
+  }
+
+  // Pre/Post Increment
+  iota_range_iterator &operator++() {
+    Op::increment(Value);
+    return *this;
+  }
+  iota_range_iterator operator++(int) {
+    iota_range_iterator Tmp = *this;
+    Op::increment(Value);
+    return Tmp;
+  }
+
+  // Pre/Post Decrement
+  iota_range_iterator &operator--() {
+    Op::decrement(Value);
+    return *this;
+  }
+  iota_range_iterator operator--(int) {
+    iota_range_iterator Tmp = *this;
+    Op::decrement(Value);
+    return Tmp;
+  }
+
+  // Compound assignment operators
+  iota_range_iterator &operator+=(difference_type Offset) {
+    Op::offset(Value, Offset);
+    return *this;
+  }
+  iota_range_iterator &operator-=(difference_type Offset) {
+    Op::offset(Value, -Offset);
+    return *this;
+  }
+
+private:
+  T Value;
 };
 
-} // end namespace detail
+} // namespace detail
 
-template <typename ValueT>
-iterator_range<detail::value_sequence_iterator<ValueT>> seq(ValueT Begin,
-                                                            ValueT End) {
-  return make_range(detail::value_sequence_iterator<ValueT>(Begin),
-                    detail::value_sequence_iterator<ValueT>(End));
+template <typename ValueT> struct iota_range {
+  static_assert(std::is_integral<ValueT>::value,
+                "ValueT must be an integral type");
+
+  using value_type = ValueT;
+  using reference = ValueT &;
+  using const_reference = const ValueT &;
+  using iterator = detail::iota_range_iterator<value_type, false>;
+  using const_iterator = iterator;
+  using reverse_iterator = detail::iota_range_iterator<value_type, true>;
+  using const_reverse_iterator = reverse_iterator;
+  using difference_type = typename iterator::difference_type;
+  using size_type = std::size_t;
+
+  value_type Begin;
+  value_type End;
+
+  template <
+      typename BeginT, typename EndT,
+      std::enable_if_t<std::is_convertible<BeginT, ValueT>::value, bool> = true,
+      std::enable_if_t<std::is_convertible<EndT, ValueT>::value, bool> = true>
+  iota_range(BeginT &&Begin, EndT &&End)
+      : Begin(std::forward<BeginT>(Begin)), End(std::forward<EndT>(End)) {}
+
+  size_t size() const { return End - Begin; }
+  bool empty() const { return Begin == End; }
+
+  auto begin() const { return const_iterator(Begin); }
+  auto end() const { return const_iterator(End); }
+
+  auto rbegin() const { return const_reverse_iterator(End - 1); }
+  auto rend() const { return const_reverse_iterator(Begin - 1); }
+
+private:
+  static_assert(std::is_same<ValueT, std::remove_cv_t<ValueT>>::value,
+                "ValueT must not be const nor volatile");
+};
+
+template <typename ValueT> auto seq(ValueT Begin, ValueT End) {
+  return iota_range<ValueT>(std::move(Begin), std::move(End));
 }
 
 } // end namespace llvm

unittests/ADT/SequenceTest.cpp

@@ -15,26 +15,37 @@ using namespace llvm;
 
 namespace {
 
-TEST(SequenceTest, Basic) {
-  int x = 0;
-  for (int i : seq(0, 10)) {
-    EXPECT_EQ(x, i);
-    x++;
+TEST(SequenceTest, Forward) {
+  int X = 0;
+  for (int I : seq(0, 10)) {
+    EXPECT_EQ(X, I);
+    ++X;
   }
-  EXPECT_EQ(10, x);
+  EXPECT_EQ(10, X);
+}
 
-  auto my_seq = seq(0, 4);
-  EXPECT_EQ(4, my_seq.end() - my_seq.begin());
-  for (int i : {0, 1, 2, 3})
-    EXPECT_EQ(i, (int)my_seq.begin()[i]);
+TEST(SequenceTest, Backward) {
+  int X = 9;
+  for (int I : reverse(seq(0, 10))) {
+    EXPECT_EQ(X, I);
+    --X;
+  }
+  EXPECT_EQ(-1, X);
+}
 
-  EXPECT_TRUE(my_seq.begin() < my_seq.end());
+TEST(SequenceTest, Distance) {
+  const auto Forward = seq(0, 10);
+  EXPECT_EQ(std::distance(Forward.begin(), Forward.end()), 10);
+  EXPECT_EQ(std::distance(Forward.rbegin(), Forward.rend()), 10);
+}
 
-  auto adjusted_begin = my_seq.begin() + 2;
-  auto adjusted_end = my_seq.end() - 2;
-  EXPECT_TRUE(adjusted_begin == adjusted_end);
-  EXPECT_EQ(2, *adjusted_begin);
-  EXPECT_EQ(2, *adjusted_end);
+TEST(SequenceTest, Dereferene) {
+  const auto Forward = seq(0, 10).begin();
+  EXPECT_EQ(Forward[0], 0);
+  EXPECT_EQ(Forward[2], 2);
+  const auto Backward = seq(0, 10).rbegin();
+  EXPECT_EQ(Backward[0], 9);
+  EXPECT_EQ(Backward[2], 7);
 }
 
 } // anonymous namespace