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 <boost/range/adaptor/reversed.hpp> #include <boost/foreach.hpp> #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<typename T, typename R> 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)) {...}