On 26/11/2014 05:11, Filip Konvička wrote:
I was trying to replace BOOST_REVERSE_FOREACH with the standard
range-based for loop.
#include
#include
#include <vector>
#include <iostream>
std::vector<int> getv() {
return std::vector<int>{1,2,3};
}
int main(int argc, char* argv[]) {
// works fine:
BOOST_REVERSE_FOREACH(int i, getv())
std::cout << i << std::endl;
// crashes:
for(int i : boost::adaptors::reverse(getv()))
std::cout << i << std::endl;
return 0;
}
Maybe this is basic C++ I should understand; it looks like the temporary
vector returned by getv() gets destroyed in the 2nd loop too soon. I
was wondering whether the adaptor could warn me that I'm using it in a
wrong way (the code looks so nice and compiles cleanly...).
This works (albeit possibly with a performance penalty in the form of a
double copy; I'm not sure if the compiler can be smart enough to elide
that):
template
std::vector<T> as_vector(const R& range)
{
return std::vector<T>(boost::begin(range), boost::end(range));
}
...
for (int i : as_vector<int>(boost::adapters::reverse(getv())))
The original code is unsafe because the compiler has no way to know that
the temporary vector returned by getv() needs to live on, as normally
that is not the case with function arguments. (And the range adapters
do not make a copy of their arguments, for performance reasons, so they
require a longer lifetime.)
But at least in theory the range-based-for itself has to preserve the
"final" temporary range for the duration of the loop, as otherwise it is
simply not safe to ever use an rvalue as the range expression. (I'm not
sure if this is mandated by the standard, but it seemed to work in
VS2013 at least.) And a collection is itself a valid range, so ensuring
that you iterate over a "real" collection instead of just an adapter works.
It seems likely that a method like as_vector already exists somewhere in
Boost, although I can't put my finger on it at the moment. There's also
probably a way to make it deduce T from R so it doesn't have to be
explicitly specified, but I'm not really familiar enough with
Boost.Range to write that. (In general I find the Boost.Range
documentation fairly incomprehensible.)
It might be easier just to rewrite it like this though, which will
satisfy the lifetime requirement while avoiding the possible performance
penalty:
auto v = getv();
for (int i : boost::adapters::reverse(v)) {...}
