[boost] Re: Formal Review: FOREACH macro

From: Gennadiy Rozental (gennadiy.rozental_at_[hidden])
Date: 2005-05-02 11:56:27

"Eric Niebler" <eric_at_[hidden]> wrote in message 
news:427652AD.1020900_at_boost-consulting.com...
> Gennadiy Rozental wrote:
>> "Eric Niebler" <eric_at_[hidden]> wrote in message 
>> news:4276407E.6030205_at_boost-consulting.com...
>>> The type of the container is complicated, but you don't have to mention 
>>> it anywhere because FOREACH correctly handles rvalues.
>>>
>>>This is very powerful.
>>
>>
>> And dangerous. Cause  allows developer to "forget" about an extra copy. I 
>> guess I could live with it. I wrap it into my own header and disable 
>> rvalues by definining appropriate macros
>>
>
>
> Code that returns STL containers by value is already inefficient. Why 
> prevent FOREACH from working with such code? I don't understand.

 That's good point.

> 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.

> 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?

> 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). std::string should work out pf the box 
(is it?). Any other string (like CString) I wouldn't bother. 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.

>>>What do you mean by and optional dependency on Boost.Range?

>> #ifndef  BOOST_FOREACH_NO_EXTENTIONS
>
> This doesn't even begin to answer my question, but maybe I should have 
> been more specific. 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

> And what sort of advice do I give people who want to extend FOREACH if the 
> extensibility points are also optional?

Depends on what you choose as default. If you do not define 
BOOST_FOREACH_NO_EXTENTIONS by default then nothing changes for those 
people. You just add a note that if you do not need support for 
extencibility one need to define BOOST_FOREACH_NO_EXTENTIONS. Or vice versa 
you define BOOST_FOREACH_NO_EXTENTIONS by default then those who need to 
extend it would need to define BOOST_FOREACH_EXTENTIONS and in you header:

#ifndef BOOST_FOREACH_EXTENTIONS
#define BOOST_FOREACH_NO_EXTENTIONS
#endif

Gennadiy