[PATCH] D25124: Support rvalue references in enumerate range adapter.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 4 09:21:46 PDT 2016
zturner added inline comments.
> chandlerc wrote in STLExtras.h:35-43
> 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...
As mentioned, `make_filter_range` also uses `IterOfRange`. In fact, there was a bug in it before, so putting it here both fixes an existing bug in `make_filter_range` and hopefully ensures that nobody will make this same mistake again. I only put `ValueOfRange` here as well since `IterOfRange` was already here.
In response to your second point, I tried it out and it doesn't quite work. In order for this to work I will need to typedef `reference` inside of `enumerator_impl::iterator`. That might seem obvious, but a cursory glance at the standard does not seem to require that range-for-iterators support `iterator_traits`. So it would be nice to be able to use `enumerate` (and any other range-adaptors we might invent) with iterators that don't have full support for `iterator_traits`.
https://reviews.llvm.org/D25124
More information about the llvm-commits
mailing list