
Hi, Let me step out of review manager role for a moment and give actual review. As many of you here I fell in love with this utility macro the moment I heard about it. Many would agree that lack of expressiveness in lambda expression generation is a major drawback in current STL design. Boost.Lambda does address part of the issue. But I still believe BOOST_FOREACH is more portable, powerfull and easier to use. Regrdless to what I am going to say later I believe this macro would be a valueable addition and should be accepted. Here is a couple remarks I wanted to make. First a general one: I believe FOREACH is so widely usefull it should work for all compilers we do our resression tests (with different levels of compartibility). Actually as some of you may noticed I implemented version of FOREACH internally in Boost.Test namely because I need portable version, while I liked to use FOREACH. Once (if) BOOST_FOREACH implementation is made portable for my purposes I would be willing to get rid of my version. As it stands now here are results for resress.cpp test (compilers I could test on): borland 5.5.1 - fail borland 5.6.4 - fail cw-8.3 - fail gcc 3.3.3 - pass Intel 8.1 - pass gcc 2.95 - fail vc 6.5 - fail vc 6.5 (stlport) - fail vc 7.1 - pass vc 7.1 (stlport) - pass sunpro 5.3 - fail Now for some specific issues: 1. Rvalues support I agree it's cool and "magical" - the way you added support for rvalues. But I believe it's actually misplaced efforts. IMO support for rvalues brings more harm than advantage. I could accidently or mindlessly (using some global replacement in sources, for example) introduce rvalue in BOOST_FOREACH statement and compiler wouldn't warn me. Now I am paying for an unnessesary copying. while exist perfectly good alternative that doesn't require it: my_collection_type my_function(); ... my_collection_type const& v = my_function() BOOST_FOREACH(... , v ) { } In addition eliminating rvalues support will signicantly simplify implementation and as a result speed up compilation. 2. Usage of Boost.Range I understand why FOREACH is using Boost.Range, but: a) In many (most) cases I (and any other developer now) working with stl containers and wouldn't need any extra help from boost.range for FOREACH to work properly. b) Usage of Boost.Range introduces an extra dependency on ... Boost.Range and everything it depends on in its' turn (including all portability limitations). I for one couldn't afford this dependency, c) Usage of Boost.Range adding a level of indiration and slightly complicate am implementation I am not going to propose to eliminate this dependency completely. But I think we need to make it optional. Users shouldn't pay for what they do not need (in most/many cases). Also I believe some old compilers could only made to work in such mode. So you may introduce another level of compartibility - working only with stl compartible container-lvalue. As to what should be the default - it's your call. 3. regress.cpp I do not see test program testing all the cases (rvalue canst collections etc). You may want to create several test files for each level of compartibility 4. auto_any auto_any staff deserves at least special mentionning in docs (if not separate header). If should explain the tecnique and how it could be used 5. type2type I believe we already have something similar in boost. 6. typeof/encode_type I believe this tecnique desirves separat eheader and special explantion in docs. 7. First elem support FOREACH could easily add support for the first element detection. I think it may be wrty addition. In any case thanks for making it available for us. Regards, Gennadiy