[foreach][range] How would BOOST_REVERSE_FOREACH be implemented?
Boost.Foreach is such a great piece of kit that once you start using it you just can't stop. Indeed I like working directly with values rather than iterators so much that is feels normal for me to write something like BOOST_FOREACH( value_type x , make_iterator_range( rbegin( seq ) , rend( seq ) ) whenever I wish to reverse iterate over a sequence. My co-workers are not big fans of typing though, so they want a REVERSE_FOREACH. At first naïve glance it looks easy to implement in terms of the original FOREACH: template<typename C> inline iterator_range</* something */> make_reverse_iterator_range(C& c) { return make_iterator_range( rbegin(c) , rend(c) ); } #define REVERSE_FOREACH( x , container ) BOOST_FOREACH( x , make_reverse_iterator_range( container ) ) This works fine when container is an lvalue. But then co-workers, used to the wonders of the original FOREACH, inevitably start to use the new macro with rvalues and get a crash due to dangling references (- the iterator_range's members point to departed quantities.....) and if you could somehow work around that, there is the issue hidden inside the "/* something */" that you have to cope with fact that MSVC has no proper const rvalue detection. Finally the #define MACRO(x,y) ANOTHER_MACRO( f(x) , g(y) ) style is not recommended because they are brittle in the face of x and/or y themselves being macros. ... but all these issues are exactly the ones that BOOST_FOREACH itself solves! Thus surely it is technically possible to write a REVERSE_FOREACH that works with rvalues as well as lvalues, e.g. by essentially copying and pasting the FOREACH implementation and replacing "begin" and "end" by "rbegin" and "rend" in the vital places. However one feels there may be a better way... has anyone had a go at creating a "fully-functional" REVERSE_FOREACH and could offer me any advice? If indeed the task is fairly intricate and requires re-use of foreach internals, would the library author consider including something in the library itself? Many thanks for reading, Pete Bartlett
Pete Bartlett wrote:
#define REVERSE_FOREACH( x , container ) BOOST_FOREACH( x , make_reverse_iterator_range( container ) )
Something like this is what I was going to suggest, but ...
This works fine when container is an lvalue. But then co-workers, used to the wonders of the original FOREACH, inevitably start to use the new macro with rvalues and get a crash due to dangling references (– the iterator_range’s members point to departed quantities…..)
... ouch. If the container is an rvalue, it will be copied by FOREACH, but it the "container" is really just a proxy (eg., an iterator pair) to a temporary object, then copying the proxy doesn't help you, as you've discovered. <snip>
… but all these issues are exactly the ones that BOOST_FOREACH itself solves! Thus surely it is technically possible to write a REVERSE_FOREACH that works with rvalues as well as lvalues, e.g. by essentially copying and pasting the FOREACH implementation and replacing “begin” and “end” by “rbegin” and “rend” in the vital places.
Yes. However
one feels there may be a better way… has anyone had a go at creating a “fully-functional” REVERSE_FOREACH and could offer me any advice? If indeed the task is fairly intricate and requires re-use of foreach internals, would the library author consider including something in the library itself?
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance. -- Eric Niebler Boost Consulting www.boost-consulting.com
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance.
Thanks very much for your interest, Eric. I've set up a ticket on the issue which you can view at http://svn.boost.org/trac/boost/ticket/1071 Pete Bartlett
HI, yeah i wanted to do reverse iteration lookup so I took liberty to edit the file of foreach.hpp. Its ugly but it "works". I haven't done any detail testing. I only test it on basic string.. so i am sure of that. On 7/1/07, Pete Bartlett <pete@pcbartlett.com> wrote:
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance.
Thanks very much for your interest, Eric. I've set up a ticket on the issue which you can view at
http://svn.boost.org/trac/boost/ticket/1071
Pete Bartlett
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
Hmm I happen to use the idea of replacing begin with rbegin and iterators with reverse_iterator, etc Doing this I wrote a simple test file to compare performance. To my surprise doing std::reverse(v.begin(), v.end()) and than using BOOST_FOREACH is actually faster than a hard brute force method of BOOST_FOREACH_REV. Test ran with mingw compiler 3.4.5 Heres the code. #include <vector> #include <iostream> #include <algorithm> #include <boost/timer.hpp> #include <boost/foreach.hpp> #include <boost/format.hpp> int main() { const long TOTAL_ELEMENT = 10000; std::cout << boost::format("%f\t%f\n") % "BOOST_FOREACH" % "BOOST_FOREACH_REV"; boost::timer t; std::vector<int> vec_int; for (int j = 1; j < 50; ++j) { vec_int.reserve(TOTAL_ELEMENT*j); double t1, t2; for (int i = 0; i < TOTAL_ELEMENT*j; ++i) { vec_int.push_back(i); } t.restart(); std::reverse(vec_int.begin(), vec_int.end()); BOOST_FOREACH(int& i, vec_int) { i -= 1; } t1 = t.elapsed(); t.restart(); BOOST_FOREACH_REV(int& i, vec_int) { i -= 1; } t2 = t.elapsed(); std::cout << boost::format("%f\t%f\n") % t1 % t2; } } /* BOOST_FOREACH BOOST_FOREACH_REV 0.001000 0.001000 0.002000 0.003000 0.002000 0.005000 0.004000 0.005000 0.005000 0.006000 0.003000 0.009000 0.010000 0.008000 0.008000 0.010000 0.008000 0.010000 0.008000 0.014000 0.009000 0.016000 0.016000 0.014000 0.010000 0.019000 0.011000 0.017000 0.013000 0.017000 0.013000 0.018000 0.014000 0.022000 0.016000 0.020000 0.016000 0.021000 0.017000 0.023000 0.017000 0.025000 0.018000 0.031000 0.018000 0.026000 0.020000 0.034000 0.019000 0.030000 0.022000 0.037000 0.021000 0.033000 0.024000 0.032000 0.023000 0.039000 0.025000 0.033000 0.026000 0.039000 0.026000 0.046000 0.029000 0.040000 0.028000 0.039000 0.030000 0.042000 0.030000 0.051000 0.032000 0.051000 0.035000 0.059000 0.035000 0.054000 0.033000 0.055000 0.035000 0.059000 0.035000 0.119000 0.035000 0.062000 0.037000 0.059000 0.037000 0.057000 0.037000 0.063000 0.072000 0.061000 0.039000 0.062000 0.039000 0.060000 */ On 8/6/07, chun ping wang <cablepuff@gmail.com> wrote:
HI, yeah i wanted to do reverse iteration lookup so I took liberty to edit the file of foreach.hpp. Its ugly but it "works". I haven't done any detail testing.
I only test it on basic string.. so i am sure of that.
On 7/1/07, Pete Bartlett <pete@pcbartlett.com> wrote:
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance.
Thanks very much for your interest, Eric. I've set up a ticket on the issue which you can view at
http://svn.boost.org/trac/boost/ticket/1071
Pete Bartlett
_______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
Pete Bartlett wrote:
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance.
Thanks very much for your interest, Eric. I've set up a ticket on the issue which you can view at
Better late than never -- I just committed a BOOST_REVERSE_FOREACH to trunk. -- Eric Niebler Boost Consulting www.boost-consulting.com
Quoting Eric Niebler <eric@boost-consulting.com>:
Pete Bartlett wrote:
You're not the first to want a REVERSE_FOREACH, but this is the first legitimate argument I've heard for it. Could you go to http://svn.boost.org/trac and open a new ticket there. Make the ticket type "feature request" and the component "foreach". I'll look into it when I have a chance.
Thanks very much for your interest, Eric. I've set up a ticket on the issue which you can view at
Better late than never -- I just committed a BOOST_REVERSE_FOREACH to trunk.
[Apologies if a similar message has already posted] Cool! Your work is much appreciated. For my particular use I am layering the for_each header over the top of Boost 1.33.1 (I know, I know, I'm way behind the times). It may be of use for some users to know the latest SVN version, despite undergoing several revisions since the old review version that was deliberately intended to work with 1.33.1, nearly works out-the-box with that old release. I've not tested the more sophisticated features, but to get basic functionality you just need to change "range_mutable_iterator" to "range_iterator" on line 316. Pete
participants (4)
-
chun ping wang -
Eric Niebler -
Pete Bartlett -
Peter Bartlett