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

[Chandler Carruth via llvm-commits]

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