Re: [boost] [range][stride] broken?

Subject: Re: [boost] [range][stride] broken?
From: Michel MORIN (mimomorin_at_[hidden])
Date: 2010-12-25 23:45:37

Hi Neil,
I'd like to make some comments on the updated strided_range.

[Comments on implementation]
increment() can generate overflowed iterator:
    // boost/range/adaptor/strided.hpp
    void increment()
    {
        m_offset += m_stride;
        if (m_offset <= m_max_offset)
            std::advance(this->base_reference(), m_stride);
    }
Do you mean "if (m_offset < m_max_offset)"?

There might be a bug in decrement(). Let rng be a strided_range. Then,
"boost::prior(rng.end())" and "boost::next(rng.begin(), boost::distance(rng)-1)"
do not point to the same element.

Neil Groves wrote:
> I have committed a new
> fix to the trunk and added your test cases
The test case is credited to Maxim!

[Comments on design]
Probably, you have introduced new concepts something like "Sizeable" range.
The updated implementation of strided_range requires that an input range
should be "Sizeable". But, IMHO, this is an over-requirement.
 - For Single Pass and Forward Ranges, input ranges don't need to be "Sizeable".
   strided_range can be implemented without requiring "Sizeable" ranges.

 - For Bidirectional Range, there is a trade-off:
   requiring "Sizeable" ranges allows us to implement decrement() that can
   be safely applied to end(). But it seems more convenient for users that
   "Non-Sizeable" ranges can be used for strided_range. Do you have
   any rationale for adopting the design that requires "Sizeable" ranges?

Regards,
Michel