[PATCH] D25124: Support rvalue references in enumerate range adapter.

[Zachary Turner via llvm-commits]

IterOfRange is used by another class in the file which is why i fixed it up
there and added ValueOfRange alongside
On Tue, Oct 4, 2016 at 12:42 AM Chandler Carruth <chandlerc at gmail.com>
wrote:

> chandlerc added a comment.
>
> This entire system really needs a bunch of work to fully interoperate well
> with iterators. I don't want to sign you up for all of it so I'm trying to
> suggest minimal changes below. Hopefully they don't cause too many
> problems. Not sure where to draw the line here without yak shaving this
> into the next century. I'll probably sign myself up to fix some of this now
> that I'm neck deep.... Anyways, lemme know if the below changes work.
>
>
>
> > STLExtras.h:35-43
> >  namespace detail {
> >
> >  template <typename RangeT>
> > -using IterOfRange = decltype(std::begin(std::declval<RangeT>()));
> > +using IterOfRange = decltype(std::begin(std::declval<RangeT &>()));
> > +
> > +template <typename RangeT>
> > +using ValueOfRange = decltype(*std::declval<IterOfRange<RangeT>>());
>
> Put these as members of `enumerate_impl` so that they can directly use
> `R`? I would alse name them `IteratorT` and `ValueT`.
>
> But I don't know that you want to define `ValueT` this way. Instead, I
> would use `std::iterator_traits<IteratorT>::value_type` and
> `std::iterator_traits<IteratorT>::reference` which seem like the more
> formal way to extract this information.
>
> However, this may in turn show that you can't actually nest these yet. But
> if you want to fix that, I think it will need a much more invasive change.
> I wouldn't worry with that unless you really want to...
>
> > STLExtras.h:643
> >    struct iterator {
> > -    iterator(I Iter, std::size_t Index) : Iter(Iter), Index(Index) {}
> > +    iterator(IterOfRange<R> &&Iter, std::size_t Index)
> > +        : Iter(std::move(Iter)), Index(Index) {}
>
> Just accept this by-value, no need for r-value reference here.
>
> https://reviews.llvm.org/D25124
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161004/22aca929/attachment.html>