Add unit test for zip iterator

SConstruct

@@ -4404,6 +4404,9 @@ class DXXArchive(DXXCommon):
 		RuntimeTest('test-xrange', (
 			'common/unittest/xrange.cpp',
 			)),
+		RuntimeTest('test-zip', (
+			'common/unittest/zip.cpp',
+			)),
 			)
 	del RuntimeTest
 

common/main/d_zip.h

@@ -6,38 +6,76 @@
  */
 #pragma once
 
+#include <iterator>
 #include <tuple>
 #include <type_traits>
 #include "dxxsconf.h"
 #include "compiler-integer_sequence.h"
 #include "ephemeral_range.h"
 
-/* This iterator terminates when the first zipped range terminates.  The caller
- * is responsible for ensuring that the first zipped range is not longer than
- * any other zipped range.
+namespace d_zip {
+
+namespace detail {
+
+template <typename... T>
+void discard_arguments(T &&...)
+{
+}
+
+template <std::size_t... N, typename... range_iterator_type>
+void increment_iterator(std::tuple<range_iterator_type...> &iterator, std::index_sequence<N...>)
+{
+	/* Order of evaluation is irrelevant, so pass the results to a
+	 * discarding function.  This permits the compiler to evaluate the
+	 * expression elements in any order.
+	 */
+	discard_arguments(++(std::get<N>(iterator))...);
+}
+
+template <std::size_t... N, typename... range_iterator_type>
+auto dereference_iterator(const std::tuple<range_iterator_type...> &iterator, std::index_sequence<N...>)
+{
+	/* std::make_tuple is not appropriate here, because the result of
+	 * dereferencing the iterator may be a reference, and the resulting
+	 * tuple should store the reference, not the underlying object.
+	 * Calling std::make_tuple would decay such references into the
+	 * underlying type.
+	 *
+	 * std::tie is not appropriate here, because it captures all
+	 * arguments as `T &`, so it fails to compile if the result of
+	 * dereferencing the iterator is not a reference.
+	 */
+	return std::tuple<
+		decltype(*(std::get<N>(iterator)))...
+		>(*(std::get<N>(iterator))...);
+}
+
+}
+
+}
+
+/* This iterator terminates when the first zipped range terminates.  The
+ * caller is responsible for ensuring that use of the zip_iterator does
+ * not increment past the end of any zipped range.  This can be done by
+ * ensuring that the first zipped range is not longer than any other
+ * zipped range, or by ensuring that external logic stops the traversal
+ * before the zip_iterator increments past the end.
+ *
+ * There is no initialization time check that the below loop would be
+ * safe, since a check external to the zip_iterator could stop before
+ * undefined behaviour occurs.
+
+	for (auto i = zip_range.begin(), e = zip_range.end(); i != e; ++i)
+	{
+		if (condition())
+			break;
+	}
+
  */
 template <typename... range_iterator_type>
 class zip_iterator : std::tuple<range_iterator_type...>
 {
 	using base_type = std::tuple<range_iterator_type...>;
-	template <typename... T>
-		void discard_arguments(T &&...)
-		{
-		}
-	template <std::size_t... N>
-		void increment(std::index_sequence<N...>)
-		{
-			/* Order of evaluation is irrelevant, so pass the results to
-			 * a discarding function.  This permits the compiler to
-			 * evaluate the expression elements in any order.
-			 */
-			discard_arguments(++(std::get<N>(*this))...);
-		}
-	template <std::size_t... N>
-		auto dereference(std::index_sequence<N...>) const
-		{
-			return std::tie(*(std::get<N>(*this))...);
-		}
 protected:
 	/* Prior to C++17, range-based for insisted on the same type for
 	 * `begin` and `end`, so method `end_internal` must return a full iterator,
@@ -65,11 +103,11 @@ public:
 	using base_type::base_type;
 	auto operator*() const
 	{
-		return dereference(index_type());
+		return d_zip::detail::dereference_iterator(*this, index_type());
 	}
 	zip_iterator &operator++()
 	{
-		increment(index_type());
+		d_zip::detail::increment_iterator(*this, index_type());
 		return *this;
 	}
 	bool operator!=(const zip_iterator &i) const
@@ -101,6 +139,8 @@ public:
 template <typename range0, typename... rangeN>
 static auto zip(range0 &&r0, rangeN &&... rN)
 {
+	using std::begin;
+	using std::end;
 	using R = zipped_range<decltype(begin(r0)), decltype(begin(rN))...>;
 	static_assert((!any_ephemeral_range<range0 &&, rangeN &&...>::value), "cannot zip storage of ephemeral ranges");
 	return R(typename R::iterator(begin(r0), begin(rN)...), end(r0));

common/unittest/zip.cpp

@@ -0,0 +1,67 @@
+#include "d_range.h"
+#include "d_zip.h"
+#include <array>
+#include <vector>
+
+#define BOOST_TEST_DYN_LINK
+#define BOOST_TEST_MODULE Rebirth zip
+#include <boost/test/unit_test.hpp>
+
+/* Test that a zipped range is empty when the component range is empty.
+ */
+BOOST_AUTO_TEST_CASE(zip_empty)
+{
+	bool empty = true;
+	std::array<int, 0> a;
+	for (auto &&v : zip(a))
+	{
+		(void)v;
+		empty = false;
+	}
+	BOOST_TEST(empty);
+}
+
+/* Test that a zipped range is the length of the first sequence.
+ *
+ * Note that there is no test for when the first sequence is longer than
+ * the later ones, since such a test would increment some iterators past
+ * their end.
+ */
+BOOST_AUTO_TEST_CASE(zip_length)
+{
+	unsigned count = 0;
+	std::array<int, 2> a;
+	short b[4];
+	for (auto &&v : zip(a, b))
+	{
+		(void)v;
+		++ count;
+	}
+	BOOST_TEST(count == a.size());
+}
+
+/* Test that a zipped value references the underlying storage.
+ */
+BOOST_AUTO_TEST_CASE(zip_passthrough_modifications)
+{
+	std::array<int, 1> a{{1}};
+	short b[1]{2};
+	for (auto &&v : zip(a, b))
+	{
+		++ std::get<0>(v);
+		++ std::get<1>(v);
+	}
+	BOOST_TEST(a[0] == 2);
+	BOOST_TEST(b[0] == 3);
+}
+
+/* Test that a zipped range can zip an xrange, and produce the correct
+ * underlying value.
+ */
+BOOST_AUTO_TEST_CASE(zip_xrange)
+{
+	for (auto &&v : zip(xrange(1u)))
+	{
+		BOOST_TEST(std::get<0>(v) == 0);
+	}
+}