runtime,analyze,codegen: Hash.new{} default block preserves key type#1815
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the compiler and runtime to support default-block hashes (Hash.new { |h, k| ... }) using a polymorphic key-value hash (PolyPolyHash) instead of a string-keyed hash. This change ensures that key types (such as symbols, strings, and integers) are preserved faithfully on inspection. Feedback on the changes highlights critical issues in lib/sp_runtime.h where the newly added dproc and dproc_self fields in both sp_SymPolyHash and sp_PolyPolyHash are not initialized to NULL in their respective _new functions, which can lead to segmentation faults. Additionally, if dproc_self references a GC-managed object, it must be marked during GC scanning to prevent premature garbage collection.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
A `Hash.new { |h,k| ... }` default-block hash was always compiled to a
`StrPolyHash`, the only poly-valued variant that carried a default-proc
field. String keys were fine, but symbol keys were coerced to their name
string, so `p h` rendered `{"a" => ...}` where CRuby prints `{a: ...}`
(the stored value read back correctly -- purely a key-type/inspect defect).
Give `SymPolyHash` and `PolyPolyHash` a `dproc` field too, mirroring
`StrPolyHash`, and compile a block-default hash as a `PolyPolyHash`. A
default block implies dynamically-populated keys, so the faithful poly
hash -- which boxes each key by value -- is the honest representation: a
symbol key round-trips as a symbol and inspects as `a:`, a string as
`"a"=>`, an integer as `1=>`, and mixed keys each render correctly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ryanseys
force-pushed
the
rs-hash-new-block-key-type
branch
from
1b08731 to
082eef4
Compare
…5% fix) The dproc dispatch #1815 added to sp_SymPolyHash_get / sp_PolyPolyHash_get sat inline on the lookup path; the extra branch/code pushed the hot inlined hash lookup over the inline threshold and cost optcarrot a reproducible 35% fps (818 -> 530, isolated by swapping only lib/sp_runtime.h under the identical generated C). Same lesson as the string-hash cache: a hot-path inline branch regresses; SP_NOINLINE the cold side. Move the whole miss handling (dproc check + default_v) into cold noinline sp_*Hash_miss helpers, leaving _get's body smaller than it originally was. fps recovers fully (~828 interleaved, at-or-above the pre-#1815 baseline); default-block hashes still pass their reproducers; gate green. Refs #1815. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
A
Hash.new { |h,k| ... }default-block hash was always compiled to aStrPolyHash, the only poly-valued hash variant that carried a default-proc field. String keys were fine, but symbol keys were coerced to their name string, sop hrendered{"a" => ...}where CRuby prints{a: ...}— the stored value read back correctly, so this was purely a key-type / inspect defect.This gives
SymPolyHashandPolyPolyHashadprocfield too (mirroringStrPolyHash), and compiles a block-default hash as aPolyPolyHash. A default block implies dynamically-populated keys, so the faithful poly hash — which boxes each key by value — is the honest representation: a symbol key round-trips as a symbol and inspects asa:, a string as"a"=>, an integer as1=>, and mixed keys each render correctly.Covered by
test/hash_new_block_key_type.rb(symbol / string / integer / mixed keys, the accumulator idiom, a method-routed case that defeats constant folding, and default-block value reads), withrubyas the expected-output oracle.