From 19cbc51a462d1aaf98fdcccdfeae0aae3209c98c Mon Sep 17 00:00:00 2001 From: hadashiA Date: Tue, 14 Jul 2026 22:14:18 +0900 Subject: [PATCH] Honor user-defined hash/eql? for Hash keys and add compare_by_identity --- .../ruby/bm_call_hashkey.rb | 18 +++++ .../ruby/bm_call_hashstr.rb | 9 +++ .../ruby/bm_call_hashsym.rb | 9 +++ sig/array.rbs | 4 +- sig/hash.rbs | 12 +++ sig/kernel.rbs | 6 +- src/ChibiRuby/MRubyState.Init.cs | 5 +- src/ChibiRuby/MRubyValueEqualityComparer.cs | 73 ++++++++++++++++++- src/ChibiRuby/RHash.cs | 63 ++++++++++++++-- src/ChibiRuby/StdLib/ArrayMembers.cs | 39 ++++++++++ src/ChibiRuby/StdLib/HashMembers.cs | 29 ++++++++ src/ChibiRuby/StdLib/KernelMembers.cs | 4 + tests/ChibiRuby.Tests/ruby/test/hash.rb | 52 +++++++++++++ 13 files changed, 305 insertions(+), 18 deletions(-) create mode 100644 sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashkey.rb create mode 100644 sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashstr.rb create mode 100644 sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashsym.rb diff --git a/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashkey.rb b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashkey.rb new file mode 100644 index 00000000..f7b1f2fe --- /dev/null +++ b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashkey.rb @@ -0,0 +1,18 @@ +class K + def hash + 42 + end + def eql?(o) + true + end +end +k = K.new +h = { k => 1 } +i = 0 +n = 1_000_000 +x = 0 +while i < n + x = h[k] + i += 1 +end +x diff --git a/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashstr.rb b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashstr.rb new file mode 100644 index 00000000..d1122b31 --- /dev/null +++ b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashstr.rb @@ -0,0 +1,9 @@ +h = { "foo" => 1, "bar" => 2 } +i = 0 +n = 1_000_000 +x = 0 +while i < n + x = h["foo"] + i += 1 +end +x diff --git a/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashsym.rb b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashsym.rb new file mode 100644 index 00000000..45599a41 --- /dev/null +++ b/sandbox/ChibiRuby.Benchmark/ruby/bm_call_hashsym.rb @@ -0,0 +1,9 @@ +h = { foo: 1 } +i = 0 +n = 1_000_000 +x = 0 +while i < n + x = h[:foo] + i += 1 +end +x diff --git a/sig/array.rbs b/sig/array.rbs index abba89f7..57dd69db 100644 --- a/sig/array.rbs +++ b/sig/array.rbs @@ -11,11 +11,13 @@ class Array[Elem] < Object # [1, 2] == [1, 2, 3] # => false def ==: (untyped) -> bool + def eql?: (untyped) -> bool + # Returns true when both arrays have the same length and corresponding elements are eql?. # # [1, 2].eql?([1, 2]) # => true # [1, 2].eql?([1.0, 2.0]) # => false - def eql?: (untyped) -> bool + def hash: () -> Integer # Appends the given element(s) to self and returns self. # diff --git a/sig/hash.rbs b/sig/hash.rbs index 277f08df..c59c19b0 100644 --- a/sig/hash.rbs +++ b/sig/hash.rbs @@ -133,6 +133,18 @@ class Hash[K, V] < Object # h.rehash # => h def rehash: () -> self + # Switches the hash to identity key semantics: keys are compared by object identity instead of hash/eql?, so mutating a key object does not invalidate it. + # + # h = {}.compare_by_identity + # a = [0] + # h[a] = 42 + # a[0] = 1 + # h[a] # => 42 + def compare_by_identity: () -> self + + # Returns whether the hash compares keys by identity. + def compare_by_identity?: () -> bool + # Internal helper used by Hash#delete to remove and return the value for a key. def __delete: (K) -> V? diff --git a/sig/kernel.rbs b/sig/kernel.rbs index 47fe9fd1..dff11b16 100644 --- a/sig/kernel.rbs +++ b/sig/kernel.rbs @@ -88,11 +88,7 @@ module Kernel # 1.frozen? # => true def frozen?: () -> bool - # Returns a hash code for self, suitable for use as a Hash key. - # - # 1.hash # => Integer - # "hi".hash # => Integer - def hash: () -> Integer + def hash: (*untyped) -> untyped # Returns true if the class of self is exactly klass (not a subclass). # diff --git a/src/ChibiRuby/MRubyState.Init.cs b/src/ChibiRuby/MRubyState.Init.cs index 73177d20..d77f60f1 100644 --- a/src/ChibiRuby/MRubyState.Init.cs +++ b/src/ChibiRuby/MRubyState.Init.cs @@ -312,7 +312,7 @@ void InitKernel() DefineMethod(KernelModule, Names.QNil, MRubyMethod.False); DefineMethod(KernelModule, Intern("freeze"u8), KernelMembers.Freeze); DefineMethod(KernelModule, Intern("frozen?"u8), KernelMembers.Frozen); - DefineMethod(KernelModule, Names.Hash, KernelMembers.Hash); + DefineMethod(KernelModule, Names.Hash, KernelMembers.HashFunc); DefineMethod(KernelModule, Intern("instance_of?"u8), KernelMembers.InstanceOf); DefineMethod(KernelModule, Names.QIsA, KernelMembers.KindOf); DefineMethod(KernelModule, Names.QKindOf, KernelMembers.KindOf); @@ -615,6 +615,7 @@ void InitArray() DefineMethod(ArrayClass, Names.OpEq, ArrayMembers.OpEq); DefineMethod(ArrayClass, Names.QEql, ArrayMembers.Eql); + DefineMethod(ArrayClass, Names.Hash, ArrayMembers.Hash); DefineMethod(ArrayClass, Names.OpLShift, ArrayMembers.Push); DefineMethod(ArrayClass, Names.OpAdd, ArrayMembers.OpAdd); DefineMethod(ArrayClass, Names.OpAref, ArrayMembers.OpAref); @@ -685,6 +686,8 @@ void InitHash() DefineMethod(HashClass, Intern("assoc"u8), HashMembers.Assoc); DefineMethod(HashClass, Intern("rassoc"u8), HashMembers.RAssoc); DefineMethod(HashClass, Intern("rehash"u8), HashMembers.Rehash); + DefineMethod(HashClass, Intern("compare_by_identity"u8), HashMembers.CompareByIdentity); + DefineMethod(HashClass, Intern("compare_by_identity?"u8), HashMembers.QCompareByIdentity); DefineMethod(HashClass, Intern("__delete"u8), HashMembers.InternalDelete); DefineMethod(HashClass, Intern("__merge"u8), HashMembers.InternalMerge); DefineMethod(HashClass, Intern("__except"u8), HashMembers.InternalExcept); diff --git a/src/ChibiRuby/MRubyValueEqualityComparer.cs b/src/ChibiRuby/MRubyValueEqualityComparer.cs index 7f14aeb9..a38a116f 100644 --- a/src/ChibiRuby/MRubyValueEqualityComparer.cs +++ b/src/ChibiRuby/MRubyValueEqualityComparer.cs @@ -1,4 +1,7 @@ +using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; +using ChibiRuby.StdLib; namespace ChibiRuby; @@ -15,15 +18,79 @@ public int GetHashCode(MRubyValue value) } } -public sealed class MRubyValueHashKeyEqualityComparer(MRubyState state) : IEqualityComparer +/// +/// Key semantics for Hash, mirroring mruby's ht_hash_value / ht_hash_equal: +/// immediates and strings are handled natively (by value / by content), while other +/// objects defer to the Ruby-visible `hash` / `eql?` methods so user overrides are +/// honored. The default `Kernel#hash` (identity) is detected via the method cache and +/// short-circuited to avoid a full Send per operation on identity-keyed hashes. +/// +/// The same class also implements Hash#compare_by_identity (via ) +/// rather than a separate comparer type, so Dictionary probe sites stay monomorphic for +/// the JIT's guarded devirtualization. +/// +public sealed class MRubyValueHashKeyEqualityComparer(MRubyState state, bool byIdentity = false) + : IEqualityComparer { + /// + /// Identity key semantics (Hash#compare_by_identity): objects compare by reference, + /// immediates by value; user-defined hash/eql? are ignored. Note strings are NOT + /// special-cased — two content-equal string instances are distinct keys, as in CRuby. + /// + public bool ByIdentity => byIdentity; + + // The hot entry points stay tiny so the JIT's guarded devirtualization can inline + // them into Dictionary probes; everything type-dispatched lives in NoInlining tails. + public bool Equals(MRubyValue a, MRubyValue b) { - return a == b || state.Send(a, Names.QEql, b).Truthy; + // Identity / immediate value equality. Remaining immediates + // (Integer/Float/Symbol/nil/true/false) are fully decided here. + if (a == b) return true; + return !byIdentity && a.Object is not null && EqualsSlow(a, b); } public int GetHashCode(MRubyValue value) { - return value.GetHashCode(); + // Immediates hash by value. + if (value.Object is null) return value.GetHashCode(); + return byIdentity + ? RuntimeHelpers.GetHashCode(value.Object) + : GetHashCodeSlow(value); } + + [MethodImpl(MethodImplOptions.NoInlining)] + int GetHashCodeSlow(MRubyValue value) + { + // Strings hash by content (RString overrides GetHashCode). + if (value.Object is RString) + { + return value.GetHashCode(); + } + + // Honor a user-defined `hash` override. When the resolved method is the default + // Kernel#hash (identity), skip the dispatch: bucketing by the CLR identity hash + // is consistent with the identity-based `eql?` default. + if (state.TryFindMethod(state.ClassOf(value), Names.Hash, out var method, out _) && + method == KernelMembers.HashFunc) + { + return value.GetHashCode(); + } + + var hashValue = state.Send(value, Names.Hash); + return hashValue.IsInteger + ? hashValue.IntegerValue.GetHashCode() + : hashValue.GetHashCode(); + } + [MethodImpl(MethodImplOptions.NoInlining)] + bool EqualsSlow(MRubyValue a, MRubyValue b) + { + // Strings compare by content regardless of class, as in mruby's ht_hash_equal. + if (a.Object is RString sa) + { + return b.Object is RString sb && sa.AsSpan().SequenceEqual(sb.AsSpan()); + } + return state.Send(a, Names.QEql, b).Truthy; + } + } diff --git a/src/ChibiRuby/RHash.cs b/src/ChibiRuby/RHash.cs index 256a0640..02283589 100644 --- a/src/ChibiRuby/RHash.cs +++ b/src/ChibiRuby/RHash.cs @@ -17,11 +17,29 @@ public sealed class RHash : RObject, IEnumerable keys; readonly List values; - readonly Dictionary indexTable; + Dictionary indexTable; - readonly IEqualityComparer keyComparer; + IEqualityComparer keyComparer; readonly IEqualityComparer valueComparer; + public bool ComparedByIdentity => keyComparer is MRubyValueHashKeyEqualityComparer { ByIdentity: true }; + + /// + /// Switch key semantics to identity (Hash#compare_by_identity): objects compare by + /// reference, immediates by value, ignoring user-defined hash/eql?. Existing entries + /// are re-bucketed; identity is strictly finer than eql?-equality so no entries merge. + /// + internal void CompareByIdentity(MRubyValueHashKeyEqualityComparer identityComparer) + { + if (ComparedByIdentity) return; + keyComparer = identityComparer; + indexTable = new Dictionary(keys.Count, keyComparer); + for (var i = 0; i < keys.Count; i++) + { + indexTable[keys[i]] = i; + } + } + internal RHash( int capacity, IEqualityComparer keyComparer, @@ -65,7 +83,7 @@ public MRubyValue this[MRubyValue key] } set { - if (indexTable.TryGetValue(key, out var index)) + if (TryGetIndexGuarded(key, out var index)) { keys[index] = key; values[index] = value; @@ -105,7 +123,7 @@ public void Add(MRubyValue key, MRubyValue value) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool ContainsKey(MRubyValue key) => indexTable.TryGetValue(key, out var i); + public bool ContainsKey(MRubyValue key) => TryGetIndexGuarded(key, out _); [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool ContainsValue(MRubyValue value) @@ -215,7 +233,7 @@ public void Rehash() for (var readPos = 0; readPos < keys.Count; readPos++) { var key = keys[readPos]; - if (indexTable.TryGetValue(key, out var existingIndex)) + if (TryGetIndexGuarded(key, out var existingIndex)) { values[existingIndex] = values[readPos]; } @@ -231,8 +249,12 @@ public void Rehash() } } - keys.RemoveRange(writePos, keys.Count - writePos); - values.RemoveRange(writePos, values.Count - writePos); + // A user-defined #hash may have mutated this hash (e.g. cleared it) during the + // probes above; clamp so the trim never underflows. + var keyTrimStart = Math.Min(writePos, keys.Count); + var valueTrimStart = Math.Min(writePos, values.Count); + keys.RemoveRange(keyTrimStart, keys.Count - keyTrimStart); + values.RemoveRange(valueTrimStart, values.Count - valueTrimStart); } public struct Enumerator(RHash source) : IEnumerator> @@ -268,7 +290,7 @@ public void Dispose() [MethodImpl(MethodImplOptions.AggressiveInlining)] bool TryGetIndexOfKey(MRubyValue key, out int index) { - if (indexTable.TryGetValue(key, out index)) + if (TryGetIndexGuarded(key, out index)) { return true; } @@ -276,6 +298,31 @@ bool TryGetIndexOfKey(MRubyValue key, out int index) return false; } + /// + /// indexTable probe that survives a user-defined #hash/#eql? mutating + /// this hash mid-probe (mruby leaves the result unspecified but must not crash). + /// The Dictionary itself stays consistent after such a mutation — only the in-flight + /// probe's guard throws — so a single retry against the new state is sound. + /// + /// + /// indexTable probe that survives a user-defined #hash/#eql? mutating + /// this hash mid-probe (mruby raises "hash modified" there; the result is unspecified + /// but must not crash). The Dictionary itself stays consistent after such a mutation — + /// only the in-flight probe's guard throws — so a single retry against the new state + /// is sound. + /// + bool TryGetIndexGuarded(MRubyValue key, out int index) + { + try + { + return indexTable.TryGetValue(key, out index); + } + catch (InvalidOperationException) + { + return indexTable.TryGetValue(key, out index); + } + } + public Enumerator GetEnumerator() => new(this); IEnumerator> IEnumerable>.GetEnumerator() => diff --git a/src/ChibiRuby/StdLib/ArrayMembers.cs b/src/ChibiRuby/StdLib/ArrayMembers.cs index 6220d531..c9f69519 100644 --- a/src/ChibiRuby/StdLib/ArrayMembers.cs +++ b/src/ChibiRuby/StdLib/ArrayMembers.cs @@ -394,6 +394,45 @@ public static MRubyValue OpEq(MRubyState state, MRubyValue self) /// [1, 2].eql?([1.0, 2.0]) # => false /// /// + /// + /// Returns a content-based hash code; equal (eql?) arrays have equal hashes. + /// + /// + /// + /// [1, 2].hash == [1, 2].hash # => true + /// + /// + [RubyDef("() -> Integer")] + public static MRubyValue Hash(MRubyState state, MRubyValue self) + { + // Same algorithm as Enumerable#hash in lib.rb (12347 seed + __update_hash), + // implemented natively because array keys make Hash probe this per operation. + var span = self.As().AsSpan(); + var h = 12347; + for (var i = 0; i < span.Length; i++) + { + var e = span[i]; + int hv; + if (e.IsInteger) + { + // Matches IntegerMembers.Hash + var n = e.IntegerValue; + hv = unchecked((int)RString.GetHashCode( + System.Runtime.InteropServices.MemoryMarshal.CreateSpan( + ref System.Runtime.CompilerServices.Unsafe.As(ref n), sizeof(long)))); + } + else + { + var hashValue = state.Send(e, Names.Hash); + hv = hashValue.IsInteger + ? unchecked((int)hashValue.IntegerValue) + : hashValue.GetHashCode(); + } + h ^= hv << (i % 16); + } + return new MRubyValue(h); + } + [RubyDef("(untyped) -> bool")] public static MRubyValue Eql(MRubyState state, MRubyValue self) { diff --git a/src/ChibiRuby/StdLib/HashMembers.cs b/src/ChibiRuby/StdLib/HashMembers.cs index bee1f605..712f0652 100644 --- a/src/ChibiRuby/StdLib/HashMembers.cs +++ b/src/ChibiRuby/StdLib/HashMembers.cs @@ -475,6 +475,35 @@ public static MRubyValue Rehash(MRubyState state, MRubyValue self) return self; } + /// + /// Switches the hash to identity key semantics: keys are compared by object identity + /// instead of hash/eql?, so mutating a key object does not invalidate it. + /// + /// + /// + /// h = {}.compare_by_identity + /// a = [0] + /// h[a] = 42 + /// a[0] = 1 + /// h[a] # => 42 + /// + /// + [RubyDef("() -> self")] + public static MRubyValue CompareByIdentity(MRubyState state, MRubyValue self) + { + var h = self.As(); + state.EnsureNotFrozen(h); + h.CompareByIdentity(new MRubyValueHashKeyEqualityComparer(state, byIdentity: true)); + return self; + } + + /// Returns whether the hash compares keys by identity. + [RubyDef("() -> bool")] + public static MRubyValue QCompareByIdentity(MRubyState state, MRubyValue self) + { + return self.As().ComparedByIdentity; + } + /// Internal helper used by Hash#delete to remove and return the value for a key. [RubyDef("(K) -> V?")] public static MRubyValue InternalDelete(MRubyState state, MRubyValue self) diff --git a/src/ChibiRuby/StdLib/KernelMembers.cs b/src/ChibiRuby/StdLib/KernelMembers.cs index ce7f48ba..a6d3b148 100644 --- a/src/ChibiRuby/StdLib/KernelMembers.cs +++ b/src/ChibiRuby/StdLib/KernelMembers.cs @@ -312,6 +312,10 @@ public static MRubyValue Hash(MRubyState state, MRubyValue self) return self.ObjectId; } + // Shared delegate instance for the default `hash`. Registered in Init and compared + // by reference in MRubyValueHashKeyEqualityComparer to detect "hash not overridden". + internal static readonly MRubyFunc HashFunc = Hash; + /// /// Returns a bound Method object for the named method on self. /// diff --git a/tests/ChibiRuby.Tests/ruby/test/hash.rb b/tests/ChibiRuby.Tests/ruby/test/hash.rb index f4098358..b9c8d466 100644 --- a/tests/ChibiRuby.Tests/ruby/test/hash.rb +++ b/tests/ChibiRuby.Tests/ruby/test/hash.rb @@ -234,6 +234,58 @@ def h.default(k); self[k] = 1; end assert_equal(2, h[:foo]) end +assert('Hash#[] with user-defined hash/eql? key') do + # A key that is eql? to the stored key (different instance) must be found: + # the table must consult the Ruby-visible #hash for bucketing. + k1 = HashKey[10] + k2 = HashKey[10] + h = { k1 => :found } + assert_equal(:found, h[k2]) + assert_true(h.key?(k2)) + assert_equal(:found, h.delete(k2)) + assert_equal(0, h.size) + + # Distinct values with colliding #hash stay distinct entries + a = HashKey[2] + b = HashKey[5] # 5 % 3 == 2 % 3 + h = { a => :a, b => :b } + assert_equal(2, h.size) + assert_equal(:a, h[HashKey[2]]) + assert_equal(:b, h[HashKey[5]]) + + # String keys keep content semantics + h = { "foo" => 1 } + assert_equal(1, h["fo" + "o"]) +end + +assert('Hash#compare_by_identity') do + h = {}.compare_by_identity + assert_true(h.compare_by_identity?) + assert_false({}.compare_by_identity?) + + # Mutating a key object must not invalidate it (identity, not content) + a = [0] + h[a] = 42 + a[0] = 1 + assert_equal(42, h[a]) + + # Content-equal but distinct instances are distinct keys + s1 = "key" + s2 = "key" + h = { s1 => 1 }.compare_by_identity + h[s2] = 2 + assert_equal(2, h.size) + assert_equal(1, h[s1]) + assert_equal(2, h[s2]) + + # Immediates still compare by value + h = {}.compare_by_identity + h[100] = :int + h[:sym] = :sym + assert_equal(:int, h[100]) + assert_equal(:sym, h[:sym]) +end + [%w[[]= 3], %w[store 26]].each do |meth, no| assert("Hash##{meth}", "15.2.13.4.#{no}") do [{}, ht_entries.hash_for].each do |h|