Boost logo

Boost :

Subject: Re: [boost] [iterator] [property_map] Enhancement proposals
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2012-10-08 22:56:05

On Mon, Oct 8, 2012 at 9:11 AM, Louis Dionne <louis.dionne92_at_[hidden]>wrote:

> Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at>> writes:
> > Regarding accessor_iterator: The implementation is very short, but it
> still
> > seems to have minimal if any advantage to just using a transform_iterator
> > directly composed with a function object wrapping the pointer-to-member
> > (either at runtime or compile-time [1]). Also, seems you're lacking a
> > convenient make_accessor_iterator (which, of course, would probably have
> to
> > be a macro, as described in [1]).
> Regarding the make_accessor_iterator:
> I did not provide such a function because I did not know how to implement
> it.
> After reading the link, I indeed see how it could be done. I will do it if
> the
> class is accepted.

Yeah, it is tricky, and the referenced solution is pretty clever :)

> Another possibility is to set the pointer to member at runtime, which then
> makes the implementation of make_accessor_iterator trivial. You can see
> this
> alternative implementation on the dynamic_accessor_iterator branch of the
> git
> repository.

Hopefully you'll excuse me for making an educated guess on what the
implementation would look like.

> Regarding the usefulness of the class vs using transform iterator:
> I too believe the same thing can be achieved using a transform_iterator. It
> really depends on whether you think creating a function object is a pain.
> It
> would be useful to know what others think of it to know whether there is a
> need in the community.

It's just unclear to me where one should draw the line as far as what
iterators should be provided by Boost.Iterator out of the box. Neither your
original accessor_iterator nor dynamic_accessor_iterator cover what I would
imagine to be the equally common case of applying a nullary member function
to an iterator's value. So I can easily imagine 6 different variants of
your accessor_iterator: { static, dynamic } x { member data pointer,
non-const member function pointer, const member function pointer }. If the
community pushes for including these variants directly in Boost.Iterator,
that's fine, but I'm disinclined to do so otherwise given the simplicity of
combining transform_iterator with the appropriate function object. Further,
I don't consider the creation and use of the appropriate function objects
to be a pain, given the present solutions within Boost.

> This could be useful, but there's a few things I need to understand about
> > your implementation first.
> > - Using is_convertible to determine iterator convertibility is, strictly
> > speaking, too loose, as it allows derived* -> base* conversions that are
> > undesirable if sizeof( derived ) > sizeof( base ). At some point I
> probably
> > need to fish through the Iterator library and change any such uses
> myself...
> Given the 3 places where I use it currently:
> template <typename OutIter>
> explicit chained_output_iterator(Function const&, OutIter const&,
> typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
> template <typename OutIter>
> chained_output_iterator(OutIter const&,
> typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
> template <typename Func, typename OutIter>
> chained_output_iterator(chained_output_iterator<Func, OutIter> const&,
> typename enable_if_convertible<Func, Function>::type* =0,
> typename enable_if_convertible<OutIter, OutputIterator>::type* =0);
> I take it that you are referring to the first two copy constructors being
> too
> loose on the `OutIter' type. Would the following be alright, given that
> OutputIterator is the type of the wrapped iterator?
> explicit chained_output_iterator(Function const&, OutputIterator
> const&);
> chained_output_iterator(OutputIterator const&);

Sorry, I had another (albeit related) concern in mind. Yes, I think it's
simpler to just make the constructor parameters the wrapped iterator type

The instances where one needs to be a bit careful are converting
constructors from one chained_output_iterator to another.

> > - I'm wondering why the increment of the wrapped iterator happens inside
> > proxy::operator= rather than chained_output_iterator::operator++...?
> Also,
> You are right, this is my mistake and it's fixed on master now.
> Since this change would make the implementation with
> function_output_iterator
> more complicated than the `from scratch' implementation, I believe
> function_output_iterator should not be used.
> Note: The implementation using function_output_iterator comes from a
> suggestion in your first reply and is available on the
> using_function_output branch of the repository.

Hmmm, yeah I don't recall anything involving function_output_iterator, I
must've missed something...

> > typically operator* is a const member function, and moving the
> advancement
> > of the wrapped iterator into chained_output_iterator::operator++ would
> > allow that. I might be missing something though.
> In order to make the chained_output_iterator::operator* const, we would
> need
> to make the proxy hold copies or const references to the wrapped iterator
> and
> function. However, using const references would add the following
> restrictions:
> - the wrapped iterator's operator* must be const

The typical case...

> - the function object's operator() must be const

Again, the typical case. As a general rule of thumb, you have to be careful
with state-mutating function objects immediately held within iterators, but
I guess strictly incrementable iterators (as chained_output_iterator is)
have more predictable access patterns so it's less of a concern.

> For these reasons, I believe the proxy should either store copies or we
> should
> give up making chained_output_iterator::operator* const. I'll wait for your
> reply before I change that.

You can always provide the best of both by providing both const and
non-const overloads of operator* (and likewise "const" and "non-const"
proxy classes).

I think I see chained_function_output_iterator as a viable addition to
Boost.Iterator; accessor_iterator, I'm yet to be convinced. Nothing
personal :)

- Jeff

Boost list run by bdawes at, gregod at, cpdaniel at, john at