From 797c8b47b271deca68965341765d1b33c00eb25c Mon Sep 17 00:00:00 2001
From: Kp <kp@valhallalegends.com>
Date: Sat, 19 Mar 2016 19:08:10 +0000
Subject: [PATCH] Move segment_object_iterator_t into segment_object_range_t

---
 common/main/segiter.h | 55 ++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/common/main/segiter.h b/common/main/segiter.h
index 636145e56..cd951e8c0 100644
--- a/common/main/segiter.h
+++ b/common/main/segiter.h
@@ -15,48 +15,65 @@ struct unspecified_pointer_t;
 };
 
 template <typename T>
-class segment_object_iterator_t : public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, typename detail::unspecified_pointer_t<T>, T>, T
+class segment_object_range_t
+{
+	class iterator;
+	iterator b;
+public:
+	segment_object_range_t(T &&o) :
+		b(std::move(o))
+	{
+	}
+	iterator begin() const { return b; }
+	iterator end() const { return T(object_none); }
+};
+
+template <typename T>
+class segment_object_range_t<T>::iterator :
+	public std::iterator<std::forward_iterator_tag, T, std::ptrdiff_t, typename detail::unspecified_pointer_t<T>, T>,
+	T
 {
 	using T::m_ptr;
 	using T::m_idx;
 public:
-	segment_object_iterator_t(T o) :
-		T(o)
+	iterator(T &&o) :
+		T(std::move(o))
 	{
 	}
 	T operator *() { return *static_cast<T *>(this); }
-	segment_object_iterator_t &operator++()
+	iterator &operator++()
 	{
 		const auto ni = m_ptr->next;
-		m_ptr = (ni == object_none) ? nullptr : m_ptr + (static_cast<std::size_t>(ni) - static_cast<std::size_t>(m_idx));
+		const auto oi = m_idx;
 		m_idx = ni;
+		/* If ni == object_none, then the iterator is at end() and there should
+		 * be no further reads of m_ptr.  Optimizing compilers will typically
+		 * recognize that setting m_ptr=nullptr in this case is unnecessary and
+		 * omit the store.  Omit the nullptr assignment so that less optimizing
+		 * compilers do not generate a useless branch.
+		 */
+		if (ni != object_none)
+			m_ptr += static_cast<std::size_t>(ni) - static_cast<std::size_t>(oi);
 		return *this;
 	}
-	bool operator==(const segment_object_iterator_t &rhs) const
+	bool operator==(const iterator &rhs) const
 	{
 		return m_idx == rhs.m_idx;
 	}
-	bool operator!=(const segment_object_iterator_t &rhs) const
+	bool operator!=(const iterator &rhs) const
 	{
 		return !(*this == rhs);
 	}
 };
 
-template <typename T>
-struct segment_object_range_t
-{
-	segment_object_iterator_t<T> b;
-	segment_object_range_t(T o) :
-		b(o)
-	{
-	}
-	segment_object_iterator_t<T> begin() const { return b; }
-	segment_object_iterator_t<T> end() const { return T(object_none); }
-};
-
 __attribute_warn_unused_result
 static inline segment_object_range_t<objptridx_t> objects_in(segment &s)
 {
+	/* The factory function invocation cannot be moved out of the
+	 * individual branches because the ternary operator calls different
+	 * overloads of the factory function, depending on whether the true
+	 * branch or false branch is taken.
+	 */
 	return s.objects == object_none ? objptridx(object_none) : objptridx(s.objects);
 }