From 5ec11ce13a6bf4479205f3cf2a9cc96342df7f60 Mon Sep 17 00:00:00 2001
From: Behdad Esfahbod <behdad@behdad.org>
Date: Thu, 27 Dec 2018 17:17:28 -0500
Subject: [PATCH] [iter] Clarify readonly vs lvalue iterators

lvalue iterators must declare __item_type__ as a reference.
---
 src/hb-array.hh  |  6 +++---
 src/hb-iter.hh   | 11 ++++++-----
 src/hb-set.hh    |  6 +++---
 src/test-iter.cc |  6 ++++--
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/hb-array.hh b/src/hb-array.hh
index 7dd02cdae..05d37a719 100644
--- a/src/hb-array.hh
+++ b/src/hb-array.hh
@@ -38,8 +38,8 @@ struct hb_sorted_array_t;
 
 template <typename Type>
 struct hb_array_t :
-	hb_iter_t<hb_array_t<Type>, Type>,
-	hb_iter_mixin_t<hb_array_t<Type>, Type>
+	hb_iter_t<hb_array_t<Type>, Type&>,
+	hb_iter_mixin_t<hb_array_t<Type>, Type&>
 {
   /*
    * Constructors.
@@ -52,7 +52,7 @@ struct hb_array_t :
   /*
    * Iterator implementation.
    */
-  typedef Type __item_type__;
+  typedef Type& __item_type__;
   Type& __item_at__ (unsigned i) const
   {
     if (unlikely (i >= length)) return CrapOrNull (Type);
diff --git a/src/hb-iter.hh b/src/hb-iter.hh
index 6d4adca27..6b49f803c 100644
--- a/src/hb-iter.hh
+++ b/src/hb-iter.hh
@@ -60,9 +60,10 @@ struct hb_iter_t
   iter_t iter () const { return *thiz(); }
   explicit_operator bool () const { return thiz()->__more__ (); }
   unsigned len () const { return thiz()->__len__ (); }
-  item_t* operator -> () const { return hb_addressof (*thiz()); }
-  item_t& operator * () const { return thiz()->__item__ (); }
-  item_t& operator [] (unsigned i) const { return thiz()->__item_at__ (i); }
+  /* TODO enable_if item_t is reference type only. */
+  typename hb_remove_reference (item_t)* operator -> () const { return hb_addressof (*thiz()); }
+  item_t operator * () const { return thiz()->__item__ (); }
+  item_t operator [] (unsigned i) const { return thiz()->__item_at__ (i); }
   iter_t& operator += (unsigned count) { thiz()->__forward__ (count); return *thiz(); }
   iter_t& operator ++ () { thiz()->__next__ (); return *thiz(); }
   iter_t& operator -= (unsigned count) { thiz()->__rewind__ (count); return *thiz(); }
@@ -120,8 +121,8 @@ struct hb_iter_mixin_t
   public:
 
   /* Access: Implement __item__(), or __item_at__() if random-access. */
-  item_t& __item__ () const { return (*thiz())[0]; }
-  item_t& __item_at__ (unsigned i) const { return *(*thiz() + i); }
+  item_t __item__ () const { return (*thiz())[0]; }
+  item_t __item_at__ (unsigned i) const { return *(*thiz() + i); }
 
   /* Termination: Implement __more__(), or __len__() if random-access. */
   bool __more__ () const { return thiz()->len (); }
diff --git a/src/hb-set.hh b/src/hb-set.hh
index b8f4a8f93..1015741f5 100644
--- a/src/hb-set.hh
+++ b/src/hb-set.hh
@@ -672,14 +672,14 @@ struct hb_set_t
    * Iterator implementation.
    */
   struct iter_t :
-    hb_sorted_iter_t<iter_t, const hb_codepoint_t>,
-    hb_iter_mixin_t<iter_t, const hb_codepoint_t>
+    hb_sorted_iter_t<iter_t, hb_codepoint_t>,
+    hb_iter_mixin_t<iter_t, hb_codepoint_t>
   {
     iter_t (const hb_set_t &s_ = Null(hb_set_t)) :
       s (&s_), v (INVALID), l (s->get_population () + 1) { __next__ (); }
 
     typedef hb_codepoint_t __item_type__;
-    const hb_codepoint_t& __item__ () const { return v; }
+    hb_codepoint_t __item__ () const { return v; }
     bool __more__ () const { return v != INVALID; }
     void __next__ () { s->next (&v); if (l) l--; }
     void __prev__ () { s->previous (&v); }
diff --git a/src/test-iter.cc b/src/test-iter.cc
index e0adb393f..c6302b429 100644
--- a/src/test-iter.cc
+++ b/src/test-iter.cc
@@ -33,11 +33,13 @@
 
 
 template <typename T>
-struct array_iter_t : hb_iter_t<array_iter_t<T>, T>, hb_iter_mixin_t<array_iter_t<T>, T>
+struct array_iter_t :
+  hb_iter_t<array_iter_t<T>, T&>,
+  hb_iter_mixin_t<array_iter_t<T>, T&>
 {
   array_iter_t (hb_array_t<T> arr_) : arr (arr_) {}
 
-  typedef T __item_type__;
+  typedef T& __item_type__;
   T& __item_at__ (unsigned i) const { return arr[i]; }
   void __forward__ (unsigned n) { arr += n; }
   void __rewind__ (unsigned n) { arr -= n; }