Skip to content

Commit 538683b

Browse files
asteriteTomAFrench
andauthored
fix: a couple of UHashMap fixes (#12457)
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
1 parent fb8c60e commit 538683b

3 files changed

Lines changed: 40 additions & 2 deletions

File tree

noir_stdlib/docs/std/default/trait.Default.html

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

noir_stdlib/docs/std/hash/trait.Hasher.html

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

noir_stdlib/src/collections/umap.nr

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::default::Default;
33
use crate::hash::{BuildHasher, Hash};
44
use crate::option::Option;
55

6-
// An unconstrained hash table with open addressing and quadratic probing.
6+
// An unconstrained hash table with open addressing and linear probing.
77
// Note that "unconstrained" here means that almost all operations on this
88
// map are unconstrained and importantly are not constrained afterward either.
99
// This map is meant to be used in unconstrained or comptime code where this
@@ -84,6 +84,7 @@ impl<K, V, B> UHashMap<K, V, B> {
8484
where
8585
B: BuildHasher,
8686
{
87+
let capacity = if capacity == 0 { 1 } else { capacity };
8788
let mut _table = [].as_vector();
8889
for _ in 0..capacity {
8990
_table = _table.push_back(Slot::default());
@@ -410,7 +411,7 @@ impl<K, V, B> UHashMap<K, V, B> {
410411
/// Probing scheme: linear probing.
411412
/// Each attempt increments the index by one, wrapping around the table.
412413
fn linear_probe(&self, hash: u32, attempt: u32) -> u32 {
413-
(hash + attempt) % self._table.len()
414+
((hash as u64 + attempt as u64) % self._table.len() as u64) as u32
414415
}
415416
}
416417

@@ -464,7 +465,9 @@ where
464465
}
465466

466467
mod test {
468+
use crate::default::Default;
467469
use crate::hash::BuildHasherDefault;
470+
use crate::hash::Hasher;
468471
use crate::hash::poseidon2::Poseidon2Hasher;
469472
use crate::option::Option;
470473
use super::UHashMap;
@@ -548,4 +551,33 @@ mod test {
548551
assert_eq(map.entries(), @[(8, 9)]);
549552
assert_eq(map.keys(), @[8]);
550553
}
554+
555+
#[test]
556+
unconstrained fn test_with_hasher_and_capacity_zero() {
557+
let mut m: UHashMap<Field, Field, BuildHasherDefault<Poseidon2Hasher>> =
558+
UHashMap::with_hasher_and_capacity(BuildHasherDefault::<Poseidon2Hasher> {}, 0);
559+
let _ = m.insert(1, 1); // This used to produce an index out of bounds
560+
}
561+
562+
struct MaxHasher {
563+
_v: Field,
564+
}
565+
impl Default for MaxHasher {
566+
fn default() -> Self {
567+
MaxHasher { _v: 0 }
568+
}
569+
}
570+
impl Hasher for MaxHasher {
571+
fn finish(self) -> Field {
572+
4294967295 // u32::MAX
573+
}
574+
fn write(&mut self, _input: Field) {}
575+
}
576+
577+
#[test]
578+
unconstrained fn test_max_hash() {
579+
let mut m: UHashMap<Field, Field, BuildHasherDefault<MaxHasher>> = UHashMap::default();
580+
let _ = m.insert(1, 1);
581+
let _ = m.insert(2, 2); // This used to produce a math overflow
582+
}
551583
}

0 commit comments

Comments
 (0)