Boost C++ Libraries: Ticket #973: zip_iterator has value_type == reference

description changed; owner set

Dave Abrahams — Mon, 21 May 2007 04:28:55 GMT

owner set to Thomas Witt
description modified (diff)

dtrebbien@… — Sun, 28 Nov 2010 00:14:35 GMT

#1860 is a duplicate of this ticket. Please see that ticket for more information.

blackhc@… — Wed, 26 Sep 2012 12:47:33 GMT

Is anything ever going to happen on this issue? I have the exact same problem as described in #1860 and a solution would be really helpful...

anonymous — Mon, 12 Nov 2012 21:20:58 GMT

Dave asked for a testcase. Here's one:

#include <boost/iterator/zip_iterator.hpp>
#include <boost/iterator/iterator_traits.hpp>
#include <boost/assert.hpp>
template<typename It> void my_iter_swap(It i1, It i2) {
  typename boost::iterator_value<It>::type tmp = *i1;
  *i1 = *i2;
  *i2 = tmp;
}
int main() {
  int i[2] = { 0, 1 };
  my_iter_swap(boost::make_zip_iterator(boost::make_tuple(i+0)), boost::make_zip_iterator(boost::make_tuple(i+1)));
  BOOST_ASSERT(i[0] == 1 && i[1] == 0);
}

And here's a patch against boost 1.52.0 that fixes it. I hereby place it in the public domain.

351a352,362
>     // Metafunction to obtain the type of the tuple whose element types
>     // are the value types of an iterator tuple.
>     template<typename IteratorTuple>
>     struct tuple_of_value_types
>       : tuple_impl_specific::tuple_meta_transform<
>             IteratorTuple,
>             iterator_value<mpl::_1>
>           >
>     {
>     };
>
418c429,430
<         typedef reference value_type;
---
>         typedef typename
>         detail::tuple_of_value_types<IteratorTuple>::type value_type;

Now, can we get this fixed? Pretty please?

cc set

Mon, 12 Nov 2012 21:31:12 GMT

cc matthias_berndt@… added

Wed, 21 Nov 2012 07:02:25 GMT

OK, now what the hell is going on here?

this is an obvious and serious bug
the bug report is six years old
an obvious and trivial fix has been available for five years

Just what is supposed to happen until anybody cares to commit the fix?

owner changed

Dave Abrahams — Wed, 21 Nov 2012 22:26:37 GMT

owner changed from Thomas Witt to jeffrey.hellrung

Dave Abrahams — Wed, 21 Nov 2012 23:39:10 GMT

The current library maintainer wasn't assigned here so he hasn't seen your bump. I just fixed that.

Sat, 01 Dec 2012 16:37:07 GMT

Replying to dave:

The current library maintainer wasn't assigned here so he hasn't seen your bump. I just fixed that.

Uh, did you? In my world, a maintainer is somebody who maintains. Not committing a ready-made fix for a trivial, six-year-old bug is not what I'd call maintaining.

Let me ask again: what is supposed to happen until anybody cares to commit this trivial patch?

Steven Watanabe — Sat, 01 Dec 2012 17:27:03 GMT

A patch is not trivial just because it only changes a small amount of code. This is a significant change in the behavior of zip_iterator, and it can't just be applied blindly. At the very least, these questions need to be answered:

What exactly is the behavior supposed to be? This must be documented.
How does this behavior relate to the standard iterator requirements? The problem with this patch is that most use cases rely on fudging the requirements and hoping that the standard library doesn't notice.
What can the patch break?

Dave Abrahams — Sat, 01 Dec 2012 20:25:43 GMT

Replying to Matthias Berndt <matthias_berndt@…>:

Replying to dave:

The current library maintainer wasn't assigned here so he hasn't seen your bump. I just fixed that.

Uh, did you?

Uh, yes.

In my world, a maintainer is somebody who maintains. Not committing a ready-made fix for a trivial, six-year-old bug is not what I'd call maintaining.

Even if this were trivial to apply (which it isn't for the reasons Steven cited) Jeffrey only took over maintainership this year. I'm trying to get your concern addressed, so please try to be a little gracious about it.

Sun, 02 Dec 2012 08:18:41 GMT

You're right, it's not as trivial as it seemed. Applying the patch fixes the reference/value semantics, but it breaks the iterator's operator->. iterator_facade's operator-> assumes that taking a reference's address yields something convertible to value_type*. I'm not sure what this assumption is based upon; I can't see anything in the New Iterator Concepts that justifies this assumption, but I may have missed something.