Boost range changes [1.37.0]
I just upgraded from Boost 1.33.1 to 1.37.0 and have found a breaking change in Boost.Range. Previously, iterator_range::empty() would do the following: bool empty() const { if( singular ) return true; return m_Begin == m_End; } and similarly for size(). In the new release, instead of the test, there is an assertion, which is now breaking my code. Why (and when?) was this change introduced? This make Boost Range much in generic code, such as template<class Range> void foo(const Range & r_) { if (r_.empty()) {...} } The code is frequently called with containers and ranges and works beautifully. However, a common pattern is to call the function with a default constructed (empty range). This now crashes. The only way to test for validity AFAICS is to call is_singular(), unfortunately, this breaks when the function is called with containers. This, IMHO, was an unfortunate decision. Any suggestions anyone? Thanks, Tom
Tomas Puverle skrev:
I just upgraded from Boost 1.33.1 to 1.37.0 and have found a breaking change in Boost.Range.
Previously, iterator_range::empty() would do the following:
bool empty() const { if( singular ) return true;
return m_Begin == m_End; }
and similarly for size(). In the new release, instead of the test, there is an assertion, which is now breaking my code.
Why (and when?) was this change introduced? This make Boost Range much in generic code, such as
template<class Range> void foo(const Range & r_) { if (r_.empty()) {...} }
The code is frequently called with containers and ranges and works beautifully. However, a common pattern is to call the function with a default constructed (empty range). This now crashes. The only way to test for validity AFAICS is to call is_singular(), unfortunately, this breaks when the function is called with containers.
This, IMHO, was an unfortunate decision. Any suggestions anyone?
The change has happened long ago. It was a design mistake to try to make a range of singular iterators valid since it adds overhead also for those that don't need it. Your use-case is new to me, but seems quite ok. But making boost::iterator_range as a mixture of a range and boost.optional seems like a bad idea. template<class Range> void foo(boost::optional<const Range &> r_) { if (r && !r_->empty()) {...} } might be an idea? -Thorsten
Thorsten, Thanks for the reply. See comments inline.
The change has happened long ago. It was a design mistake to try to make a range of singular iterators valid since it adds overhead also for those that don't need it.
When you say "overhead", are you speaking of size or speed? Was this overhead demonstrated by some particular use case? Does this mean you may be also removing the "singular" flag at some point? Is there a supported way of constructing valid empty ranges? I can't see that there's a way to do it.
Your use-case is new to me, but seems quite ok. But making boost::iterator_range as a mixture of a range and boost.optional seems like a bad idea.
I feel that by the same token, you could say that a default constructed std::vector<T> is a bad idea. I really like using Boost.Range but I think this new behaviour is a big shortcoming. I also never saw any mention of these breaking changes on boost.devel...
template<class Range> void foo(boost::optional<const Range &> r_) { if (r && !r_->empty()) {...} }
might be an idea?
IMHO, this is obfuscated. There is a difference between an empty range and a range which is optional. In line with the std containers, I would argue that a default constructed range should be empty. It also quickly becomes cumbersome if your functions have multiple input ranges, like mine. Another problem I can see is overloading - you are forcing a user defined conversion, which is not going to be a better match than foo(const Range &); Also, if I have a typedef boost::range<Iter> MyRange, from a library users' perspective, calling foo(MyRange()) seems more intuitive than foo(boost::optional<const MyRange &>()); Anyway, unless I have any chance of making you change your mind, there doesn't seem much point in continuing this conversation. In any case, I've enjoyed using your library. I found it so useful I've built four other libraries, which are mostly based on passing data around through Boost.Ranges. I just wish that all of my code wasn't broken now that I've upgraded. Tom
Tomas Puverle skrev:
Thorsten,
Thanks for the reply. See comments inline.
The change has happened long ago. It was a design mistake to try to make a range of singular iterators valid since it adds overhead also for those that don't need it.
When you say "overhead", are you speaking of size or speed? Was this overhead demonstrated by some particular use case? Does this mean you may be also removing the "singular" flag at some point?
Its only there in debug builds.
Is there a supported way of constructing valid empty ranges? I can't see that there's a way to do it.
Your use-case is new to me, but seems quite ok. But making boost::iterator_range as a mixture of a range and boost.optional seems like a bad idea.
I feel that by the same token, you could say that a default constructed std::vector<T> is a bad idea. I really like using Boost.Range but I think this new behaviour is a big shortcoming. I also never saw any mention of these breaking changes on boost.devel...
template<class Range> void foo(boost::optional<const Range &> r_) { if (r && !r_->empty()) {...} }
might be an idea?
IMHO, this is obfuscated. There is a difference between an empty range and a range which is optional.
An there is a difference betweeen an empty range and a singular range. You can always add the extra state...just derive from iterator-range an implement it. If boost::iterator_range did it, there would be no way of getting rid of the overhead. -Thorsten
<snip>
IMHO, this is obfuscated. There is a difference between an empty range and a range which is optional.
An there is a difference betweeen an empty range and a singular range.
If there is a genuine difference between an empty range and a singular range, why is the functionality so different between debug and release? Dave
Dave Handley skrev:
<snip>
IMHO, this is obfuscated. There is a difference between an empty range and a range which is optional.
An there is a difference betweeen an empty range and a singular range.
If there is a genuine difference between an empty range and a singular range, why is the functionality so different between debug and release?
In debug mdoe we detect misused of singular ranges. -Thorsten
Dave Handley skrev:
<snip>
IMHO, this is obfuscated. There is a difference between an empty range and a range which is optional.
An there is a difference betweeen an empty range and a singular range.
If there is a genuine difference between an empty range and a singular range, why is the functionality so different between debug and release?
In debug mdoe we detect misused of singular ranges.
Unfortunately, different functionality between debug and release is almost always a bad thing. You are no longer testing the same code in both cases, and in this case the library will silently fail in release which would be a disaster in production. I'm going to have to side with both Tom and Rob on this thread and say that this is a severe retrograde step. From the perspective of a generic programmer, this change turns boost.range from being a really useful library, into it being close to unusable. Please reconsider the change. I just can't agree with any of the reasons for doing it - testing a bool in the empty or size functions will have no noticeable difference to performance in any real world application. Dave
On Thu, Nov 20, 2008 at 10:20, Robert Jones <robertgbjones@gmail.com> wrote:
On Thu, Nov 20, 2008 at 9:26 AM, Thorsten Ottosen <thorsten.ottosen@dezide.com> wrote:
An there is a difference between an empty range and a singular range.
Sorry to lower the tone, but can someone please explain exactly what a singular range is?
Presumably a range where at least one of the ends is a singular iterator, so roughly a range object that doesn't have an associated sequence of values. See 24.1/5 for a precise definition. (Draft available here, if needed: http://www.csci.csusb.edu/dick/c++std/cd2/lib-iterators.html)
On Wed, Nov 19, 2008 at 10:15 PM, Thorsten Ottosen < thorsten.ottosen@dezide.com> wrote:
Tomas Puverle skrev:
I just upgraded from Boost 1.33.1 to 1.37.0 and have found a breaking change in Boost.Range.
Previously, iterator_range::empty() would do the following:
bool empty() const { if( singular ) return true; return m_Begin == m_End; }
and similarly for size(). In the new release, instead of the test, there is an assertion, which is now breaking my code.
Why (and when?) was this change introduced? This make Boost Range much in generic code, such as
template<class Range> void foo(const Range & r_) { if (r_.empty()) {...} }
The code is frequently called with containers and ranges and works beautifully. However, a common pattern is to call the function with a default constructed (empty range). This now crashes. The only way to test for validity AFAICS is to call is_singular(), unfortunately, this breaks when the function is called with containers. This, IMHO, was an unfortunate decision. Any suggestions anyone?
The change has happened long ago. It was a design mistake to try to make a range of singular iterators valid since it adds overhead also for those that don't need it.
Your use-case is new to me, but seems quite ok. But making boost::iterator_range as a mixture of a range and boost.optional seems like a bad idea.
template<class Range> void foo(boost::optional<const Range &> r_) { if (r && !r_->empty()) {...} }
might be an idea?
I too feel this change is a retrograde step. While I can see that there is some merit in not making a singular range valid if we are starting from square-one, as a change to an existing library I think this unacceptable. Having a default constructed iterator range exhibit different behaviour from, say, a vector, which is also a range seems wrong. - Rob.
AMDG Tomas Puverle wrote:
Why (and when?) was this change introduced? This make Boost Range much in generic code, such as
template<class Range> void foo(const Range & r_) { if (r_.empty()) {...} }
template<class T> bool is_singular_or_empty(const T& t) { return(t.empty()); } template<class T> bool is_singular_or_empty(const iterator_range<T>& t) { return(t.is_singular() || t._empty()); } ? In Christ, Steven Watanabe
participants (6)
-
Dave Handley -
Robert Jones -
Scott McMurray -
Steven Watanabe -
Thorsten Ottosen -
Tomas Puverle