[boost] Re: Formal Review: FOREACH macro

From: Eric Niebler (eric_at_[hidden])
Date: 2005-05-02 12:19:54

Gennadiy Rozental wrote:
> "Eric Niebler" <eric_at_[hidden]> wrote in message 
> news:427652AD.1020900_at_boost-consulting.com...
> 
> 
>>Besides, eliminating the rvalue requirement won't simplify the code 
>>because much of that machinery is needed anyway to prevent the macro args 
>>from being reevaluated. The rvalue stuff practically falls out of that.
> 
> 
> Lets see: to support rvalues you need:
> 
> # include <new>
> # include <boost/aligned_storage.hpp>
> # include <boost/utility/enable_if.hpp>
> # include <boost/type_traits/is_array.hpp>
> struct rvalue_probe
> template<typename T>
> struct simple_variant
> cheap_copy
> 
> Quite a lot actually.
> 

True, but much of this is only needed for the *const* rvalue stuff. If 
there were general agreement that there's fat to trim (is there?), 
dropping *const* rvalue support would be an easy target. Plain rvalue 
support is just too useful to drop, IMO.

Anyway, you're the first person to complain about this dependency. Are 
your compile times noticeably faster when you define the no-rvalue 
macros? If you could post compile times with and without, that would be 
interesting.

> 
>>I hope that you aren't reintroducing reevaluation bugs by "defining 
>>appropriate macros".
> 
> 
> I would define
> BOOST_FOREACH_NO_RVALUE_DETECTION
> BOOST_FOREACH_NO_CONST_RVALUE_DETECTION
> does it reintroduce reevaluation bugs?
> 

No, that's fine.

> 
>>The complexity is there for a reason. Being able to iterate over arrays, 
>>strings, containers, iterator ranges is a design goal. So is providing an 
>>extensibility mechanism.
> 
> 
> I see it as least important goal. IMO any container that isn't stl 
> compartible is not worth paying attention anyway. Raw arrays I do not use 
> (and we shouldn't encourage users).

They are quite useful in some domains.

> std::string should work out pf the box 
> (is it?).

Yes, but I was refering to native (C-style) strings, as in:
BOOST_FOREACH(char ch, "hello world")

> Any other string (like CString) I wouldn't bother.

The fact that BOOST_FOREACH is extensible enough to handle CString is a 
strong argument in its favor, IMO.

  And as for
> iterator ranges I would use boost::iterator_range instead of std::pair. So 
> making it optional is worth trying I believe. On the way you could 
> eliminate:
> #include <boost/range/end.hpp>
> #include <boost/range/begin.hpp>
> #include <boost/range/result_iterator.hpp>
> 
> and everything it depends on.

I still don't see why this is such a concern.

>>How would I implement the code to make the dependency 
>>on Boost.Range optional?
> 
> 
> The same way you deal with other optional functionality:
> 
> #ifndef  BOOST_FOREACH_NO_EXTENTIONS
> template<typename T, typename C>
> inline auto_any<BOOST_DEDUCED_TYPENAME foreach_iterator<T, C>::type>
> begin(auto_any_t col, type2type<T, C> *, void *, boost::mpl::true_ *)
> {
>     return auto_any_cast<T, C>(col).begin();
> }
> #else
> template<typename T, typename C>
> inline auto_any<BOOST_DEDUCED_TYPENAME foreach_iterator<T, C>::type>
> begin(auto_any_t col, type2type<T, C> *, void *, boost::mpl::true_ *)
> {
>     return foreach_detail_::adl_begin(auto_any_cast<T, C>(col));
> }
> #endif
> 

This would effectively double the amount of code in foreach.hpp, and 
double the test matrix. I would need to see some hard numbers about 
compile times before I do that.

-- 
Eric Niebler
Boost Consulting
www.boost-consulting.com