Re: [boost] [Range] patched: boost/range/iterator.hpp

From: David Abrahams (dave_at_[hidden])
Date: 2006-05-08 13:05:24

"Eric Niebler" <eric_at_[hidden]> writes:

> David Abrahams wrote:
>> Hi Thorsten,
>> 
>> I took the liberty of checking this patch into the HEAD:
>> 
>> 
>> 
>> ------------------------------------------------------------------------
>> 
>> Index: iterator.hpp
>> ===================================================================
>> RCS file: /cvsroot/boost/boost/boost/range/iterator.hpp,v
>> retrieving revision 1.4
>> retrieving revision 1.5
>> diff -b -d -u -b -u -r1.4 -r1.5
>> --- iterator.hpp	5 Jan 2005 18:19:31 -0000	1.4
>> +++ iterator.hpp	8 May 2006 10:21:50 -0000	1.5
>> @@ -37,6 +37,12 @@
>>          typedef BOOST_DEDUCED_TYPENAME C::iterator type;
>>      };
>>      
>> +    template< typename C >
>> +    struct range_iterator<C const>
>> +    {
>> +        typedef BOOST_DEDUCED_TYPENAME C::const_iterator type;
>> +    };
>> +    
>>      //////////////////////////////////////////////////////////////////////////
>>      // pair
>>      //////////////////////////////////////////////////////////////////////////
>> 
>> 
>> ------------------------------------------------------------------------
>> 
>> 
>> Please let me know if you have any objections.
>
>
>
> I object. In the current range interface, the meaning of the range 
> metafunctions are as follows:
>
> range_iterator: always return ::iterator
> range_const_iterator: always return ::const_iterator
> range_result_iterator: return either ::iterator or ::const_iterator 
> depending on the const-ness of the argument.

Ugh, I forgot about the existence of range_result_iterator.

> This was deemed an unfortunate design, and there is a reworking of
> the Range interface in the pipeline. Thorsten wanted to apply it for
> 1.34, but it was too late in the process.
>
> Your patch gets everything all mixed up. 

That's rather a bald and sweeping statement to make without any
explanation.  What do you really mean by "everything" in this case?

AFAICT it's entirely consistent.  Look:

  typedef std::vector<int> vec;
  vec v;
  vec const& vc;

  boost::begin(v)        -> vec::iterator
                         -> range_iterator<typeof(v)>::type   

  boost::begin(vc)       -> vec::const_iterator
                         -> range_iterator<typeof(v)>::type

  boost::const_begin(v)  -> vec::const_iterator
                         -> range_const_iterator<typeof(v)>::type   

  boost::const_begin(vc) -> vec::const_iterator
                         -> range_const_iterator<typeof(v)>::type   

That is, the result type of begin is always expressed by
range_iterator and the result type of const_begin is always expressed
by range_const_iterator.  Perfectly symmetric and consistent.

Before my change, the best you could say was that the result type of
begin is always expressed by range_result_iterator and the result type
of const_begin is always expressed by range_const_iterator.

> My suggestion is that you back 
> this change out and Thorsten applies his changes to HEAD.

Why?  What other changes do you think are needed in order to make this
consistent?

Anyway, now that you've reminded me of range_result_iterator, I'll be
happy to roll back my change if this is breaking any code or if
Thorsten asks me to.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com