From f530276638d297640e894d1231c33ff9d4b8b3f5 Mon Sep 17 00:00:00 2001
From: Gwilym Inzani <gw@ilym.me>
Date: Tue, 9 May 2023 22:38:04 +0100
Subject: [PATCH] Use a concrete type for hash

---
 agb-hashmap/src/lib.rs          | 44 +++++++++++++++++++++++++++++----
 agb-hashmap/src/node.rs         |  2 +-
 agb-hashmap/src/node_storage.rs | 19 ++++----------
 3 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/agb-hashmap/src/lib.rs b/agb-hashmap/src/lib.rs
index 4200ad46..4b4afbab 100644
--- a/agb-hashmap/src/lib.rs
+++ b/agb-hashmap/src/lib.rs
@@ -22,6 +22,7 @@
 #![deny(clippy::missing_panics_doc)]
 #![deny(clippy::doc_markdown)]
 #![deny(clippy::return_self_not_must_use)]
+#![deny(clippy::cast_possible_truncation)]
 
 extern crate alloc;
 
@@ -43,8 +44,6 @@ mod node_storage;
 use node::Node;
 use node_storage::NodeStorage;
 
-type HashType = u32;
-
 // # Robin Hood Hash Tables
 //
 // The problem with regular hash tables where failing to find a slot for a specific
@@ -463,7 +462,7 @@ where
     {
         let mut hasher = self.hasher.build_hasher();
         key.hash(&mut hasher);
-        hasher.finish() as HashType
+        hasher.finish().into()
     }
 }
 
@@ -882,6 +881,40 @@ const fn number_before_resize(capacity: usize) -> usize {
     capacity * 85 / 100
 }
 
+#[derive(Default, Clone, Copy, PartialEq, Eq)]
+pub(crate) struct HashType(u32);
+
+impl From<u64> for HashType {
+    fn from(value: u64) -> Self {
+        // we explicitly want to allow truncation
+        #[allow(clippy::cast_possible_truncation)]
+        Self(value as u32)
+    }
+}
+
+impl From<usize> for HashType {
+    fn from(value: usize) -> Self {
+        // we explicitly want to allow truncation
+        #[allow(clippy::cast_possible_truncation)]
+        Self(value as u32)
+    }
+}
+
+impl HashType {
+    pub(crate) fn fast_mod(self, len: usize) -> usize {
+        debug_assert!(len.is_power_of_two(), "Length must be a power of 2");
+        (self.0 as usize) & (len - 1)
+    }
+}
+
+impl core::ops::Add<i32> for HashType {
+    type Output = HashType;
+
+    fn add(self, rhs: i32) -> Self::Output {
+        Self(self.0.wrapping_add_signed(rhs))
+    }
+}
+
 #[cfg(test)]
 mod test {
     use core::cell::RefCell;
@@ -1034,7 +1067,7 @@ mod test {
 
         for _ in 0..5_000 {
             let command = rng.next_i32().rem_euclid(2);
-            let key = rng.next_i32().rem_euclid(answers.len() as i32);
+            let key = rng.next_i32().rem_euclid(answers.len().try_into().unwrap());
             let value = rng.next_i32();
 
             match command {
@@ -1053,7 +1086,8 @@ mod test {
 
             for (i, answer) in answers.iter().enumerate() {
                 assert_eq!(
-                    map.get(&NoisyDrop::new(i as i32)).map(|nd| &nd.i),
+                    map.get(&NoisyDrop::new(i.try_into().unwrap()))
+                        .map(|nd| &nd.i),
                     answer.as_ref()
                 );
             }
diff --git a/agb-hashmap/src/node.rs b/agb-hashmap/src/node.rs
index 15d54be0..d9101def 100644
--- a/agb-hashmap/src/node.rs
+++ b/agb-hashmap/src/node.rs
@@ -18,7 +18,7 @@ pub(crate) struct Node<K, V> {
 impl<K, V> Node<K, V> {
     fn new() -> Self {
         Self {
-            hash: 0,
+            hash: HashType::default(),
             distance_to_initial_bucket: -1,
             key: MaybeUninit::uninit(),
             value: MaybeUninit::uninit(),
diff --git a/agb-hashmap/src/node_storage.rs b/agb-hashmap/src/node_storage.rs
index 15e78cc0..43bc7d79 100644
--- a/agb-hashmap/src/node_storage.rs
+++ b/agb-hashmap/src/node_storage.rs
@@ -58,10 +58,9 @@ impl<K, V, ALLOCATOR: ClonableAllocator> NodeStorage<K, V, ALLOCATOR> {
         let mut inserted_location = usize::MAX;
 
         loop {
-            let location = fast_mod(
-                self.backing_vec_size(),
-                new_node.hash() + new_node.distance() as HashType,
-            );
+            let location =
+                (new_node.hash() + new_node.distance()).fast_mod(self.backing_vec_size());
+
             let current_node = &mut self.nodes[location];
 
             if current_node.has_value() {
@@ -120,7 +119,7 @@ impl<K, V, ALLOCATOR: ClonableAllocator> NodeStorage<K, V, ALLOCATOR> {
 
         loop {
             let next_location =
-                fast_mod(self.backing_vec_size(), (current_location + 1) as HashType);
+                HashType::from(current_location + 1).fast_mod(self.backing_vec_size());
 
             // if the next node is empty, or the next location has 0 distance to initial bucket then
             // we can clear the current node
@@ -140,10 +139,7 @@ impl<K, V, ALLOCATOR: ClonableAllocator> NodeStorage<K, V, ALLOCATOR> {
         Q: Eq + ?Sized,
     {
         for distance_to_initial_bucket in 0..(self.max_distance_to_initial_bucket + 1) {
-            let location = fast_mod(
-                self.nodes.len(),
-                hash + distance_to_initial_bucket as HashType,
-            );
+            let location = (hash + distance_to_initial_bucket).fast_mod(self.nodes.len());
 
             let node = &self.nodes[location];
             let node_key_ref = node.key_ref()?;
@@ -192,8 +188,3 @@ impl<K, V, ALLOCATOR: ClonableAllocator> NodeStorage<K, V, ALLOCATOR> {
         self.nodes.get_unchecked_mut(at)
     }
 }
-
-const fn fast_mod(len: usize, hash: HashType) -> usize {
-    debug_assert!(len.is_power_of_two(), "Length must be a power of 2");
-    (hash as usize) & (len - 1)
-}