Re: [boost] Constant iterator of Single Pass Range

Subject: Re: [boost] Constant iterator of Single Pass Range
From: Neil Groves (neil_at_[hidden])
Date: 2014-05-05 13:38:59

>
> All containers are at least forward ranges, not merely single pass ones.
> Since iterating a forward range does not change it, there is no problem
> for it
> to have a `const` `begin()` and `end()` methods (returning const
> iterators).
>
> What I had issue with is single pass range. Consider something that behaves
> like `std::istream`. The `std::istream_iterator` can be only constructed
> from
> non-const reference to `std::istream`, because the
> `std::istream_iterator::operator++` (and the constructor itself) call the
> `std::istream::operator>>`, which is non-const.
>
>
I apologise for misunderstanding your original post. In this scenario the
range_const_iterator meta-function may validly return a mutable iterator.
The purpose of the range_const_iterator meta-function is to define the
iterator type for functions that use the const Range& overloads. The
range_const_iterator does not have to return a type that is a constant
iterator. The separate mechanism is necessary (pre C++11) since there are
many examples where the type of iterator needs to be determined differently
for the const and non-const function overloads. In most cases, of course,
the range_const_iterator does return a const_iterator. You have correctly
highlighted that sometimes you need to do something else. I believe that,
if I have understood your example correctly, that you can do this by simply
defining range_const_iterator to return a mutable iterator. The example I
linked to previously does just this. It returns the iterator type from a
pair. It returns the same type whether or not the range is passed as a
const or mutable reference.

> Well, it can't be implemented using non-const methods of the _range_
> _itself_.
>
> If I have a stream-like object
>
>     class int_reader {
>        ...
>        int get_next(); // NOT const
>     };
>
> than I can create iterators for it. But the iterators will look like
>
>     class int_reader_iterator :
>         boost::iterator_facade<int_reader_iterator, int,
> boost::single_pass_traversal_tag, int const &>
>     {
>         int_reader *reader; // NOT const
>         ...
>         void advance() { value = reader->get_next(); } // must NOT be
> const HERE
>         ...
>         int_reader_iterator(int_reader &reader) : reader(&reader) { } //
> can't be const here
>     };
>
>
As mentioned above, you don't need to create a const_iterator that isn't a
const iterator. You can just return the mutable iterator from
range_const_iterator. It seems to me that this type doesn't really have a
const_iterator. It isn't sensible for me to make a special case for
single_pass_traversal versions of range_const_iterator since this is merely
one of a number of reasons one might wish to return a mutable iterator from
range_const_iterator. Any change for single_pass_traversal would be
insufficient while adding complexity to the rules of extension.

> And therefore I can't provide
>
>     int_reader_iterator range_begin(int_reader_iterator const &);
>
> And therefore I can't make `int_reader` a range. I can make a range
> adapter for it,
> that will break the const chain like `boost::istream_range` does.
>
> Now why I wanted that was that I wanted to work with temporaries and was
> concerned
> that I would get dangling reference, because BOOST_FOREACH will extend
> life of the
> passed range by binding it to a reference, but that does not extend to
> arguments of
> the expression. Basically I thought of writing something like
>
>     BOOST_FOREACH(int i, boost::input_range<int>(std::
> ifstream("file.txt")))
>        ...
>
> Now this won't extend the life of the ifstream, but fortunately it won't
> compile
> either, because input_range requires a non-const reference.
>
>
The potential for lifetime issues when combining temporaries with
BOOST_FOREACH is a known problem without a good solution with broad
compatibility with C++03. In many cases while the range lifetime may end,
the underlying iterators live on. The BOOST_FOREACH library then behaves
correctly in many cases where it might look like undefined behaviour ought
to occur. There are however some lifetime issues for example applying range
adaptors to a container returned by value. The solution, for now at least,
is to create a temporary variable before the BOOST_FOREACH statement.

I did consider the use of a slightly naughty const range to try and extend
the lifetime, but it is rather odd and doesn't work in the general case.

> And if I had my object that would directly be range, but didn't have const
> begin,
> well, then BOOST_FOREACH wouldn't accept a temporary either, because
> temporary
> only binds to const reference. So the object that needs to be modified
> can't
> be a temporary whatever I do and I can wrap it and don't need to bother.
>

I didn't write BOOST_FOREACH, but I believe that it isn't accepting the
temporary rather deliberately to avoid lifetime issues.

>
> What is still a problem is wrapper of a wrapper, but it's enough to
> simplify that
> to one level of wrapper.
>
>
I believe I understand your aim. I rejected this direction of design since
it only helps in a small number of cases. I would love to have a complete
solution and have spent considerable time to no avail. In my production
code I resort to one of:
1. Use one of the Boost.Range algorithms such as boost::push_back or
boost::transform (works with versions of C++ prior to C++11)
2. Add a temporary (works with versions of C++ prior to C++11)
3. Replace BOOST_FOREACH with the C++11 range-base for loop
4. Replace BOOST_FOREACH with boost::for_each

>
> --
> Jan Hudec <bulb_at_[hidden]>
>

I hope I've understood the issue properly this time around. If you are
using C++11 then simply not using BOOST_FOREACH is probably the best
solution. The other alternatives I listed above lack elegance in some
cases, but do work reliably.

Regards,
Neil Groves