Opened 15 years ago

Last modified 10 years ago

#973 new Bugs

zip_iterator has value_type == reference

Reported by: anonymous Owned by: jeffrey.hellrung
Milestone: Component: iterator
Version: Boost 1.34.0 Severity: Problem
Keywords: Cc: matthias_berndt@…

Description (last modified by Dave Abrahams)

Change History (12)

comment:1 by Dave Abrahams, 15 years ago

Description: modified (diff)
Owner: set to Thomas Witt

comment:2 by dtrebbien@…, 12 years ago

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

comment:3 by blackhc@…, 10 years ago

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...

comment:4 by anonymous, 10 years ago

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?

comment:5 by Matthias Berndt <matthias_berndt@…>, 10 years ago

Cc: matthias_berndt@… added

comment:6 by Matthias Berndt <matthias_berndt@…>, 10 years ago

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?

comment:7 by Dave Abrahams, 10 years ago

Owner: changed from Thomas Witt to jeffrey.hellrung

comment:8 by Dave Abrahams, 10 years ago

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

in reply to:  8 ; comment:9 by Matthias Berndt <matthias_berndt@…>, 10 years ago

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?

comment:10 by Steven Watanabe, 10 years ago

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?

in reply to:  9 comment:11 by Dave Abrahams, 10 years ago

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.

comment:12 by Matthias Berndt <matthias_berndt@…>, 10 years ago

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.

Note: See TracTickets for help on using tickets.