diff --git a/builtin/array_block.mbt b/builtin/array_block.mbt index 652d03ce9..4c817393b 100644 --- a/builtin/array_block.mbt +++ b/builtin/array_block.mbt @@ -140,10 +140,10 @@ test "Array::blit_to/grow_destination" { test "Array::blit_to/grow_destination_directly" { let src = [1, 2, 3, 4] let dst = [0] - Array::blit_to(src, dst, len=2, src_offset=1, dst_offset=1) + src[1:3].blit_to(dst, dst_offset=1) assert_true(dst == [0, 2, 3]) let self = [1, 2, 3] - Array::blit_to(self, self, len=2, dst_offset=3) + self[0:2].blit_to(self, dst_offset=3) assert_true(self == [1, 2, 3, 1, 2]) } @@ -206,7 +206,7 @@ test "panic Array::blit_to/boundary_cases4" { test "panic Array::blit_to/reject_overflowed_source_range" { let src = [1] let dst = [0] - Array::blit_to(src, dst, len=0x7fffffff, src_offset=1) + src[1:1 + 0x7fffffff].blit_to(dst) } ///| diff --git a/immut/hashmap/HAMT.mbt b/immut/hashmap/HAMT.mbt index 248ff8e96..993a5d7e7 100644 --- a/immut/hashmap/HAMT.mbt +++ b/immut/hashmap/HAMT.mbt @@ -369,7 +369,7 @@ pub fn[K, V] HashMap::filter( Branch(children) => match children.filter(go) { None => None - Some(new_children) => Some(Branch(new_children)) + Some(new_children) => Some(collapse_branch(new_children)) } } } @@ -450,6 +450,29 @@ pub fn[K : Eq + Hash, V] HashMap::add( } } +///| +/// Rebuild a branch whose children may have shrunk, restoring the `Node` +/// invariant that a subtree holding a single entry is represented as +/// `Flat` (structural `Eq` relies on shapes being canonical). A lone +/// `Leaf` can only be a terminal collision node whose bucket emptied; its +/// remaining path is fully consumed, so the `Flat` path is rebuilt from +/// the slot index alone. +fn[K, V] collapse_branch( + children : @sparse_array.SparseArray[Node[K, V]], +) -> Node[K, V] { + match children.data { + [Flat(key, value, path)] => + Flat(key, value, path.push(children.elem_info.first_idx())) + [Leaf(key, value, Empty)] => + Flat( + key, + value, + @path.Path::exhausted().push(children.elem_info.first_idx()), + ) + _ => Branch(children) + } +} + ///| /// Remove an element from a map pub fn[K : Eq + Hash, V] HashMap::remove( @@ -499,17 +522,7 @@ fn[K : Eq, V] Node::remove_with_path( (_, None) => children.remove(idx) (_, Some(new_child)) => children.replace(idx, new_child) } - match new_children.data { - [Flat(key1, value1, path1)] => - Some( - Flat( - key1, - value1, - path1.push(new_children.elem_info.first_idx()), - ), - ) - _ => Some(Branch(new_children)) - } + Some(collapse_branch(new_children)) } } } @@ -662,9 +675,7 @@ pub fn[K : Eq, V] HashMap::intersection( (Branch(children1), Branch(children2)) => match children1.intersection(children2, go) { None => None - Some({ data: [Flat(key, value, path)], elem_info }) => - Some(Flat(key, value, path.push(elem_info.first_idx()))) - Some(children) => Some(Branch(children)) + Some(children) => Some(collapse_branch(children)) } (Leaf(key1, value1, bucket1), Leaf(key2, value2, bucket2)) => { let kvs1 = bucket1.add((key1, value1)) @@ -708,9 +719,7 @@ pub fn[K : Eq, V] HashMap::intersection_with( (Branch(children1), Branch(children2)) => match children1.intersection(children2, go) { None => None - Some({ data: [Flat(key, value, path)], elem_info }) => - Some(Flat(key, value, path.push(elem_info.first_idx()))) - Some(children) => Some(Branch(children)) + Some(children) => Some(collapse_branch(children)) } (Leaf(key1, value1, bucket1), Leaf(key2, value2, bucket2)) => { let kvs1 = bucket1.add((key1, value1)) @@ -767,9 +776,7 @@ pub fn[K : Eq, V] HashMap::difference( (Branch(children1), Branch(children2)) => match children1.difference(children2, go) { None => None - Some({ data: [Flat(key, value, path)], elem_info }) => - Some(Flat(key, value, path.push(elem_info.first_idx()))) - Some(children) => Some(Branch(children)) + Some(children) => Some(collapse_branch(children)) } (Leaf(key1, value1, bucket1), Leaf(key2, value2, bucket2)) => { let kvs1 = bucket1.add((key1, value1)) diff --git a/immut/hashmap/canonical_structure_test.mbt b/immut/hashmap/canonical_structure_test.mbt new file mode 100644 index 000000000..be1838cbb --- /dev/null +++ b/immut/hashmap/canonical_structure_test.mbt @@ -0,0 +1,98 @@ +// Copyright 2026 International Digital Economy Academy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// `HashMap` derives structural `Eq`, so every operation must leave the tree +// in the canonical shape a fresh construction of the same content would +// have. These tests pin that contract for the shrinking operations +// (`remove`, `filter`, `difference`): before the fix, they left +// non-canonical remnants — an uncollapsed singleton branch after `filter`, +// and a `Leaf(k, v, Empty)` node (instead of the collapsed `Flat`) after +// removing from a full-hash collision bucket — making content-equal maps +// compare unequal. +// +// Found by the QuickCheck property suite in #3999. + +///| +/// Keys whose 32-bit hash keeps only the low three bits, so keys that agree +/// on `n & 7` collide on the full hash while `Eq` still distinguishes them. +priv struct CollidingKey(Int) derive(Eq) + +///| +impl Hash for CollidingKey with fn hash(self) { + self.0 & 7 +} + +///| +impl Hash for CollidingKey with fn hash_combine(self, hasher) { + hasher.combine_int(self.0 & 7) +} + +///| +test "removing a colliding key restores the canonical structure" { + let original : @hashmap.HashMap[CollidingKey, Int] = @hashmap.singleton( + CollidingKey(0), + 0, + ) + let round_trip = original.add(CollidingKey(24), 0).remove(CollidingKey(24)) + assert_true(round_trip == original) + assert_true(round_trip == @hashmap.singleton(CollidingKey(0), 0)) +} + +///| +test "filter down to one key yields the canonical map" { + // No hash collisions involved: `filter` must collapse the branch that + // shrank to a single entry, exactly like `remove` does. + let original : @hashmap.HashMap[Int, Int] = @hashmap.singleton(0, 0) + assert_true(original.add(1, 1).filter((k, _) => k == 0) == original) +} + +///| +test "filter dropping a colliding key yields the canonical map" { + let original : @hashmap.HashMap[CollidingKey, Int] = @hashmap.singleton( + CollidingKey(0), + 0, + ) + let filtered = original + .add(CollidingKey(24), 1) + .filter((k, _) => k == CollidingKey(0)) + assert_true(filtered == original) +} + +///| +test "difference removing a colliding key yields the canonical map" { + let original : @hashmap.HashMap[CollidingKey, Int] = @hashmap.singleton( + CollidingKey(0), + 0, + ) + let both = original.add(CollidingKey(24), 1) + let only_other : @hashmap.HashMap[CollidingKey, Int] = @hashmap.singleton( + CollidingKey(24), + 1, + ) + assert_true(both.difference(only_other) == original) +} + +///| +test "chained removals from a collision bucket collapse in any order" { + // 0, 8, and 24 all hash to bucket 0, forming a three-entry collision node. + let base : @hashmap.HashMap[CollidingKey, Int] = @hashmap.new() + let all = base + .add(CollidingKey(0), 0) + .add(CollidingKey(8), 1) + .add(CollidingKey(24), 2) + let one_way = all.remove(CollidingKey(8)).remove(CollidingKey(24)) + let other_way = all.remove(CollidingKey(24)).remove(CollidingKey(8)) + assert_true(one_way == @hashmap.singleton(CollidingKey(0), 0)) + assert_true(one_way == other_way) +} diff --git a/immut/hashset/HAMT.mbt b/immut/hashset/HAMT.mbt index be2e76dcd..f2a3467b7 100644 --- a/immut/hashset/HAMT.mbt +++ b/immut/hashset/HAMT.mbt @@ -305,6 +305,24 @@ pub fn[A : Eq + Hash] HashSet::remove(self : HashSet[A], key : A) -> HashSet[A] } } +///| + +///| +/// Rebuild a branch whose children may have shrunk, restoring the `Node` +/// invariant that a subtree holding a single element is represented as +/// `Flat` (structural `Eq` relies on shapes being canonical). A lone +/// `Leaf` can only be a terminal collision node whose bucket emptied; its +/// remaining path is fully consumed, so the `Flat` path is rebuilt from +/// the slot index alone. +fn[A] collapse_branch(children : @sparse_array.SparseArray[Node[A]]) -> Node[A] { + match children.data { + [Flat(key, path)] => Flat(key, path.push(children.elem_info.first_idx())) + [Leaf(key, Empty)] => + Flat(key, @path.Path::exhausted().push(children.elem_info.first_idx())) + _ => Branch(children) + } +} + ///| fn[A : Eq] Node::remove_with_path( self : Node[A], @@ -340,11 +358,7 @@ fn[A : Eq] Node::remove_with_path( (_, None) => children.remove(idx) (_, Some(new_child)) => children.replace(idx, new_child) } - match new_children.data { - [Flat(key1, path1)] => - Some(Flat(key1, path1.push(new_children.elem_info.first_idx()))) - _ => Some(Branch(new_children)) - } + Some(collapse_branch(new_children)) } } } @@ -425,9 +439,7 @@ pub fn[K : Eq] HashSet::intersection( (Branch(children1), Branch(children2)) => match children1.intersection(children2, go) { None => None - Some({ data: [Flat(key, path)], elem_info }) => - Some(Flat(key, path.push(elem_info.first_idx()))) - Some(children) => Some(Branch(children)) + Some(children) => Some(collapse_branch(children)) } (Leaf(key1, bucket1), Leaf(key2, bucket2)) => { let keys1 = bucket1.add(key1) @@ -467,9 +479,7 @@ pub fn[K : Eq] HashSet::difference( (Branch(children1), Branch(children2)) => match children1.difference(children2, go) { None => None - Some({ data: [Flat(key, path)], elem_info }) => - Some(Flat(key, path.push(elem_info.first_idx()))) - Some(children) => Some(Branch(children)) + Some(children) => Some(collapse_branch(children)) } (Leaf(key1, bucket1), Leaf(key2, bucket2)) => { let keys1 = bucket1.add(key1) diff --git a/immut/hashset/canonical_structure_test.mbt b/immut/hashset/canonical_structure_test.mbt new file mode 100644 index 000000000..6217a1b33 --- /dev/null +++ b/immut/hashset/canonical_structure_test.mbt @@ -0,0 +1,50 @@ +// Copyright 2026 International Digital Economy Academy +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// `HashSet` shares the HAMT design (and its structural `Eq`) with +// `immut/hashmap`, so shrinking operations must likewise collapse to the +// canonical shape a fresh construction would have. Before the fix, +// removing an element from a full-hash collision bucket left a +// non-canonical `Leaf(k, Empty)` node, making content-equal sets compare +// unequal; `difference` shares the same path. + +///| +/// Elements whose 32-bit hash keeps only the low three bits, so values +/// agreeing on `n & 7` collide on the full hash. +priv struct CollidingElem(Int) derive(Eq) + +///| +impl Hash for CollidingElem with fn hash(self) { + self.0 & 7 +} + +///| +impl Hash for CollidingElem with fn hash_combine(self, hasher) { + hasher.combine_int(self.0 & 7) +} + +///| +test "removing a colliding element restores the canonical structure" { + let original : @hashset.HashSet[CollidingElem] = HashSet([CollidingElem(0)]) + let round_trip = original.add(CollidingElem(24)).remove(CollidingElem(24)) + assert_true(round_trip == original) +} + +///| +test "difference removing a colliding element yields the canonical set" { + let original : @hashset.HashSet[CollidingElem] = HashSet([CollidingElem(0)]) + let both = original.add(CollidingElem(24)) + let only_other : @hashset.HashSet[CollidingElem] = HashSet([CollidingElem(24)]) + assert_true(both.difference(only_other) == original) +} diff --git a/immut/internal/path/path.mbt b/immut/internal/path/path.mbt index 9fd7bbb8a..50ad108ac 100644 --- a/immut/internal/path/path.mbt +++ b/immut/internal/path/path.mbt @@ -89,3 +89,11 @@ pub fn Path::advance(self : Path, depth : Int) -> Path { let Path(self) = self self >> (SEGMENT_LENGTH * depth) } + +///| +/// The path that remains once every index segment has been consumed: just +/// the head tag bits. This is the remaining path of a terminal collision +/// node, and the starting point for rebuilding a full path with `push`. +pub fn Path::exhausted() -> Path { + Path(HEAD_TAG >> (SEGMENT_LENGTH * SEGMENT_NUM)) +} diff --git a/immut/internal/path/pkg.generated.mbti b/immut/internal/path/pkg.generated.mbti index c878203a4..5833421d7 100644 --- a/immut/internal/path/pkg.generated.mbti +++ b/immut/internal/path/pkg.generated.mbti @@ -10,6 +10,7 @@ pub fn[A : Hash] of(A) -> Path pub struct Path(UInt) derive(Eq) pub fn Path::advance(Self, Int) -> Self pub fn Path::equal(Self, Self) -> Bool +pub fn Path::exhausted() -> Self pub fn Path::idx(Self) -> Int pub fn Path::idx_at(Self, Int) -> Int pub fn Path::is_last(Self) -> Bool diff --git a/strconv/double.mbt b/strconv/double.mbt index 461d5f2a8..fb4998681 100644 --- a/strconv/double.mbt +++ b/strconv/double.mbt @@ -57,7 +57,6 @@ let max_mantissa_fast_path : UInt64 = 2UL << mantissa_explicit_bits /// /// Examples: /// ```mbt check -/// #warnings("-deprecated") /// test { /// inspect(@strconv.parse_double("123"), content="123") /// inspect(@strconv.parse_double("12.34"), content="12.34") diff --git a/strconv/int.mbt b/strconv/int.mbt index d1e5fcd3d..5908bd781 100644 --- a/strconv/int.mbt +++ b/strconv/int.mbt @@ -70,7 +70,6 @@ test { /// These underscores do not affect the value. /// Examples: /// ```mbt check -/// #warnings("-deprecated") /// test { /// inspect(@strconv.parse_int64("123"), content="123") /// inspect(@strconv.parse_int64("0xff", base=0), content="255") diff --git a/strconv/uint.mbt b/strconv/uint.mbt index 713df77de..31825e31f 100644 --- a/strconv/uint.mbt +++ b/strconv/uint.mbt @@ -30,7 +30,6 @@ const UINT64_MAX : UInt64 = 0xffffffffffffffffUL /// These underscores do not affect the value. /// Examples: /// ```mbt check -/// #warnings("-deprecated") /// test { /// inspect(@strconv.parse_uint64("123"), content="123") /// inspect(@strconv.parse_uint64("0xff", base=0), content="255")