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

[Zachary Turner via llvm-commits]

zturner added inline comments.


> chandlerc wrote in STLExtras.h:642-646
> I don't really understand this comment or the remove_const below...
> 
> For example, does the remove_const cause this to allow mutation even when the range itself doesn't? That doesn't seem good...
> 
> (also, 80 columns)

The `remove_const` snuck in which I was tinkering around with things.  I need to remove that.  Thanks for pointing it out.

Correct me if I'm wrong here, but at least the conclusion I came to when trying to get this working was this:

1. `std::declval<R>()` returns an `R&&`
2. `std::begin(R&&)` will select the overload of `std::begin()` that accepts a `const Container&`, because the result of `std::declval<R>()` is an rvalue, so it can't bind to the overload of `std::begin()` that accepts a `Container&`.
3. As a result, `std::begin()` returns a `const_iterator`.
4. This means `I` is typedefed to `std::vector<T>::const_iterator`, which will obviously not work for mutation.  (`vector` just an example since that's what i used in the tests, replace with arbitrary `R`).

By writing `std::declval<R&>()` you're forcing it to return an rvalue, so we get a non-const iterator (unless of course `R` is itself a const type)

https://reviews.llvm.org/D25124