From 3387f7abbba3cb9d5eddca457612112167786160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 22 May 2024 11:05:21 +0200 Subject: [PATCH 1/2] Fix `Hash#rehash` reset `@first` --- spec/std/hash_spec.cr | 29 +++++++++++++++++++---------- src/hash.cr | 3 +++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/spec/std/hash_spec.cr b/spec/std/hash_spec.cr index d098c8f7bf11..6d1fccd95e3c 100644 --- a/spec/std/hash_spec.cr +++ b/spec/std/hash_spec.cr @@ -1393,16 +1393,25 @@ describe "Hash" do hash.@indices_size_pow2.should eq(12) end - it "rehashes" do - a = [1] - h = {a => 0} - (10..100).each do |i| - h[[i]] = i - end - a << 2 - h[a]?.should be_nil - h.rehash - h[a].should eq(0) + describe "#rehash" do + it "rehashes" do + a = [1] + h = {a => 0} + (10..100).each do |i| + h[[i]] = i + end + a << 2 + h[a]?.should be_nil + h.rehash + h[a].should eq(0) + end + + it "resets @first (#14602)" do + h = {"a" => 1, "b" => 2} + h.delete("a") + h.rehash + h["b"].should eq 2 + end end describe "some edge cases while changing the implementation to open addressing" do diff --git a/src/hash.cr b/src/hash.cr index 24a658dad21d..929e84a1b455 100644 --- a/src/hash.cr +++ b/src/hash.cr @@ -631,6 +631,9 @@ class Hash(K, V) new_entry_index += 1 end + # Reset offset to first non-deleted entry + @first = 0 + # We have to mark entries starting from the final new index # as deleted so the GC can collect them. entries_to_clear = entries_size - new_entry_index From fe4c49d466432fc9b8bc1ed61164046f35cdb84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20M=C3=BCller?= Date: Wed, 22 May 2024 12:30:16 +0200 Subject: [PATCH 2/2] Use string comparison --- spec/std/hash_spec.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/std/hash_spec.cr b/spec/std/hash_spec.cr index 6d1fccd95e3c..684e5af9c0d1 100644 --- a/spec/std/hash_spec.cr +++ b/spec/std/hash_spec.cr @@ -1410,7 +1410,8 @@ describe "Hash" do h = {"a" => 1, "b" => 2} h.delete("a") h.rehash - h["b"].should eq 2 + # We cannot test direct equivalence here because `Hash#==(Hash)` does not depend on `@first` + h.to_s.should eq %({"b" => 2}) end end