From f1aa7fa7758b0c5e19743c528f357f44f145377d Mon Sep 17 00:00:00 2001
From: Danny Robson <danny@nerdcruft.net>
Date: Tue, 25 Feb 2020 16:17:04 +1100
Subject: [PATCH] iterator/zip: rewrite for simplicity and forwarding support

---
 iterator/zip.hpp | 126 ++++++++++++++++++++++++++---------------------
 1 file changed, 69 insertions(+), 57 deletions(-)

diff --git a/iterator/zip.hpp b/iterator/zip.hpp
index cc07d2d0..ccad083f 100644
--- a/iterator/zip.hpp
+++ b/iterator/zip.hpp
@@ -19,17 +19,21 @@ namespace cruft::iterator {
     namespace detail::zip {
         /// A container that holds multiple iterators and returns a tuple of
         /// their results when dereferenced.
-        ///
-        /// \tparam IteratorT  A tuple-like object that contains iterators
         template <
             typename IteratorT,
             typename = std::make_index_sequence<std::tuple_size_v<IteratorT>>
         >
-        struct zipped_iterator;
+        class zipped_iterator;
 
 
-        template <typename IteratorT, std::size_t ...Indices>
-        struct zipped_iterator<IteratorT, std::index_sequence<Indices...>> {
+        template <
+            typename IteratorT,
+            std::size_t ...IndexV
+        >
+        class zipped_iterator<
+            IteratorT,
+            std::index_sequence<IndexV...>
+        > {
         public:
             // We can't declare ourselves as a forward_iterator because we're
             // unable to supply references to our value_type when we get
@@ -41,25 +45,22 @@ namespace cruft::iterator {
             using iterator_category = std::input_iterator_tag;
             using difference_type = std::ptrdiff_t;
             using value_type = std::tuple<
-                decltype(
-                    *std::get<Indices> (
-                        std::declval<IteratorT> ()
-                    )
-                )...
+                typename std::iterator_traits<
+                    std::tuple_element_t<IndexV, IteratorT>
+                >::reference...
             >;
             using reference = value_type;
             using pointer = value_type*;
 
-
-            zipped_iterator (IteratorT _iterators):
-                m_iterators (_iterators)
+            zipped_iterator (IteratorT _iterators)
+                : m_iterators (std::move (_iterators))
             { ; }
 
 
             zipped_iterator&
             operator++ (void)
             {
-                (++std::get<Indices> (m_iterators), ...);
+                (++std::get<IndexV> (m_iterators), ...);
                 return *this;
             }
 
@@ -70,14 +71,8 @@ namespace cruft::iterator {
             decltype(auto)
             operator* (void)
             {
-                // We deliberately construct a tuple manually here to reduce
-                // the risk of dangling references. `forward_as_tuple` and
-                // `make_tuple` have resulted in references to locals in the
-                // past.
-                return std::tuple<
-                    decltype(*std::get<Indices> (m_iterators))...
-                > (
-                    *std::get<Indices> (m_iterators)...
+                return std::forward_as_tuple (
+                    *std::get<IndexV> (m_iterators)...
                 );
             }
 
@@ -85,27 +80,21 @@ namespace cruft::iterator {
             decltype (auto)
             operator* (void) const
             {
-                // We deliberately construct a tuple manually here to reduce
-                // the risk of dangling references. `forward_as_tuple` and
-                // `make_tuple` have resulted in references to locals in the
-                // past.
-                return std::tuple<
-                    decltype(*std::get<Indices> (m_iterators))...
-                > (
-                    *std::get<Indices> (m_iterators)...
+                return std::forward_as_tuple (
+                    *std::get<IndexV> (m_iterators)...
                 );
             }
 
 
             bool
-            operator== (const zipped_iterator &rhs) const
+            operator== (zipped_iterator const &rhs) const
             {
                 return m_iterators == rhs.m_iterators;
             }
 
 
             bool
-            operator!= (const zipped_iterator &rhs) const
+            operator!= (zipped_iterator const &rhs) const
             {
                 return !(*this == rhs);
             }
@@ -116,6 +105,10 @@ namespace cruft::iterator {
         };
 
 
+        template <typename TupleT>
+        zipped_iterator (TupleT) -> zipped_iterator<TupleT>;
+
+
         /// A simple container for multiple collections that returns a
         /// wrapped multi-iterator for begin and end.
         ///
@@ -124,46 +117,64 @@ namespace cruft::iterator {
         ///
         /// \tparam StoreT  A tuple-like object of collections (or references
         ///                 to collections).
-        template <typename ...StoreT>
-        class collection {
+        template <
+            typename StoreT,
+            typename = std::make_index_sequence<
+                std::tuple_size_v<StoreT>
+            >
+        >
+        class collection;
+
+        template <
+            typename StoreT,
+            std::size_t ...IndexV
+        > class collection<
+            StoreT,
+            std::index_sequence<IndexV...>
+        > {
         public:
-            collection (StoreT&&... _store):
-                m_store (std::forward<StoreT> (_store)...)
+            collection (StoreT _store)
+                : m_store (std::move (_store))
             { ; }
 
-            using inner_t = std::tuple<decltype(std::begin (std::declval<StoreT> ()))...>;
-            using indices_t  = std::make_index_sequence<sizeof...(StoreT)>;
 
-            using iterator = zipped_iterator<inner_t, indices_t>;
-
-
-            iterator begin (void)&
+            auto begin (void)&
             {
-                return iterator (
-                    tuple::value::map (::cruft::functor::begin {}, m_store)
+                return zipped_iterator (
+                    std::tuple (
+                        std::begin (
+                            std::get<IndexV> (m_store)
+                        )...
+                    )
                 );
             }
 
-            iterator begin (void) const&
+            auto begin (void) const&
             {
-                return iterator (
-                    tuple::value::map (::cruft::functor::begin {}, m_store)
+                return zipped_iterator (
+                    std::tuple (
+                        std::begin (std::get<IndexV> (m_store))...
+                    )
                 );
             }
 
 
-            iterator end (void)&
+            auto end (void)&
             {
-                return iterator (
-                    tuple::value::map (cruft::functor::end {}, m_store)
+                return zipped_iterator (
+                    std::tuple (
+                        std::end (std::get<IndexV> (m_store))...
+                    )
                 );
             }
 
 
-            iterator end (void) const &
+            auto end (void) const &
             {
-                return iterator (
-                    tuple::value::map (cruft::functor::end {}, m_store)
+                return zipped_iterator (
+                    std::tuple (
+                        std::end (std::get<IndexV> (m_store))...
+                    )
                 );
             }
 
@@ -177,9 +188,8 @@ namespace cruft::iterator {
                 return std::size (std::get<0> (m_store));
             }
 
-
         private:
-            std::tuple<StoreT...> m_store;
+            StoreT m_store;
         };
     }
 
@@ -200,8 +210,10 @@ namespace cruft::iterator {
     {
         CHECK (((std::size (data) == std::size (variadic::get<0> (data...))) && ...));
 
-        return detail::zip::collection<ContainerT...> (
-            std::forward<ContainerT> (data)...
+        return detail::zip::collection<std::tuple<ContainerT...>> (
+            std::forward_as_tuple (
+                std::forward<ContainerT> (data)...
+            )
         );
     }