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