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

[Justin Bogner via llvm-commits]

Chandler Carruth via llvm-commits <llvm-commits at lists.llvm.org> writes:
> chandlerc created this revision.
> chandlerc added reviewers: hfinkel, dexonsmith.
> chandlerc added a subscriber: llvm-commits.
> Herald added a subscriber: mcrosier.
>
> ..., iterators, and any other type providing these basic arithmetic operations.
>
> This came up in the C++ standards committee meeting, and it seemed like
> a useful construct that LLVM might want as well, and I wanted to
> understand how easily we could solve it. I suspect this can be used to
> write simpler counting loops even in LLVM along the lines of:
>
>   for (int i : seq(0, v.size())) {
>     ...
>   };

I don't get how this gains us anything. this adds a bunch of code to
save, well, three characters over a regular loop:

    for (int i = 0; i < v.size(); ++i) {
      ...
    }

I suppose it could be useful if/when we have higher order operations on
ranges, but just to replace a for loop it just adds another way to do
something for no reason. I'm against adopting this without a use-case
that's more interesting than "a new way to write a for loop!".

>
> As part of this, I had to fix the lack of a proxy object returned from
> the operator[] in our iterator facade.
>
> http://reviews.llvm.org/D17870
>
> Files:
>   include/llvm/ADT/Sequence.h
>   include/llvm/ADT/iterator.h
>   unittests/ADT/CMakeLists.txt
>   unittests/ADT/SequenceTest.cpp
>
> Index: unittests/ADT/SequenceTest.cpp
> ===================================================================
> --- /dev/null
> +++ unittests/ADT/SequenceTest.cpp
> @@ -0,0 +1,41 @@
> +//===- SequenceTest.cpp - Unit tests for a sequence abstraciton -----------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "llvm/ADT/Sequence.h"
> +#include "gtest/gtest.h"
> +
> +#include <iterator>
> +#include <list>
> +#include <vector>
> +
> +using namespace llvm;
> +
> +namespace {
> +
> +TEST(SequenceTest, Basic) {
> +  int x = 0;
> +  for (int i : seq(0, 10))
> +    EXPECT_EQ(x++, i);
> +  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]);
> +
> +  EXPECT_TRUE(my_seq.begin() < my_seq.end());
> +
> +  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);
> +}
> +
> +} // anonymous namespace
> Index: unittests/ADT/CMakeLists.txt
> ===================================================================
> --- unittests/ADT/CMakeLists.txt
> +++ unittests/ADT/CMakeLists.txt
> @@ -32,6 +32,7 @@
>    PostOrderIteratorTest.cpp
>    RangeAdapterTest.cpp
>    SCCIteratorTest.cpp
> +  SequenceTest.cpp
>    SmallPtrSetTest.cpp
>    SmallStringTest.cpp
>    SmallVectorTest.cpp
> Index: include/llvm/ADT/iterator.h
> ===================================================================
> --- include/llvm/ADT/iterator.h
> +++ include/llvm/ADT/iterator.h
> @@ -46,6 +46,22 @@
>          std::is_base_of<std::bidirectional_iterator_tag, IteratorCategoryT>::value,
>    };
>  
> +  /// A proxy object for computing a reference via indirecting a copy of an
> +  /// iterator. This is used in APIs which need to produce a reference via
> +  /// indirection but for which the iterator object might be a temporary. The
> +  /// proxy preserves the iterator internally and exposes the indirected
> +  /// reference via a conversion operator.
> +  class ReferenceProxy {
> +    friend iterator_facade_base;
> +
> +    DerivedT I;
> +
> +    ReferenceProxy(DerivedT I) : I(std::move(I)) {}
> +
> +  public:
> +    operator ReferenceT() { return *I; }
> +  };
> +
>  public:
>    DerivedT operator+(DifferenceTypeT n) const {
>      static_assert(
> @@ -120,10 +136,10 @@
>    PointerT operator->() const {
>      return &static_cast<const DerivedT *>(this)->operator*();
>    }
> -  ReferenceT operator[](DifferenceTypeT n) const {
> +  ReferenceProxy operator[](DifferenceTypeT n) const {
>      static_assert(IsRandomAccess,
>                    "Subscripting is only defined for random access iterators.");
> -    return *static_cast<const DerivedT *>(this)->operator+(n);
> +    return ReferenceProxy(static_cast<const DerivedT *>(this)->operator+(n));
>    }
>  };
>  
> Index: include/llvm/ADT/Sequence.h
> ===================================================================
> --- /dev/null
> +++ include/llvm/ADT/Sequence.h
> @@ -0,0 +1,87 @@
> +//===- Sequence.h - Utility for producing sequences of values ---*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +/// \file
> +/// This routine provides some synthesis utilities to produce sequences of
> +/// values. The names are intentionally kept very short as they tend to occur
> +/// in common and widely used contexts.
> +///
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef LLVM_ADT_SEQ_H
> +#define LLVM_ADT_SEQ_H
> +
> +#include "llvm/ADT/iterator.h"
> +#include "llvm/ADT/iterator_range.h"
> +
> +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> {
> +  typedef typename value_sequence_iterator::iterator_facade_base BaseT;
> +
> +  ValueT Value;
> +
> +public:
> +  typedef typename BaseT::difference_type difference_type;
> +  typedef typename BaseT::reference reference;
> +
> +  value_sequence_iterator() = default;
> +
> +  template <typename 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;
> +  }
> +
> +  // We have to explicitly provide ++ and -- rather than letting the facade
> +  // forward to += because the value might not support +=.
> +  using BaseT::operator++;
> +  value_sequence_iterator &operator++() {
> +    ++Value;
> +    return *this;
> +  }
> +  using BaseT::operator--;
> +  value_sequence_iterator &operator--() {
> +    --Value;
> +    return *this;
> +  }
> +
> +  bool operator==(const value_sequence_iterator &RHS) const { return Value == RHS.Value; }
> +  bool operator<(const value_sequence_iterator &RHS) const {
> +    return Value < RHS.Value;
> +  }
> +
> +  reference operator*() const { return Value; }
> +};
> +} // End detail namespace.
> +
> +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));
> +}
> +
> +}
> +
> +#endif
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits