Re: [Boost-bugs] [Boost C++ Libraries] #10422: Counting Iterator return-type causes lifetime issues

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #10422: Counting Iterator return-type causes lifetime issues
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2014-08-29 17:19:19

#10422: Counting Iterator return-type causes lifetime issues
-------------------------------+------------------------------
  Reporter:  neilgroves        |      Owner:  jeffrey.hellrung
      Type:  Bugs              |     Status:  new
 Milestone:  To Be Determined  |  Component:  iterator
   Version:  Boost 1.56.0      |   Severity:  Problem
Resolution:                    |   Keywords:
-------------------------------+------------------------------

Comment (by neilgroves):

 As requested by Eric I have included his thoughts on one of the possible
 solutions:

 On 8/28/2014 3:49 PM, Neil Groves wrote:
 > On 28 August 2014 22:35, Krzysztof Czainski wrote:
 >
 >> I think the problem is counting_iterator returns a reference to an
 >> int it owns. Iterators shouldn't do that.
 >
 > This is exactly what is causing the issue.

 Right.

 >> Iterators should either:
 >> - return a reference to something they don't own, or
 >> - return by value.
 >
 > Your logic for return type makes a lot of sense. Unfortunately there
 > are conflated requirements upon the return type of dereference
 > operations for the various iterator traversals. See
 >
 http://www.boost.org/doc/libs/1_56_0/libs/multi_array/doc/iterator_categories.html.


 I see no conflated requirements. In Boost.Iterators (as opposed to the
 standard iterator categories), iterator *traversal* deals only with
 traversal (++, --, +=, -=, etc) and says nothing about the return type
 of the dereference operation.

 Krzysztof is right, though. The dereference operation should return an
 rvalue. That would make the counting_iterator a Readable iterator in the
 new-style categories. "Readable" is orthogonal to the iterator traversal.

 So IMO, counting_iterator should be a Readable Iterator and a Random
 Access Traversal Iterator. When this is mapped to one of the standard
 iterator categories, it becomes std::input_iterator_tag (because of the
 requirements on Forward Iterators in the standard).

 > I imagine that this is why the counting_iterator is implemented to
 > return a true reference.

 No, I think this is a bug, plain and simple. Fixing it is likely to
 cause trouble though. :-(

 > This leaves us with rather a thorny problem, a counting_iterator
 > can't have the dereference operator return by value and model the
 > Bidirectional Iterator Concept. Therefore if counting_iterator were
 > to return by value we wouldn't be allowed to reverse (if we
 > constrained ourselves to the Standard iterator traversal categories).
 > It might be that there is room for improvement while working with the
 > Boost traversal tags rather than the iterator traversal tags.

 If Boost.Range were made aware of the Boost new-style iterator
 categories -- which make traversal and access independent -- the
 reversed view could still work with counting_iterator. The resulting
 view's iterators would also be Random Access Traversal and Readable.

 Making all of Boost.Range aware of the new-style iterators would be a
 piece of work, though.

 > Thanks for pointing this out, I hadn't spotted this issue with the
 > counting_iterator. After giving this some more thought I shall raise
 > a ticket for Boost.Iterator.

 Indeed. Please include this discussion in the bug report.

 Eric

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/10422#comment:1>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.