From 889dc1eb06a80ea9be4223a19011e47a52abebdd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Tue, 14 May 2019 22:28:07 -0700 Subject: [PATCH] [iter] Remove sort categorization See comments. --- src/hb-array.hh | 2 +- src/hb-iter.hh | 48 +++++++++++++++++++++++++------------- src/hb-ot-layout-common.hh | 2 +- src/hb-set.hh | 2 +- 4 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/hb-array.hh b/src/hb-array.hh index 6f6fd7fb1..705bc6a46 100644 --- a/src/hb-array.hh +++ b/src/hb-array.hh @@ -226,7 +226,7 @@ struct hb_sorted_array_t : typedef hb_iter_t, Type&> iter_base_t; HB_ITER_USING (iter_base_t); static constexpr bool is_random_access_iterator = true; - static constexpr hb_sortedness_t is_sorted_iterator = hb_sortedness_t::SORTED; + static constexpr bool is_sorted_iterator = true; hb_sorted_array_t () : hb_array_t () {} hb_sorted_array_t (Type *array_, unsigned int length_) : hb_array_t (array_, length_) {} diff --git a/src/hb-iter.hh b/src/hb-iter.hh index 906d54e33..2448e1944 100644 --- a/src/hb-iter.hh +++ b/src/hb-iter.hh @@ -55,13 +55,6 @@ * type of .end()? */ -enum hb_sortedness_t -{ - NOT_SORTED = 0, - SORTED, - STRICTLY_SORTED, -}; - /* * Base classes for iterators. */ @@ -74,7 +67,7 @@ struct hb_iter_t static constexpr unsigned item_size = hb_static_size (Item); static constexpr bool is_iterator = true; static constexpr bool is_random_access_iterator = false; - static constexpr hb_sortedness_t is_sorted_iterator = hb_sortedness_t::NOT_SORTED; + static constexpr bool is_sorted_iterator = false; private: /* https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern */ @@ -367,9 +360,10 @@ struct hb_map_iter_t : typedef decltype (hb_get (hb_declval (Proj), *hb_declval (Iter))) __item_t__; static constexpr bool is_random_access_iterator = Iter::is_random_access_iterator; - static constexpr hb_sortedness_t is_sorted_iterator = - Sorted == hb_function_sortedness_t::SORTED ? hb_sortedness_t::SORTED - : Sorted == hb_function_sortedness_t::RETAINS_SORTING ? Iter::is_sorted_iterator : hb_sortedness_t::NOT_SORTED; + static constexpr bool is_sorted_iterator = + Sorted == hb_function_sortedness_t::SORTED ? true : + Sorted == hb_function_sortedness_t::RETAINS_SORTING ? Iter::is_sorted_iterator : + false; __item_t__ __item__ () const { return hb_get (f.get (), *it); } __item_t__ __item_at__ (unsigned i) const { return hb_get (f.get (), it[i]); } bool __more__ () const { return bool (it); } @@ -436,7 +430,7 @@ struct hb_filter_iter_t : { while (it && !hb_has (p.get (), hb_get (f.get (), *it))) ++it; } typedef typename Iter::item_t __item_t__; - static constexpr hb_sortedness_t is_sorted_iterator = Iter::is_sorted_iterator; + static constexpr bool is_sorted_iterator = Iter::is_sorted_iterator; __item_t__ __item__ () const { return *it; } bool __more__ () const { return bool (it); } void __next__ () { do ++it; while (it && !hb_has (p.get (), hb_get (f.get (), *it))); } @@ -520,9 +514,31 @@ struct hb_zip_iter_t : static constexpr bool is_random_access_iterator = A::is_random_access_iterator && B::is_random_access_iterator; - static constexpr hb_sortedness_t is_sorted_iterator = - A::is_sorted_iterator == hb_sortedness_t::SORTED ? - B::is_sorted_iterator : A::is_sorted_iterator; + /* Note. The following categorization is only valid if A is strictly sorted, + * ie. does NOT have duplicates. Previously I tried to categorize sortedness + * more granularly, see commits: + * + * 513762849a683914fc266a17ddf38f133cccf072 + * 4d3cf2adb669c345cc43832d11689271995e160a + * + * However, that was not enough, since hb_sorted_array_t, hb_sorted_vector_t, + * SortedArrayOf, etc all needed to be updated to add more variants. At that + * point I saw it not worth the effort, and instead we now deem all sorted + * collections as essentially strictly-sorted for the purposes of zip. + * + * The above assumption is not as bad as it sounds. Our "sorted" comes with + * no guarantees. It's just a contract, put in place to help you remember, + * and think about, whether an iterator you receive is expected to be + * sorted or not. As such, it's not perfect by definition, and should not + * be treated so. The inaccuracy here just errs in the direction of being + * more permissive, so your code compiles instead of erring on the side of + * marking your zipped iterator unsorted in which case your code won't + * compile. + * + * This semantical limitation does NOT affect logic in any other place I + * know of as of this writing. + */ + static constexpr bool is_sorted_iterator = A::is_sorted_iterator; __item_t__ __item__ () const { return __item_t__ (*a, *b); } __item_t__ __item_at__ (unsigned i) const { return __item_t__ (a[i], b[i]); } @@ -590,7 +606,7 @@ struct hb_counter_iter_t : typedef T __item_t__; static constexpr bool is_random_access_iterator = true; - static constexpr hb_sortedness_t is_sorted_iterator = hb_sortedness_t::STRICTLY_SORTED; + static constexpr bool is_sorted_iterator = true; __item_t__ __item__ () const { return +v; } __item_t__ __item_at__ (unsigned j) const { return v + j * step; } bool __more__ () const { return v != end_; } diff --git a/src/hb-ot-layout-common.hh b/src/hb-ot-layout-common.hh index 67f2617fe..71c0ca4a9 100644 --- a/src/hb-ot-layout-common.hh +++ b/src/hb-ot-layout-common.hh @@ -1115,7 +1115,7 @@ struct Coverage struct iter_t : hb_iter_with_fallback_t { - static constexpr hb_sortedness_t is_sorted_iterator = hb_sortedness_t::STRICTLY_SORTED; + static constexpr bool is_sorted_iterator = true; iter_t (const Coverage &c_ = Null(Coverage)) { memset (this, 0, sizeof (*this)); diff --git a/src/hb-set.hh b/src/hb-set.hh index d9ffded57..332e07bd0 100644 --- a/src/hb-set.hh +++ b/src/hb-set.hh @@ -691,7 +691,7 @@ struct hb_set_t */ struct iter_t : hb_iter_with_fallback_t { - static constexpr hb_sortedness_t is_sorted_iterator = hb_sortedness_t::STRICTLY_SORTED; + static constexpr bool is_sorted_iterator = true; iter_t (const hb_set_t &s_ = Null(hb_set_t)) : s (&s_), v (INVALID), l (s->get_population () + 1) { __next__ (); }