Move tuple/frozenset hash algorithms into common::hash (#8213)

* Move tuple/frozenset hash algorithms into common::hash

Move the pure tuplehash fold (xxHash-based) into `hash::hash_tuple`, taking
a fallible iterator of element hashes, and the frozenset XOR-fold into a
`hash::FrozenSetHash` streaming accumulator. `builtins::tuple` and
`builtins::set` keep only the element-hash loop and delegate the arithmetic.

Assisted-by: Claude

* common/hash: rename acc_pyhash to acc_py_hash

Drop the now-unused `// cspell:ignore pyhash` from tuple.rs left behind
by the tuple hash move; the only remaining occurrence was the
acc_pyhash local. Renaming it lets cspell pass without an ignore.

Assisted-by: Claude

crates/common/src/hash.rs

@@ -198,3 +198,101 @@ pub fn keyed_hash(key: u64, buf: &[u8]) -> u64 {
     buf.hash(&mut hasher);
     hasher.finish()
 }
+
+/// tuplehash: fold the element hashes of a tuple (xxHash-based).
+///
+/// The caller supplies each element's hash lazily; a hash computation may fail,
+/// in which case the error short-circuits the fold.
+pub fn hash_tuple<E>(
+    element_hashes: impl IntoIterator<Item = Result<PyHash, E>>,
+) -> Result<PyHash, E> {
+    const PRIME1: PyUHash = cfg_select! {
+        target_pointer_width = "64" => 11400714785074694791,
+        target_pointer_width = "32" => 2654435761,
+        _ => unreachable!(),
+    };
+
+    const PRIME2: PyUHash = cfg_select! {
+        target_pointer_width = "64" => 14029467366897019727,
+        target_pointer_width = "32" => 2246822519,
+        _ => unreachable!(),
+    };
+
+    const PRIME5: PyUHash = cfg_select! {
+        target_pointer_width = "64" => 2870177450012600261,
+        target_pointer_width = "32" => 374761393,
+        _ => unreachable!(),
+    };
+
+    const ROTATE: u32 = cfg_select! {
+        target_pointer_width = "64" => 31,
+        target_pointer_width = "32" => 13,
+        _ => unreachable!(),
+    };
+
+    let mut acc = PRIME5;
+    let mut len: PyUHash = 0;
+
+    for element_hash in element_hashes {
+        let lane = element_hash? as PyUHash;
+        acc = acc.wrapping_add(lane.wrapping_mul(PRIME2));
+        acc = acc.rotate_left(ROTATE);
+        acc = acc.wrapping_mul(PRIME1);
+        len += 1;
+    }
+
+    acc = acc.wrapping_add(len ^ (PRIME5 ^ 3527539));
+
+    let acc_py_hash = acc as PyHash;
+    if acc_py_hash == -1 {
+        return Ok(1546275796);
+    }
+
+    Ok(acc_py_hash)
+}
+
+/// frozenset_hash: order-independent XOR-fold of a frozenset's element hashes.
+///
+/// The entry hashes are fed in one at a time via [`FrozenSetHash::add`], so the
+/// caller keeps ownership of the iteration (which may hold a lock and compute
+/// each element hash fallibly). The fold is commutative, so element order does
+/// not affect the result.
+pub struct FrozenSetHash {
+    hash: u64,
+}
+
+impl FrozenSetHash {
+    #[must_use]
+    pub fn new(len: usize) -> Self {
+        // Factor in the number of active entries
+        Self {
+            hash: (len as u64 + 1).wrapping_mul(1927868237),
+        }
+    }
+
+    pub fn add(&mut self, element_hash: PyHash) {
+        // Work to increase the bit dispersion for closely spaced hash values.
+        // This is important because some use cases have many combinations of a
+        // small number of elements with nearby hashes so that many distinct
+        // combinations collapse to only a handful of distinct hash values.
+        const fn shuffle_bits(h: u64) -> u64 {
+            ((h ^ 89869747) ^ (h.wrapping_shl(16))).wrapping_mul(3644798167)
+        }
+        // Xor-in shuffled bits from every entry's hash field because xor is
+        // commutative and a frozenset hash should be independent of order.
+        self.hash ^= shuffle_bits(element_hash as u64);
+    }
+
+    #[must_use]
+    pub fn finish(self) -> PyHash {
+        let mut hash = self.hash;
+        // Disperse patterns arising in nested frozen-sets
+        hash ^= (hash >> 11) ^ (hash >> 25);
+        hash = hash.wrapping_mul(69069).wrapping_add(907133923);
+        // -1 is reserved as an error code
+        if hash == u64::MAX {
+            hash = 590923713;
+        }
+        hash as PyHash
+    }
+}

crates/vm/src/builtins/set.rs

@@ -442,28 +442,14 @@ impl PySetInner {
     }
 
     fn hash(&self, vm: &VirtualMachine) -> PyResult<PyHash> {
-        // Work to increase the bit dispersion for closely spaced hash values.
-        // This is important because some use cases have many combinations of a
-        // small number of elements with nearby hashes so that many distinct
-        // combinations collapse to only a handful of distinct hash values.
-        const fn _shuffle_bits(h: u64) -> u64 {
-            ((h ^ 89869747) ^ (h.wrapping_shl(16))).wrapping_mul(3644798167)
-        }
-        // Factor in the number of active entries
-        let mut hash: u64 = (self.len() as u64 + 1).wrapping_mul(1927868237);
-        // Xor-in shuffled bits from every entry's hash field because xor is
-        // commutative and a frozenset hash should be independent of order.
-        hash = self.content.try_fold_keys(hash, |h, element| {
-            Ok(h ^ _shuffle_bits(element.hash(vm)? as u64))
-        })?;
-        // Disperse patterns arising in nested frozen-sets
-        hash ^= (hash >> 11) ^ (hash >> 25);
-        hash = hash.wrapping_mul(69069).wrapping_add(907133923);
-        // -1 is reserved as an error code
-        if hash == u64::MAX {
-            hash = 590923713;
-        }
-        Ok(hash as PyHash)
+        let hasher = self.content.try_fold_keys(
+            hash::FrozenSetHash::new(self.len()),
+            |mut hasher, element| {
+                hasher.add(element.hash(vm)?);
+                Ok(hasher)
+            },
+        )?;
+        Ok(hasher.finish())
     }
 
     // Run operation, on failure, if item is a set/set subclass, convert it

crates/vm/src/builtins/tuple.rs

@@ -1,14 +1,8 @@
-// cspell:ignore pyhash
-
 use super::{
     PositionIterInternal, PyGenericAlias, PyStrRef, PyType, PyTypeRef, iter::builtins_iter,
 };
 use crate::common::lock::LazyLock;
-use crate::common::{
-    hash::{PyHash, PyUHash},
-    lock::PyMutex,
-    wtf8::wtf8_concat,
-};
+use crate::common::{hash, hash::PyHash, lock::PyMutex, wtf8::wtf8_concat};
 use crate::object::{Traverse, TraverseFn};
 use crate::{
     AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
@@ -727,46 +721,5 @@ pub(crate) fn init(context: &'static Context) {
 }
 
 pub(super) fn tuple_hash(elements: &[PyObjectRef], vm: &VirtualMachine) -> PyResult<PyHash> {
-    const PRIME1: PyUHash = cfg_select! {
-        target_pointer_width = "64" => 11400714785074694791,
-        target_pointer_width = "32" => 2654435761,
-        _ => unreachable!(),
-    };
-
-    const PRIME2: PyUHash = cfg_select! {
-        target_pointer_width = "64" => 14029467366897019727,
-        target_pointer_width = "32" => 2246822519,
-        _ => unreachable!(),
-    };
-
-    const PRIME5: PyUHash = cfg_select! {
-        target_pointer_width = "64" => 2870177450012600261,
-        target_pointer_width = "32" => 374761393,
-        _ => unreachable!(),
-    };
-
-    const ROTATE: u32 = cfg_select! {
-        target_pointer_width = "64" => 31,
-        target_pointer_width = "32" => 13,
-        _ => unreachable!(),
-    };
-
-    let mut acc = PRIME5;
-    let len = elements.len() as PyUHash;
-
-    for val in elements {
-        let lane = val.hash(vm)? as PyUHash;
-        acc = acc.wrapping_add(lane.wrapping_mul(PRIME2));
-        acc = acc.rotate_left(ROTATE);
-        acc = acc.wrapping_mul(PRIME1);
-    }
-
-    acc = acc.wrapping_add(len ^ (PRIME5 ^ 3527539));
-
-    let acc_pyhash = acc as PyHash;
-    if acc_pyhash == -1 {
-        return Ok(1546275796);
-    }
-
-    Ok(acc_pyhash)
+    hash::hash_tuple(elements.iter().map(|val| val.hash(vm)))
 }