Re: [boost] Is Boost.Range broken?

Subject: Re: [boost] Is Boost.Range broken?
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2008-11-22 09:13:07

On Sat, Nov 22, 2008 at 4:25 PM, Mathias Gaunard <
mathias.gaunard_at_[hidden]> wrote:

> Tomas Puverle wrote:
>
>  Here are the reasons why I think Boost.Range is broken:
>>
>> 1) Empty ranges are useless and they cannot even be reliably tested for
>> emptiness.  The empty() function now asserts and the is_singular()
>> function
>> behaves differently between debug and release builds, making it impossible
>> to
>> detect if a range is, in fact, empty.  In addition, this is not documented
>> well
>> and can lead to subtle bugs and undefined behaviour, which will only
>> manifest
>> itself in release builds.
>> 2) The behaviour is unintuitive.  Range is a generalisation of the
>> interface of
>> the std::containers.  With this change, containers and ranges can no
>> longer be
>> used in the same code path.
>> 3) Reintroducing the "singular" member into release builds to make the
>> is_singular() function work correctly will defeat the purpose of the size
>> optimisation, while still not achieving interface compatibility with
>> std::containers.
>> 4) Having the additional if() condition in size() and empty()is unlikely
>> to be a
>> large burden on most programs.  I would expect most programs will spend
>> more
>> time iterating data than testing ranges for emptiness.
>> 5) The change could be reverted without affecting users.  For those who
>> are
>> relying on the new behaviour, the change to empty() and size() should be
>> immaterial, as they cannot be calling them now on singular ranges anyway.
>>
>
> This is a bad explanation which can prove misleading.
>
> The reason why you think Boost.Range is broken is because iterator_range
> (which is nothing more than a fancy std::pair, I never found the use of it
> myself) used to allow operations such as "empty" on an uninitialized range
> but doesn't anymore.
>

With such a notion this, indeed, is a rather useless component. I have to
agree with Tomas and others in one of their points: one of the major
advantages or ranges before pairs of iterators is that they attempt to
reproduce containers as far as possible. Yes, in general default-constructed
iterators are singular and they cannot be used reliably. However, not all
iterators behave that way. Pointers, for example, are NULL on
default-initialization (which is used in the default constructor of the
iterator_range class). I can well have my own iterators that are
default-constructible and allow comparison in this state. And I don't
understand why an iterator range of these iterators suddenly restricts me
from being able to compare these iterators in empty(), for example.

The documentation of the iterator_range states for several functions, such
as size() and operator[], that these functions depend on the iterator
nature. I believe, empty() could just do the same - rely on the ability to
compare iterators, even default-initialized ones. Of course, this should be
clearly stated in the docs.

If it was just me, default-constructing iterators and ranges shouldn't even
> be allowed (unless that is sufficient that initialize it).
> A range that is not properly initialized is not in any usable state. It's
> not an empty range, it's more like a pointer to somewhere random. Just don't
> try to use it. If Boost.Range used to allow it, that was a bug; good thing
> it is now fixed.
>

> As for is_singular, that function shouldn't be exposed to the public.

I agree that is_singular is evil. More over, I am sure that asserts based on
it are also evil. My point is: don't try to make iterator_range more clever
than it is stated in the docs (that is, a pair of iterators with a
forwarding interface of a container).