Merge pull request #3626 from harfbuzz/fix-map

Fix map

src/hb-algs.hh

@@ -227,14 +227,6 @@ struct
 }
 HB_FUNCOBJ (hb_bool);
 
-template <typename T>
-static inline
-constexpr T hb_coerce (const T v) { return v; }
-template <typename T, typename V,
-	  hb_enable_if (!hb_is_same (hb_decay<T>, hb_decay<V>) && std::is_pointer<V>::value)>
-static inline
-constexpr T hb_coerce (const V v) { return *v; }
-
 struct
 {
   private:
@@ -247,6 +239,11 @@ struct
   {
     return v.get () ? v.get ()->hash () : 0;
   }
+  template <typename T> constexpr uint32_t
+  impl (const hb::unique_ptr<T>& v, hb_priority<1>) const
+  {
+    return v.get () ? v.get ()->hash () : 0;
+  }
 
   template <typename T> constexpr auto
   impl (const T& v, hb_priority<0>) const HB_RETURN (uint32_t, std::hash<hb_decay<decltype (hb_deref (v))>>{} (hb_deref (v)))

src/hb-array.hh

@@ -56,7 +56,6 @@ struct hb_array_t : hb_iter_with_fallback_t<hb_array_t<Type>, Type&>
   hb_array_t& operator= (const hb_array_t&) = default;
   hb_array_t& operator= (hb_array_t&&) = default;
 
-  constexpr hb_array_t (std::nullptr_t) : hb_array_t () {}
   constexpr hb_array_t (Type *array_, unsigned int length_) : arrayZ (array_), length (length_) {}
   template <unsigned int length_>
   constexpr hb_array_t (Type (&array_)[length_]) : hb_array_t (array_, length_) {}
@@ -314,7 +313,6 @@ struct hb_sorted_array_t :
   hb_sorted_array_t& operator= (const hb_sorted_array_t&) = default;
   hb_sorted_array_t& operator= (hb_sorted_array_t&&) = default;
 
-  constexpr hb_sorted_array_t (std::nullptr_t) : hb_sorted_array_t () {}
   constexpr hb_sorted_array_t (Type *array_, unsigned int length_) : hb_array_t<Type> (array_, length_) {}
   template <unsigned int length_>
   constexpr hb_sorted_array_t (Type (&array_)[length_]) : hb_array_t<Type> (array_) {}

src/hb-bimap.hh

@@ -64,7 +64,8 @@ struct hb_bimap_t
   hb_codepoint_t backward (hb_codepoint_t rhs) const { return back_map.get (rhs); }
 
   hb_codepoint_t operator [] (hb_codepoint_t lhs) const { return get (lhs); }
-  bool has (hb_codepoint_t lhs, hb_codepoint_t *vp = nullptr) const { return forw_map.has (lhs, vp); }
+  bool has (hb_codepoint_t lhs) const { return forw_map.has (lhs); }
+
 
   void del (hb_codepoint_t lhs)
   {

src/hb-cplusplus.hh

@@ -58,8 +58,8 @@ struct shared_ptr
   shared_ptr (const shared_ptr &o) : p (v::reference (o.p)) {}
   shared_ptr (shared_ptr &&o) : p (o.p) { o.p = nullptr; }
   shared_ptr& operator = (const shared_ptr &o) { if (p != o.p) { destroy (); p = o.p; reference (); } return *this; }
-  shared_ptr& operator = (shared_ptr &&o) { destroy (); p = o.p; o.p = nullptr; return *this; }
-  ~shared_ptr () { v::destroy (p); }
+  shared_ptr& operator = (shared_ptr &&o) { v::destroy (p); p = o.p; o.p = nullptr; return *this; }
+  ~shared_ptr () { v::destroy (p); p = nullptr; }
 
   T* get() const { return p; }
 
@@ -89,6 +89,38 @@ struct shared_ptr
 template<typename T> struct is_shared_ptr : std::false_type {};
 template<typename T> struct is_shared_ptr<shared_ptr<T>> : std::true_type {};
 
+template <typename T>
+struct unique_ptr
+{
+  using element_type = T;
+
+  using v = vtable<T>;
+
+  explicit unique_ptr (T *p = nullptr) : p (p) {}
+  unique_ptr (const unique_ptr &o) = delete;
+  unique_ptr (unique_ptr &&o) : p (o.p) { o.p = nullptr; }
+  unique_ptr& operator = (const unique_ptr &o) = delete;
+  unique_ptr& operator = (unique_ptr &&o) { v::destroy (p); p = o.p; o.p = nullptr; return *this; }
+  ~unique_ptr () { v::destroy (p); p = nullptr; }
+
+  T* get() const { return p; }
+  T* release () { T* v = p; p = nullptr; return v; }
+
+  void swap (unique_ptr &o) { std::swap (p, o.p); }
+  friend void swap (unique_ptr &a, unique_ptr &b) { std::swap (a.p, b.p); }
+
+  operator T * () const { return p; }
+  T& operator * () const { return *get (); }
+  T* operator -> () const { return get (); }
+  operator bool () { return p; }
+
+  private:
+  T *p;
+};
+
+template<typename T> struct is_unique_ptr : std::false_type {};
+template<typename T> struct is_unique_ptr<unique_ptr<T>> : std::true_type {};
+
 template <typename T,
 	  T * (*_get_empty) (void),
 	  T * (*_reference) (T *),
@@ -144,6 +176,16 @@ struct std::hash<hb::shared_ptr<T>>
     }
 };
 
+template<typename T>
+struct std::hash<hb::unique_ptr<T>>
+{
+    std::size_t operator()(const hb::unique_ptr<T>& v) const noexcept
+    {
+        std::size_t h = std::hash<decltype (v.get ())>{}(v.get ());
+        return h;
+    }
+};
+
 
 #endif /* __cplusplus */
 

src/hb-map.hh

@@ -35,14 +35,10 @@
  */
 
 template <typename K, typename V,
-	  typename k_invalid_t = K,
-	  typename v_invalid_t = V,
-	  k_invalid_t kINVALID = std::is_pointer<K>::value ? 0 : std::is_signed<K>::value ? hb_int_min (K) : (K) -1,
-	  v_invalid_t vINVALID = std::is_pointer<V>::value ? 0 : std::is_signed<V>::value ? hb_int_min (V) : (V) -1>
+	  bool minus_one = false>
 struct hb_hashmap_t
 {
   hb_hashmap_t ()  { init (); }
-  hb_hashmap_t (std::nullptr_t) : hb_hashmap_t () {}
   ~hb_hashmap_t () { fini (); }
 
   hb_hashmap_t (const hb_hashmap_t& o) : hb_hashmap_t () { resize (population); hb_copy (o, *this); }
@@ -68,38 +64,37 @@ struct hb_hashmap_t
   struct item_t
   {
     K key;
-    uint32_t hash;
+    uint32_t hash : 30;
+    uint32_t is_used_ : 1;
+    uint32_t is_tombstone_ : 1;
     V value;
 
+    bool is_used () const { return is_used_; }
+    void set_used (bool is_used) { is_used_ = is_used; }
+    bool is_tombstone () const { return is_tombstone_; }
+    void set_tombstone (bool is_tombstone) { is_tombstone_ = is_tombstone; }
+    bool is_real () const { return is_used_ && !is_tombstone_; }
+
+    template <bool v = minus_one,
+	      hb_enable_if (v == false)>
+    static const V& default_value () { return Null(V); };
+    template <bool v = minus_one,
+	      hb_enable_if (v == true)>
+    static const V& default_value () { static const V minus_1 = -1; return minus_1; };
+
     void clear ()
     {
       new (std::addressof (key)) K ();
-      key = hb_coerce<K> (kINVALID);
       new (std::addressof (value)) V ();
-      value = hb_coerce<V> (vINVALID);
       hash = 0;
+      is_used_ = false;
+      is_tombstone_ = false;
     }
 
     bool operator == (const K &o) { return hb_deref (key) == hb_deref (o); }
     bool operator == (const item_t &o) { return *this == o.key; }
-    bool is_unused () const
-    {
-      const K inv = hb_coerce<K> (kINVALID);
-      return key == inv;
-    }
-    bool is_tombstone () const
-    {
-      const K kinv = hb_coerce<K> (kINVALID);
-      const V vinv = hb_coerce<V> (vINVALID);
-      return key != kinv && value == vinv;
-    }
-    bool is_real () const
-    {
-      const K kinv = hb_coerce<K> (kINVALID);
-      const V vinv = hb_coerce<V> (vINVALID);
-      return key != kinv && value != vinv;
-    }
     hb_pair_t<K, V> get_pair() const { return hb_pair_t<K, V> (key, value); }
+    hb_pair_t<const K &, const V &> get_pair_ref() const { return hb_pair_t<const K &, const V &> (key, value); }
 
     uint32_t total_hash () const
     { return (hash * 31) + hb_hash (value); }
@@ -203,27 +198,39 @@ struct hb_hashmap_t
     return true;
   }
 
-  bool set (K key, const V& value) { return set_with_hash (key, hb_hash (key), value); }
-  bool set (K key, V&& value) { return set_with_hash (key, hb_hash (key), std::move (value)); }
+  template <typename VV>
+  bool set (K key, VV&& value) { return set_with_hash (key, hb_hash (key), std::forward<VV> (value)); }
 
-  V get (K key) const
+  const V& get (K key) const
   {
-    if (unlikely (!items)) return hb_coerce<V> (vINVALID);
+    if (unlikely (!items)) return item_t::default_value ();
     unsigned int i = bucket_for (key);
-    return items[i].is_real () && items[i] == key ? items[i].value : hb_coerce<V> (vINVALID);
+    return items[i].is_real () && items[i] == key ? items[i].value : item_t::default_value ();
   }
 
-  void del (K key) { set (key, hb_coerce<V> (vINVALID)); }
+  void del (K key) { set_with_hash (key, hb_hash (key), item_t::default_value (), true); }
 
   /* Has interface. */
   typedef V value_t;
   value_t operator [] (K k) const { return get (k); }
-  bool has (K k, V *vp = nullptr) const
+  bool has (K key, const V **vp = nullptr) const
   {
-    const V &v = (*this)[k];
-    if (vp) *vp = v;
-    const V vinv = hb_coerce<V> (vINVALID);
-    return v != vinv;
+    if (unlikely (!items))
+    {
+      if (vp) *vp = &item_t::default_value ();
+      return false;
+    }
+    unsigned int i = bucket_for (key);
+    if (items[i].is_real () && items[i] == key)
+    {
+      if (vp) *vp = &items[i].value;
+      return true;
+    }
+    else
+    {
+      if (vp) *vp = &item_t::default_value ();
+      return false;
+    }
   }
   /* Projection. */
   V operator () (K k) const { return get (k); }
@@ -275,6 +282,12 @@ struct hb_hashmap_t
     | hb_filter (&item_t::is_real)
     | hb_map (&item_t::get_pair)
   )
+  auto iter_ref () const HB_AUTO_RETURN
+  (
+    + hb_array (items, mask ? mask + 1 : 0)
+    | hb_filter (&item_t::is_real)
+    | hb_map (&item_t::get_pair_ref)
+  )
   auto keys () const HB_AUTO_RETURN
   (
     + hb_array (items, mask ? mask + 1 : 0)
@@ -297,19 +310,16 @@ struct hb_hashmap_t
   protected:
 
   template <typename VV>
-  bool set_with_hash (K key, uint32_t hash, VV&& value)
+  bool set_with_hash (K key, uint32_t hash, VV&& value, bool is_delete=false)
   {
     if (unlikely (!successful)) return false;
-    const K kinv = hb_coerce<K> (kINVALID);
-    if (unlikely (key == kinv)) return true;
     if (unlikely ((occupancy + occupancy / 2) >= mask && !resize ())) return false;
     unsigned int i = bucket_for_hash (key, hash);
 
-    const V vinv = hb_coerce<V> (vINVALID);
-    if (value == vinv && items[i].key != key)
+    if (is_delete && items[i].key != key)
       return true; /* Trying to delete non-existent key. */
 
-    if (!items[i].is_unused ())
+    if (items[i].is_used ())
     {
       occupancy--;
       if (!items[i].is_tombstone ())
@@ -317,27 +327,30 @@ struct hb_hashmap_t
     }
 
     items[i].key = key;
-    items[i].value = value;
+    items[i].value = std::forward<VV> (value);
     items[i].hash = hash;
+    items[i].set_used (true);
+    items[i].set_tombstone (is_delete);
 
     occupancy++;
-    if (!items[i].is_tombstone ())
+    if (!is_delete)
       population++;
 
     return true;
   }
 
-  unsigned int bucket_for (K key) const
+  unsigned int bucket_for (const K &key) const
   {
     return bucket_for_hash (key, hb_hash (key));
   }
 
-  unsigned int bucket_for_hash (K key, uint32_t hash) const
+  unsigned int bucket_for_hash (const K &key, uint32_t hash) const
   {
+    hash &= 0x3FFFFFFF; // We only store lower 30bit of hash
     unsigned int i = hash % prime;
     unsigned int step = 0;
     unsigned int tombstone = (unsigned) -1;
-    while (!items[i].is_unused ())
+    while (items[i].is_used ())
     {
       if (items[i].hash == hash && items[i] == key)
 	return i;
@@ -406,21 +419,14 @@ struct hb_hashmap_t
 
 struct hb_map_t : hb_hashmap_t<hb_codepoint_t,
 			       hb_codepoint_t,
-			       hb_codepoint_t,
-			       hb_codepoint_t,
-			       HB_MAP_VALUE_INVALID,
-			       HB_MAP_VALUE_INVALID>
+			       true>
 {
   using hashmap = hb_hashmap_t<hb_codepoint_t,
 			       hb_codepoint_t,
-			       hb_codepoint_t,
-			       hb_codepoint_t,
-			       HB_MAP_VALUE_INVALID,
-			       HB_MAP_VALUE_INVALID>;
+			       true>;
 
   ~hb_map_t () = default;
   hb_map_t () : hashmap () {}
-  hb_map_t (std::nullptr_t) : hb_map_t () {}
   hb_map_t (const hb_map_t &o) : hashmap ((hashmap &) o) {}
   hb_map_t (hb_map_t &&o) : hashmap (std::move ((hashmap &) o)) {}
   hb_map_t& operator= (const hb_map_t&) = default;

src/hb-ot-cmap-table.hh

@@ -1476,27 +1476,26 @@ struct SubtableUnicodesCache {
 
  private:
   const void* base;
-  hb_hashmap_t<intptr_t, hb_set_t*> cached_unicodes;
+  hb_hashmap_t<intptr_t, hb::unique_ptr<hb_set_t>> cached_unicodes;
 
  public:
   SubtableUnicodesCache(const void* cmap_base)
       : base(cmap_base), cached_unicodes() {}
-  ~SubtableUnicodesCache()
-  {
-    for (hb_set_t* s : cached_unicodes.values()) {
-      hb_set_destroy (s);
-    }
-  }
 
-  hb_set_t* set_for(const EncodingRecord* record)
+  hb_set_t* set_for (const EncodingRecord* record)
   {
-    if (!cached_unicodes.has ((intptr_t) record)) {
-      hb_set_t* new_set = hb_set_create ();
-      if (!cached_unicodes.set ((intptr_t) record, new_set)) {
-        hb_set_destroy (new_set);
+    if (!cached_unicodes.has ((intptr_t) record))
+    {
+      hb_set_t *s = hb_set_create ();
+      if (unlikely (s->in_error ()))
+	return hb_set_get_empty ();
+	
+      (base+record->subtable).collect_unicodes (s);
+
+      if (unlikely (!cached_unicodes.set ((intptr_t) record, hb::unique_ptr<hb_set_t> {s})))
         return hb_set_get_empty ();
-      }
-      (base+record->subtable).collect_unicodes (cached_unicodes.get ((intptr_t) record));
+
+      return s;
     }
     return cached_unicodes.get ((intptr_t) record);
   }

src/hb-ot-color-cpal-table.hh

@@ -97,10 +97,10 @@ struct CPALV1Tail
       c->push ();
       for (const auto _ : colorLabels)
       {
-	hb_codepoint_t v;
+	const hb_codepoint_t *v;
         if (!color_index_map->has (_, &v)) continue;
         NameID new_color_idx;
-	new_color_idx = v;
+	new_color_idx = *v;
         if (!c->copy<NameID> (new_color_idx))
         {
           c->pop_discard ();

src/hb-ot-layout-common.hh

@@ -102,7 +102,7 @@ static void ClassDef_remap_and_serialize (
 struct hb_prune_langsys_context_t
 {
   hb_prune_langsys_context_t (const void         *table_,
-                              hb_hashmap_t<unsigned, hb_set_t *> *script_langsys_map_,
+                              hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *script_langsys_map_,
                               const hb_map_t     *duplicate_feature_map_,
                               hb_set_t           *new_collected_feature_indexes_)
       :table (table_),
@@ -122,7 +122,7 @@ struct hb_prune_langsys_context_t
 
   public:
   const void *table;
-  hb_hashmap_t<unsigned, hb_set_t *> *script_langsys_map;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *script_langsys_map;
   const hb_map_t     *duplicate_feature_map;
   hb_set_t           *new_feature_indexes;
 
@@ -162,14 +162,14 @@ struct hb_subset_layout_context_t :
   hb_subset_context_t *subset_context;
   const hb_tag_t table_tag;
   const hb_map_t *lookup_index_map;
-  const hb_hashmap_t<unsigned, hb_set_t *> *script_langsys_map;
+  const hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *script_langsys_map;
   const hb_map_t *feature_index_map;
   unsigned cur_script_index;
 
   hb_subset_layout_context_t (hb_subset_context_t *c_,
 			      hb_tag_t tag_,
 			      hb_map_t *lookup_map_,
-			      hb_hashmap_t<unsigned, hb_set_t *> *script_langsys_map_,
+			      hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *script_langsys_map_,
 			      hb_map_t *feature_index_map_) :
 				subset_context (c_),
 				table_tag (tag_),
@@ -659,8 +659,8 @@ struct LangSys
     auto *out = c->serializer->start_embed (*this);
     if (unlikely (!out || !c->serializer->extend_min (out))) return_trace (false);
 
-    unsigned v;
-    out->reqFeatureIndex = l->feature_index_map->has (reqFeatureIndex, &v) ? v : 0xFFFFu;
+    const unsigned *v;
+    out->reqFeatureIndex = l->feature_index_map->has (reqFeatureIndex, &v) ? *v : 0xFFFFu;
 
     if (!l->visitFeatureIndex (featureIndex.len))
       return_trace (false);
@@ -723,12 +723,8 @@ struct Script
 
     if (!c->script_langsys_map->has (script_index))
     {
-      hb_set_t* empty_set = hb_set_create ();
-      if (unlikely (!c->script_langsys_map->set (script_index, empty_set)))
-      {
-	hb_set_destroy (empty_set);
+      if (unlikely (!c->script_langsys_map->set (script_index, hb::unique_ptr<hb_set_t> {hb_set_create ()})))
 	return;
-      }
     }
 
     unsigned langsys_count = get_lang_sys_count ();

src/hb-ot-layout-gsubgpos.hh

@@ -109,14 +109,10 @@ struct hb_closure_context_t :
     {
       done_lookups_glyph_count->set (lookup_index, glyphs->get_population ());
 
-      if (!done_lookups_glyph_set->get (lookup_index))
+      if (!done_lookups_glyph_set->has (lookup_index))
       {
-	hb_set_t* empty_set = hb_set_create ();
-	if (unlikely (!done_lookups_glyph_set->set (lookup_index, empty_set)))
-	{
-	  hb_set_destroy (empty_set);
+	if (unlikely (!done_lookups_glyph_set->set (lookup_index, hb::unique_ptr<hb_set_t> {hb_set_create ()})))
 	  return true;
-	}
       }
 
       hb_set_clear (done_lookups_glyph_set->get (lookup_index));
@@ -171,7 +167,7 @@ struct hb_closure_context_t :
   hb_closure_context_t (hb_face_t *face_,
 			hb_set_t *glyphs_,
 			hb_map_t *done_lookups_glyph_count_,
-			hb_hashmap_t<unsigned, hb_set_t *> *done_lookups_glyph_set_,
+			hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *done_lookups_glyph_set_,
 			unsigned int nesting_level_left_ = HB_MAX_NESTING_LEVEL) :
 			  face (face_),
 			  glyphs (glyphs_),
@@ -195,7 +191,7 @@ struct hb_closure_context_t :
 
   private:
   hb_map_t *done_lookups_glyph_count;
-  hb_hashmap_t<unsigned, hb_set_t *> *done_lookups_glyph_set;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *done_lookups_glyph_set;
   unsigned int lookup_count = 0;
 };
 
@@ -3733,7 +3729,7 @@ struct GSUBGPOS
   }
 
   void prune_langsys (const hb_map_t *duplicate_feature_map,
-                      hb_hashmap_t<unsigned, hb_set_t *> *script_langsys_map,
+                      hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *script_langsys_map,
                       hb_set_t       *new_feature_indexes /* OUT */) const
   {
     hb_prune_langsys_context_t c (this, script_langsys_map, duplicate_feature_map, new_feature_indexes);
@@ -3791,7 +3787,7 @@ struct GSUBGPOS
                                 hb_map_t *duplicate_feature_map /* OUT */) const
   {
     if (feature_indices->is_empty ()) return;
-    hb_hashmap_t<hb_tag_t, hb_set_t *> unique_features;
+    hb_hashmap_t<hb_tag_t, hb::unique_ptr<hb_set_t>> unique_features;
     //find out duplicate features after subset
     for (unsigned i : feature_indices->iter ())
     {
@@ -3799,16 +3795,9 @@ struct GSUBGPOS
       if (t == HB_MAP_VALUE_INVALID) continue;
       if (!unique_features.has (t))
       {
-        hb_set_t* indices = hb_set_create ();
-        if (unlikely (indices == hb_set_get_empty () ||
-                      !unique_features.set (t, indices)))
-        {
-          hb_set_destroy (indices);
-          for (auto _ : unique_features.iter ())
-            hb_set_destroy (_.second);
+        if (unlikely (!unique_features.set (t, hb::unique_ptr<hb_set_t> {hb_set_create ()})))
           return;
-        }
-        if (unique_features.get (t))
+        if (unique_features.has (t))
           unique_features.get (t)->add (i);
         duplicate_feature_map->set (i, i);
         continue;
@@ -3853,9 +3842,6 @@ struct GSUBGPOS
         duplicate_feature_map->set (i, i);
       }
     }
-
-    for (auto _ : unique_features.iter ())
-      hb_set_destroy (_.second);
   }
 
   void prune_features (const hb_map_t *lookup_indices, /* IN */

src/hb-ot-layout.cc

@@ -1501,15 +1501,12 @@ hb_ot_layout_lookup_substitute_closure (hb_face_t    *face,
 					hb_set_t     *glyphs /* OUT */)
 {
   hb_map_t done_lookups_glyph_count;
-  hb_hashmap_t<unsigned, hb_set_t *> done_lookups_glyph_set;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> done_lookups_glyph_set;
   OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set);
 
   const OT::SubstLookup& l = face->table.GSUB->table->get_lookup (lookup_index);
 
   l.closure (&c, lookup_index);
-
-  for (auto _ : done_lookups_glyph_set.iter ())
-    hb_set_destroy (_.second);
 }
 
 /**
@@ -1529,7 +1526,7 @@ hb_ot_layout_lookups_substitute_closure (hb_face_t      *face,
 					 hb_set_t       *glyphs /* OUT */)
 {
   hb_map_t done_lookups_glyph_count;
-  hb_hashmap_t<unsigned, hb_set_t *> done_lookups_glyph_set;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> done_lookups_glyph_set;
   OT::hb_closure_context_t c (face, glyphs, &done_lookups_glyph_count, &done_lookups_glyph_set);
   const GSUB& gsub = *face->table.GSUB->table;
 
@@ -1551,9 +1548,6 @@ hb_ot_layout_lookups_substitute_closure (hb_face_t      *face,
     }
   } while (iteration_count++ <= HB_CLOSURE_MAX_STAGES &&
 	   glyphs_length != glyphs->get_population ());
-
-  for (auto _ : done_lookups_glyph_set.iter ())
-    hb_set_destroy (_.second);
 }
 
 /*

src/hb-ot-post-table-v2subset.hh

@@ -78,16 +78,18 @@ HB_INTERNAL bool postV2Tail::subset (hb_subset_context_t *c) const
 
   post::accelerator_t _post (c->plan->source);
 
-  hb_hashmap_t<hb_bytes_t, unsigned, std::nullptr_t, unsigned, nullptr, (unsigned)-1> glyph_name_to_new_index;
+  hb_hashmap_t<hb_bytes_t, unsigned, true> glyph_name_to_new_index;
   for (hb_codepoint_t new_gid = 0; new_gid < num_glyphs; new_gid++)
   {
     hb_codepoint_t old_gid = reverse_glyph_map.get (new_gid);
     unsigned old_index = glyphNameIndex[old_gid];
 
     unsigned new_index;
+    const unsigned *new_index2;
     if (old_index <= 257) new_index = old_index;
-    else if (!old_new_index_map.has (old_index, &new_index))
+    else if (!old_new_index_map.has (old_index, &new_index2))
     {
+      new_index = *new_index2;
       hb_bytes_t s = _post.find_glyph_name (old_gid);
       new_index = glyph_name_to_new_index.get (s);
       if (new_index == (unsigned)-1)

src/hb-repacker.hh

@@ -430,8 +430,8 @@ struct graph_t
     auto new_subgraph =
         + subgraph.keys ()
         | hb_map([&] (unsigned node_idx) {
-	  unsigned v;
-          if (index_map.has (node_idx, &v)) return v;
+	  const unsigned *v;
+          if (index_map.has (node_idx, &v)) return *v;
           return node_idx;
         })
         ;
@@ -443,11 +443,11 @@ struct graph_t
     unsigned next = HB_SET_VALUE_INVALID;
     while (roots.next (&next))
     {
-      unsigned v;
+      const unsigned *v;
       if (index_map.has (next, &v))
       {
         roots.del (next);
-        roots.add (v);
+        roots.add (*v);
       }
     }
 
@@ -458,10 +458,10 @@ struct graph_t
   {
     for (const auto& link : vertices_[node_idx].obj.all_links ())
     {
-      unsigned v;
+      const unsigned *v;
       if (subgraph.has (link.objidx, &v))
       {
-        subgraph.set (link.objidx, v + 1);
+        subgraph.set (link.objidx, *v + 1);
         continue;
       }
       subgraph.set (link.objidx, 1);
@@ -943,11 +943,11 @@ struct graph_t
     {
       for (auto& link : vertices_[i].obj.all_links_writer ())
       {
-	unsigned v;
+	const unsigned *v;
         if (!id_map.has (link.objidx, &v)) continue;
         if (only_wide && !(link.width == 4 && !link.is_signed)) continue;
 
-        reassign_link (link, i, v);
+        reassign_link (link, i, *v);
       }
     }
   }

src/hb-serialize.hh

@@ -719,9 +719,7 @@ struct hb_serialize_context_t
   hb_vector_t<object_t *> packed;
 
   /* Map view of packed objects. */
-  hb_hashmap_t<const object_t *, objidx_t,
-	       const object_t *, objidx_t,
-	       nullptr, 0> packed_map;
+  hb_hashmap_t<const object_t *, objidx_t> packed_map;
 };
 
 #endif /* HB_SERIALIZE_HH */

src/hb-set.hh

@@ -166,7 +166,6 @@ struct hb_set_t : hb_sparseset_t<hb_bit_set_invertible_t>
 
   ~hb_set_t () = default;
   hb_set_t () : sparseset () {};
-  hb_set_t (std::nullptr_t) : hb_set_t () {};
   hb_set_t (const hb_set_t &o) : sparseset ((sparseset &) o) {};
   hb_set_t (hb_set_t&& o) : sparseset (std::move ((sparseset &) o)) {}
   hb_set_t& operator = (const hb_set_t&) = default;

src/hb-subset-plan.cc

@@ -43,7 +43,7 @@
 using OT::Layout::GSUB::GSUB;
 
 
-typedef hb_hashmap_t<unsigned, hb_set_t *> script_langsys_map;
+typedef hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> script_langsys_map;
 #ifndef HB_NO_SUBSET_CFF
 static inline void
 _add_cff_seac_components (const OT::cff1::accelerator_t &cff,
@@ -630,9 +630,6 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
 
   if (plan->gsub_langsys)
   {
-    for (auto _ : plan->gsub_langsys->iter ())
-      hb_set_destroy (_.second);
-
     hb_object_destroy (plan->gsub_langsys);
     plan->gsub_langsys->fini_shallow ();
     hb_free (plan->gsub_langsys);
@@ -640,9 +637,6 @@ hb_subset_plan_destroy (hb_subset_plan_t *plan)
 
   if (plan->gpos_langsys)
   {
-    for (auto _ : plan->gpos_langsys->iter ())
-      hb_set_destroy (_.second);
-
     hb_object_destroy (plan->gpos_langsys);
     plan->gpos_langsys->fini_shallow ();
     hb_free (plan->gpos_langsys);

src/hb-subset-plan.hh

@@ -87,8 +87,8 @@ struct hb_subset_plan_t
   hb_map_t *gpos_lookups;
 
   //active langsys we'd like to retain
-  hb_hashmap_t<unsigned, hb_set_t *> *gsub_langsys;
-  hb_hashmap_t<unsigned, hb_set_t *> *gpos_langsys;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *gsub_langsys;
+  hb_hashmap_t<unsigned, hb::unique_ptr<hb_set_t>> *gpos_langsys;
 
   //active features after removing redundant langsys and prune_features
   hb_map_t *gsub_features;

src/test-map.cc

@@ -27,11 +27,6 @@
 #include "hb-set.hh"
 #include <string>
 
-static const std::string invalid{"invalid"};
-static const hb_map_t invalid_map{};
-static const hb_set_t invalid_set{};
-static const hb_vector_t<unsigned> invalid_vector{};
-
 int
 main (int argc, char **argv)
 {
@@ -127,19 +122,19 @@ main (int argc, char **argv)
 
   /* Test class key / value types. */
   {
-    hb_hashmap_t<hb_bytes_t, int, std::nullptr_t, int, nullptr, 0> m1;
-    hb_hashmap_t<int, hb_bytes_t, int, std::nullptr_t, 0, nullptr> m2;
-    hb_hashmap_t<hb_bytes_t, hb_bytes_t, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m3;
+    hb_hashmap_t<hb_bytes_t, int> m1;
+    hb_hashmap_t<int, hb_bytes_t> m2;
+    hb_hashmap_t<hb_bytes_t, hb_bytes_t> m3;
     assert (m1.get_population () == 0);
     assert (m2.get_population () == 0);
     assert (m3.get_population () == 0);
   }
 
   {
-    hb_hashmap_t<int, int, int, int, 0, 0> m0;
-    hb_hashmap_t<std::string, int, const std::string*, int, &invalid, 0> m1;
-    hb_hashmap_t<int, std::string, int, const std::string*, 0, &invalid> m2;
-    hb_hashmap_t<std::string, std::string, const std::string*, const std::string*, &invalid, &invalid> m3;
+    hb_hashmap_t<int, int> m0;
+    hb_hashmap_t<std::string, int> m1;
+    hb_hashmap_t<int, std::string> m2;
+    hb_hashmap_t<std::string, std::string> m3;
 
     std::string s;
     for (unsigned i = 1; i < 1000; i++)
@@ -156,82 +151,60 @@ main (int argc, char **argv)
   {
     using pair = hb_pair_t<hb_codepoint_t, hb_codepoint_t>;
 
-    hb_hashmap_t<hb_map_t, hb_map_t, const hb_map_t *, const hb_map_t *, &invalid_map, &invalid_map> m1;
-    hb_hashmap_t<hb_map_t, hb_map_t, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m2;
+    hb_hashmap_t<hb_map_t, hb_map_t> m1;
 
     m1.set (hb_map_t (), hb_map_t {});
-    m2.set (hb_map_t (), hb_map_t {});
-
     m1.set (hb_map_t (), hb_map_t {pair (1u, 2u)});
-    m2.set (hb_map_t (), hb_map_t {pair (1u, 2u)});
-
     m1.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)});
-    m2.set (hb_map_t {pair (1u, 2u)}, hb_map_t {pair (2u, 3u)});
-
-    /* Cannot override empty map. */
-    assert (m1.get (hb_map_t ()) == hb_map_t ());
-    assert (m2.get (hb_map_t ()) == hb_map_t ());
 
+    assert (m1.get (hb_map_t ()) == hb_map_t {pair (1u, 2u)});
     assert (m1.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)});
-    assert (m2.get (hb_map_t {pair (1u, 2u)}) == hb_map_t {pair (2u, 3u)});
   }
 
   /* Test hashing sets. */
   {
-    hb_hashmap_t<hb_set_t, hb_set_t, const hb_set_t *, const hb_set_t *, &invalid_set, &invalid_set> m1;
-    hb_hashmap_t<hb_set_t, hb_set_t, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m2;
+    hb_hashmap_t<hb_set_t, hb_set_t> m1;
 
     m1.set (hb_set_t (), hb_set_t ());
-    m2.set (hb_set_t (), hb_set_t ());
-
     m1.set (hb_set_t (), hb_set_t {1});
-    m2.set (hb_set_t (), hb_set_t {1});
-
     m1.set (hb_set_t {1, 1000}, hb_set_t {2});
-    m2.set (hb_set_t {1, 1000}, hb_set_t {2});
-
-    /* Cannot override empty set. */
-    assert (m1.get (hb_set_t ()) == hb_set_t ());
-    assert (m2.get (hb_set_t ()) == hb_set_t ());
 
+    assert (m1.get (hb_set_t ()) == hb_set_t {1});
     assert (m1.get (hb_set_t {1000, 1}) == hb_set_t {2});
-    assert (m2.get (hb_set_t {1000, 1}) == hb_set_t {2});
   }
 
   /* Test hashing vectors. */
   {
     using vector_t = hb_vector_t<unsigned>;
 
-    hb_hashmap_t<vector_t, vector_t, const vector_t *, const vector_t *, &invalid_vector, &invalid_vector> m1;
-    hb_hashmap_t<vector_t, vector_t, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m2;
+    hb_hashmap_t<vector_t, vector_t> m1;
 
     m1.set (vector_t (), vector_t ());
-    m2.set (vector_t (), vector_t ());
-
     m1.set (vector_t (), vector_t {1});
-    m2.set (vector_t (), vector_t {1});
-
     m1.set (vector_t {1}, vector_t {2});
-    m2.set (vector_t {1}, vector_t {2});
-
-    /* Cannot override empty vector. */
-    assert (m1.get (vector_t ()) == vector_t ());
-    assert (m2.get (vector_t ()) == vector_t ());
 
+    assert (m1.get (vector_t ()) == vector_t {1});
     assert (m1.get (vector_t {1}) == vector_t {2});
-    assert (m2.get (vector_t {1}) == vector_t {2});
   }
 
   /* Test hb::shared_ptr. */
   hb_hash (hb::shared_ptr<hb_set_t> ());
-#if 0
   {
-    hb_hashmap_t<hb::shared_ptr<hb_set_t>, hb::shared_ptr<hb_set_t>, std::nullptr_t, std::nullptr_t, nullptr, nullptr> m;
+    hb_hashmap_t<hb::shared_ptr<hb_set_t>, hb::shared_ptr<hb_set_t>> m;
 
     m.get (hb::shared_ptr<hb_set_t> ());
     m.get (hb::shared_ptr<hb_set_t> (hb_set_get_empty ()));
+    m.iter ();
+  }
+  /* Test hb::unique_ptr. */
+  hb_hash (hb::unique_ptr<hb_set_t> ());
+  {
+    hb_hashmap_t<hb::unique_ptr<hb_set_t>, hb::unique_ptr<hb_set_t>> m;
+
+    m.get (hb::unique_ptr<hb_set_t> ());
+    m.get (hb::unique_ptr<hb_set_t> (hb_set_get_empty ()));
+    m.iter_ref ();
   }
-#endif
 
   return 0;
 }

test/api/test-cplusplus.cc

@@ -106,5 +106,7 @@ main ()
   pb.set_user_data (&key, b, nullptr, true);
   (void) pb.get_user_data (&key);
 
+  hb::unique_ptr<hb_buffer_t> pb5 {pb3.reference ()};
+
   return pb == pb.get_empty () || pb == pb2;
 }