Skip to content

Fusion based zip_iterator, close #7526 #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 24, 2015
Merged

Fusion based zip_iterator, close #7526 #2

merged 9 commits into from
Aug 24, 2015

Conversation

Flast
Copy link
Contributor

@Flast Flast commented Jun 14, 2014

I reimplement zip_iterator with Boost.Fusion to support other tuple implementations.
And backward compatibility for Boost.Tuple should be appeared.

Notes for release manager: This change depends on boostorg/fusion#7 (already merged into develop, but not merged yet into master/release).
Ticket: https://svn.boost.org/trac/boost/ticket/7526

Flast added 7 commits June 12, 2014 01:05
  By default, backward compatibility for Boost.Tuple is presented.

Signed-off-by: Kohei Takahashi <flast@flast.jp>
Signed-off-by: Kohei Takahashi <flast@flast.jp>
Signed-off-by: Kohei Takahashi <flast@flast.jp>
Signed-off-by: Kohei Takahashi <flast@flast.jp>
Signed-off-by: Kohei Takahashi <flast@flast.jp>
Signed-off-by: Kohei Takahashi <flast@flast.jp>
@MauricioCarneiro
Copy link

👍 to this patch, any plans to merge it in? I'd love to be able to use zip_iterator with std::tuples.

@claashk
Copy link

claashk commented Mar 26, 2015

👍 from me as well. We have been waiting for this feature for too long already.

@Flast
Copy link
Contributor Author

Flast commented Mar 26, 2015

Notes for release manager: This change depends on boostorg/fusion#7 (already merged into develop, but not merged yet into master/release).

Note again, next release (i.e. 1.58.0) includes boostorg/fusion#7.

@eldiener
Copy link
Contributor

I have tested this out and it is working fine.

The docs for zip_iterator need to be updated to explain what the tuple type may be. I am willing to do this myself. Am I correct that the tuple type for zip_iterator may be a std::tuple, a boost::tuple, or any fusion sequence ? Or are we limited in any way by the fusion sequence we specify ?

@eldiener
Copy link
Contributor

My tests show that the implementation is failing if the tuple is a boost::fusion::deque. The failing is occurring when the code attempts to dereference a zip_iterator. The failing code, without the appropriate header files, is:

std::vector<double> vect1(3);
vect1[0] = 42.;
vect1[1] = 43.;
vect1[2] = 44.;

std::set<int> intset;
intset.insert(52);
intset.insert(53);
intset.insert(54);
//

typedef
boost::zip_iterator<
    boost::fusion::deque<
        std::set<int>::iterator
      , std::vector<double>::iterator
    >
> zit_mixed;

zit_mixed zip_it_mixed = zit_mixed(
  boost::fusion::make_deque(
      intset.begin()
    , vect1.begin()
  )
);

boost::fusion::deque<int, double> val_tuple(
    *zip_it_mixed);

Once the dereference of *zip_it_mixed occurs we get the errors:

In file included from ..\..\../boost/fusion/container/deque/convert.hpp:12:0,
                 from ..\..\../boost/fusion/container/deque.hpp:14,
                 from ..\..\../boost/fusion/include/deque.hpp:11,
                 from zip_iterator_test2_fusion_deque.cpp:3:
..\..\../boost/fusion/container/deque/detail/convert_impl.hpp: In instantiation of 'static          boost::fusion::extension::convert_impl<boost::fusion::deque_tag>::apply<Sequence>::type boost::fusion::extension::convert_impl<boost::fusion::deque_tag>::apply<Sequence>::call(Sequence&) [with Sequence = boost::fusion::transform_view<const boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >, boost::iterators::detail::dereference_iterator, boost::fusion::void_>; boost::fusion::extension::convert_impl<boost::fusion::deque_tag>::apply<Sequence>::type = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]':
..\..\../boost/fusion/sequence/convert.hpp:39:25:   required from 'typename boost::fusion::result_of::convert<Tag, Sequence>::type boost::fusion::convert(Sequence&) [with Tag = boost::fusion::deque_tag; Sequence = boost::fusion::transform_view<const boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >, boost::iterators::detail::dereference_iterator, boost::fusion::void_>; typename boost::fusion::result_of::convert<Tag, Sequence>::type = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]'
..\..\../boost/iterator/zip_iterator.hpp:214:40:   required from 'static reference boost::iterators::detail::converter<reference>::call(Seq) [with Seq = boost::fusion::transform_view<const boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >, boost::iterators::detail::dereference_iterator, boost::fusion::void_>; reference = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]'
..\..\../boost/iterator/zip_iterator.hpp:287:25:   required from 'typename boost::iterators::detail::zip_iterator_base<IteratorTuple>::type::reference boost::iterators::zip_iterator<IteratorTuple>::dereference() const [with IteratorTuple = boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >; typename boost::iterators::detail::zip_iterator_base<IteratorTuple>::type::reference = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]'
..\..\../boost/iterator/iterator_facade.hpp:549:32:   required from 'static typename Facade::reference boost::iterators::iterator_core_access::dereference(const Facade&) [with Facade = boost::iterators::zip_iterator<boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > > >; typename Facade::reference = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]'
..\..\../boost/iterator/iterator_facade.hpp:655:53:   required from 'boost::iterators::detail::iterator_facade_base<Derived, Value, CategoryOrTraversal, Reference, Difference, false, false>::reference boost::iterators::detail::iterator_facade_base<Derived, Value, CategoryOrTraversal, Reference, Difference, false, false>::operator*() const [with Derived = boost::iterators::zip_iterator<boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > > >; Value = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>; CategoryOrTraversal = boost::iterators::bidirectional_traversal_tag; Reference = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>; Difference = int; boost::iterators::detail::iterator_facade_base<Derived, Value, CategoryOrTraversal, Reference, Difference, false, false>::reference = boost::fusion::deque<const int&, double&, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_, boost::fusion::void_>]'
detail/zip_iterator_test_original.ipp:137:8:   required from here
..\..\../boost/fusion/container/deque/detail/convert_impl.hpp:44:37: error: 'call' is not a member of 'boost::fusion::extension::convert_impl<boost::fusion::deque_tag>::apply<boost::fusion::transform_view<const boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >, boost::iterators::detail::dereference_iterator, boost::fusion::void_> >::gen {aka boost::fusion::result_of::as_deque<boost::fusion::transform_view<const boost::fusion::deque<std::_Rb_tree_const_iterator<int>, __gnu_cxx::__normal_iterator<double*, std::vector<double> > >, boost::iterators::detail::dereference_iterator, boost::fusion::void_> >}'
                     return gen::call(seq);

I need some explanation for this or some resolution of this before I can push the PR. I think the PR has great merit but it either needs to work in all relevant cases or the reason it will not work with the fusion container boost::fusion::deque needs to be explained in the documentation. It does work with both boost::fusion::vector and boost::fusion::list as the tuple for the tuple of iterators in your implementation.

@eldiener
Copy link
Contributor

I discovered that the problem with boost::fusion::deque is in the fusion library and submitted a trac ticket against that problem at https://svn.boost.org/trac/boost/ticket/11572.

eldiener added a commit that referenced this pull request Aug 24, 2015
Fusion based zip_iterator, close #7526
@eldiener eldiener merged commit 2283f08 into boostorg:develop Aug 24, 2015
@Flast
Copy link
Contributor Author

Flast commented Aug 24, 2015

The docs for zip_iterator need to be updated to explain what the tuple type may be. I am willing to do this myself.

Thank you.

Am I correct that the tuple type for zip_iterator may be a std::tuple, a boost::tuple, or any fusion sequence ? Or are we limited in any way by the fusion sequence we specify ?

It requires at least Forward Sequence, i.e. any fusion sequence should be accepted.

I discovered that the problem with boost::fusion::deque is in the fusion library and submitted a trac ticket against that problem at https://svn.boost.org/trac/boost/ticket/11572.

Thanks, I will investigate and fix it until next release.

@Flast Flast deleted the pr/zip_iterator/fusionize branch August 24, 2015 04:50
@eldiener
Copy link
Contributor

I updated the iterator tests in order to test out specific fusion sequences as the tuple iterator for the zip_iterator. I added a test for the fusion deque sequence but commented it out in the jamfile. When a fix appears for trac ticket 11572, I will uncomment that test.

Are you helping to maintain Boost fusion ? If so fusion really needs tests for the generic sequence 'convert' functionality. I think the reason that the bug for trac ticket 11572 exists is that there are no fusion tests for the generic sequence 'convert' functionality, else the bug would have been found much earlier.

@Flast
Copy link
Contributor Author

Flast commented Aug 25, 2015

If so fusion really needs tests for the generic sequence 'convert' functionality. I think the reason that the bug for trac ticket 11572 exists is that there are no fusion tests for the generic sequence 'convert' functionality, else the bug would have been found much earlier.

Definitely.

Now, I open a PR to fix ticket 11572 and added tests for converting each other.

@morinmorin
Copy link
Member

Is this PR related to Trac ticket 12895?

@morinmorin
Copy link
Member

Daniel submitted PR #22 for trac ticket 12895.
(PR #23 is a testcase for it.)

Could someone merge them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants