fix(codegen): destructured-value copies no longer truncate to i32 (#4766 regression)#4785
Merged
Merged
Conversation
46af5c0 to
111c686
Compare
regression) A value pulled from an array binding pattern (const [k, v] = pair, or the parameter form ([k, v]) => ...) and then copied into another local (const cb = v) read back as -2147483648 (i32::MIN) instead of the original object/string; calling a method on it threw "TypeError: (number).<method> is not a function". This broke every drizzle-orm schema (Object.fromEntries(Object.entries(cols).map(([name, col]) => { col.setName(name); ... }))), crashing apps at startup with "(number).setName is not a function". Regressing commit: #4766 (iterator-protocol array destructuring), which emits a scaffolding local `let __destruct_N = undefined` per binding element. That mutable+undefined shape is one of collect_integer_let_ids's seeds (it targets the clampIdx do-while pattern), so the scaffolding local was optimistically seeded into integer_locals. The forward closure then propagated integer-ness down the init-only copy chain (cbBase = __destruct -> cb = cbBase). The disqualify fixed point only prunes locals via non-int LocalSet writes, so it removed the scaffolding local (writes are undefined/step.value) but never re-validated the forward-propagated copies (their Let-init is their sole definition, no LocalSet). The second hop (const cb = cbBase) ended up both in integer_locals and strictly_i32_bounded_locals, qualifying for an i32 shadow slot; storing the NaN-boxed value did fptosi(NaN) = i32::MIN. Fix: in the disqualify fixed point, also re-validate any candidate Let with no LocalSet write (collect_non_int_init_only_let_ids) -- every const, plus never-reassigned lets such as the mutable bindings the parameter-destructuring path emits -- against its defining init over the current candidate set. When the source local is disqualified the copy is pruned too, cascading through the chain. Locals with real int-producing LocalSet writes (clampIdx's xx) stay governed by those writes, preserving the image_convolution i32 fast paths. Regression tests in collectors/hir_facts.rs cover the immutable and mutable (parameter) destructure-copy chains plus an over-pruning guard. Refs #793.
111c686 to
e3da116
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a regression introduced by #4766 (iterator-protocol array destructuring): a value pulled from an array binding pattern (
const [k, v] = pair, or the parameter form([k, v]) => …) and then copied into another local (const cb = v) read back as-2147483648(i32::MIN) instead of the original object/string. Calling a method on it threwTypeError: (number).<method> is not a function.This broke every drizzle-orm schema:
so any app using drizzle crashed at startup with
(number).setName is not a function.Regression window
cf56a6faf(fix(runtime): @hono/node-server c.req.text()/.json()/.formData() on POST/PUT #4765)08a588eccfix(hir): class destructuring (dstr) + default-parameter parity)Root cause
The new array-destructuring lowering emits a scaffolding local
let __destruct_N = undefined(mutable, initUndefined) per binding element. That mutable+undefinedshape is one ofcollect_integer_let_ids's seeds (it targets the clampIdxlet xx = undefined; do { xx = clampIdx(…) } while(false)pattern), so the scaffolding local was optimistically seeded intointeger_locals.The forward closure (
collect_extra_integer_let_ids) then propagated integer-ness down the init-only copy chain:cbBase = __destruct_N→cb = cbBase.The disqualify fixed point only prunes locals via non-int
LocalSetwrites. It correctly removed the scaffolding local (its writes areundefined/step.value), but the forward-propagated copies have noLocalSetwrite — their definingLet-init is their sole definition — so they were never re-validated and stayed stale-integer. The first copy escaped (its source left the set, so it was no longerstrictly_i32_bounded), but the second hop (const cb = cbBase) was both ininteger_localsandstrictly_i32_bounded_locals, qualifying it for an i32 shadow slot. Storing a NaN-boxed value there didfptosi(NaN) = i32::MIN.This is why direct
obj.idwas always correct (read from the double slot) but the value mis-read as a number only when copied out of a destructure.Fix
In the disqualify fixed point, also re-validate any candidate
Letthat has noLocalSetwrite (collect_non_int_init_only_let_ids) — everyconst, plus never-reassignedlets such as the mutable bindings the parameter-destructuring path emits — against its defining init over the current (shrinking) candidate set. When the init's source local is disqualified, the copy is pruned too, cascading through the chain.Locals with real int-producing
LocalSetwrites (clampIdx'sxx) remain governed by those writes, so the image_convolution i32 fast paths are preserved.Validation
Minimal repro (drizzle-free) before → after:
DRIZZLE OK: columns = id,name.(number).setName is not a functionstartup crash (previously it could not start at all).perry-codegentest suite green.Regression tests in
collectors/hir_facts.rs:destructure_undefined_seed_does_not_leak_into_const_copy_chain(immutable body-letpath)destructure_mutable_param_bindings_do_not_leak_into_copy(mutable parameter path)const_copy_of_live_integer_accumulator_stays_integer(over-pruning guard)Refs #793.