From ec1f354d1628c835f5b6e90309c95a7ad392a719 Mon Sep 17 00:00:00 2001
From: Danny Robson <danny@nerdcruft.net>
Date: Wed, 26 Feb 2020 12:00:29 +1100
Subject: [PATCH] iterator/zip: avoid std::forward_as_tuple for zip value_type

This tends to result in references to temporaries higher up the stack
being returned. decltype on the iterator dereference is a more reliable
method to determine the return type.
---
 iterator/zip.hpp | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/iterator/zip.hpp b/iterator/zip.hpp
index ccad083f..b02e15fc 100644
--- a/iterator/zip.hpp
+++ b/iterator/zip.hpp
@@ -44,11 +44,13 @@ namespace cruft::iterator {
             // pressing need for this functionality.
             using iterator_category = std::input_iterator_tag;
             using difference_type = std::ptrdiff_t;
+
             using value_type = std::tuple<
                 typename std::iterator_traits<
                     std::tuple_element_t<IndexV, IteratorT>
                 >::reference...
             >;
+
             using reference = value_type;
             using pointer = value_type*;
 
@@ -68,19 +70,39 @@ namespace cruft::iterator {
             zipped_iterator operator++ (int);
 
 
-            decltype(auto)
+            // Do NOT use std::forward_as_tuple here. It favours rvalue
+            // references which aren't useful after we've returned. decltype
+            // gives us lvalues and lvalue-references.
+            //
+            // If you insist on using another approach then you should test
+            // under a sanitizer (debug and release) before you commit the
+            // changes.
+            auto
             operator* (void)
             {
-                return std::forward_as_tuple (
+                return std::tuple<
+                    decltype (*std::get<IndexV> (m_iterators))...
+                > (
                     *std::get<IndexV> (m_iterators)...
                 );
             }
 
 
-            decltype (auto)
+            // Do NOT use std::forward_as_tuple here. It favours rvalue
+            // references which aren't useful after we've returned. decltype
+            // gives us lvalues and lvalue-references.
+            //
+            // If you insist on using another approach then you should test
+            // under a sanitizer (debug and release) before you commit the
+            // changes.
+            auto
             operator* (void) const
             {
-                return std::forward_as_tuple (
+                return std::tuple<
+                    std::add_const_t<
+                        decltype(*std::get<IndexV> (m_iterators))
+                    >...
+                > (
                     *std::get<IndexV> (m_iterators)...
                 );
             }