Merge train 203: null-typed field GC slot mask, un-imported export shadowing (v0.5.1580) - #10367
Merged
Merged
Conversation
added 6 commits
September 16, 2026 10:57
…10348) A closed-shape object literal lowers to `new __AnonShape_*(…)`, and the synthesized class's field types are not a hint — codegen turns them into the class's compile-time GC masks (`typed_shape::typed_layout_from_fields`), `js_gc_typed_shape_id_for_keys` registers those against a dedicated ShapeId, and every allocation then stamps `SIDE_MASK | TYPED_LAYOUT_INTACT` straight from the baked header image (#8405). That path has no per-object validation and no downgrade, so a field the pointer mask omits is a field the collector never scans: its child is neither marked nor rewritten. `Type::Null` and `Type::Void` are the two declared types `type_is_pointer_bearing` answers `false` for, and the two that a *variable* is most trivially wrong about. Perry infers `var head = null` as `Type::Null` and repairs it only in the post-lowering widening pass, which runs long after the anon-shape class has been minted — and that pass did not cover `Null` / `Void` at all (`_ => false`). So the issue's reproducer var head = null; for (var i = 0; i < 8; i++) head = { id: …, payload: […], tag: null, next: head }; registered `ptr_mask = 0b0011` for a record whose live pointer slots are `{payload, next}`: `next` was excluded, and 3.7% of the object graph was silently truncated and cross-linked (at TOTAL=100000 the walk reported 329,754 nodes for 320,000 ever allocated), then read as a corrupted object once the freed addresses were reused. `PERRY_GC_VERIFY_EVACUATION=1` aborted on it with `parent_space=old_page remembered=no visitor=ObjectFields`. Two edits, both keyed on what is actually provable: * `lower/expr_object.rs` — a record field takes a `Null` / `Void` type only from an expression that IS that value (a literal `null` / `undefined`). Anything else contributes `Any`. `{ next: null }` therefore keeps its exact field type, its mask and its `POINTER_FREE` eligibility. * `lower/type_widening.rs` — the `Null` / `Void` arm the pass was missing, gated on a new `non_nullish` set so a local that only ever holds `null`/`undefined` is not widened for nothing (`object_like` is deliberately entered by those assignments, so widening off it alone would demote every nullable local in the program). The issue's narrowing table understates the blast radius: with the same wrong mask and the `c.tag = …` store dropped, the reproducer prints the CORRECT node count while `PERRY_GC_FROMSPACE_SCAN=1` still reports 15,123 dangling references. Output parity was a false green, which is why the regression test asserts the collector's own whole-heap invariant instead. Validation on perrybuilder (Linux x86_64, v0.5.1579): * reproducer 320000/320000, `truncated_chains=0`, byte-identical to node v26.8.1; `PERRY_GC_FROMSPACE_SCAN=1` clean on every cycle; `PERRY_GC_VERIFY_EVACUATION=1` no longer aborts. Ten shape variants of the reproducer: all match node, all report zero offenders (six of them reported offenders before). * No performance tradeoff, shown structurally rather than by timing: the object files a base and a fixed compiler emit for honest code — a linked list built from `{ value, next: null }`, numeric `{ v: i, w: i + 1 }` record churn, and a mixed pointer/primitive record with arrays and strings — are BYTE-IDENTICAL. Only the miscompiled shape differs. * `cargo test -p perry-hir` 50/50 binaries green, `-p perry-codegen --lib` 1563 green, `cargo fmt --check` clean, no new clippy warnings. Tests, each verified to fail without its half of the fix: * `crates/perry/tests/gc_record_null_typed_field_10348.rs` — 2000 retained chains under `PERRY_GC_FROMSPACE_SCAN_ABORT=1` (~1 s; aborts with exit 134 before the fix). * `anon_shape_field_types.rs` — the minted field types, both directions. * `type_widening.rs` — widen-on-object, widen-on-string, and preserve-on- nullish; the last one fails if the arm is widened off `object_like` alone.
…y_object_like already decides Mirrors the sibling predicate's structural list the other way round, so the new nullish test costs no second infer_expr_type on any RHS that was already decided without one. Behaviour-preserving in the safe direction: the arms that short-circuit to false can only widen MORE, never less.
#10356. When a module imports anything from another native-compiled module, run_pipeline registers every exported class of that module for dispatch -- deliberately, "even when the class name wasn't in the specifier list". The comment argues this is safe because a same-named LOCAL class wins in compile_module. That holds for local classes, but a global intrinsic is not a local class, so nothing outranked the implicit entry. So a module exporting `class Request` made an unrelated `new Request(url, init)` in ANY importer construct that class instead of the global fetch Request -- `.headers` came back undefined. Generated SDKs exporting Request/Response/ Headers are common (hey-api, openapi-typescript, oazapfts); this is OpenCode's TUI bootstrap wall, where packages/sdk/js/src/v2/client.ts imports only OpencodeClient from a gen/sdk.gen.ts that also exports `class Request`. Skip builtin global names in that implicit loop only. An explicit `import { Request } from "./mod.js"` is pushed by the specifier-driven sites above and already wins the name dedup, so it is unaffected -- covered by cell 11 of the test.
This was referenced Sep 16, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe change fixes nullish record-field GC masks, prevents unimported exports from shadowing global intrinsics, adds regression tests and changelog entries, and updates the project version to 0.5.1580. ChangesNullish record-field GC mask
Global intrinsic resolution
Version metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: High ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
This train lands #10352 and #10358 as v0.5.1580. Both sit on
fcd108bfb0, touch no file in common, and cherry-pick clean.Fixes #10348) — a null-typed field is not a proof the GC may skip the slot. A record field whose declared type isnullwas being treated as a proof that the slot can never hold a pointer, so it was masked out of the GC's scan.rhs_certainly_nullishnow answers by shape for the same formsrhs_certainly_object_likealready decides, so the slot stays scanned.Four source commits, each verified to preserve its patch-id and authorship. The train adds two repairs.
Train repairs
#10358 shipped no
changelog.d/fragment — a hard failure of the changeset gate for anycrates/change. Added, keyed to #10358.#10358 fails
cargo fmt --check: the newis_builtin_global_value_namere-export inanalysis.rsis declared before thebuiltins::{…}group rustfmt orders it after. Whitespace only.On the version: #10352 carries its own bump to
0.5.1580, which the contributor rules ask PRs not to do — the maintainer bumps at merge time, precisely so two in-flight PRs cannot claim the same patch version. It collided with an in-flight train that had also taken 1580. Sincemainis at 1579 and this train lands next, 1580 is simply correct, so the bump is kept as authored rather than rewritten, and the other train rebases to 1581 behind it.Validation
Validated head
c0957bc2ee. Five-package release build pinned and hash-verified.gc_record_null_typed_field_10348,issue_10356_unimported_export_shadows_global,anon_shape_field_types.main.record,null,object,shape,global,intrinsic,import,export— about 193 fixture selections, each filter asserting it selected fixtures. The weighting is deliberate: fix(compile): an un-imported export must not shadow a global intrinsic #10358 changes name resolution, so its exposure is every module that resolves a name, not just the fixture it ships.Source CI is the primary gap evidence here and it is already in: #10352's own run completed the full six-shard gap suite with only
main's three known regressions (test_gap_iterator_prototype_next_patch,test_gap_2899_2779_2777_static_helpers,test_gap_disposablestack_2875), alongside 19 passing checks. #10358 passescargo-test,check,warningsande2e-scoped. Both PRs' remaining red checks are the ones every PR inherits: benchmark-evidence freshness, the pre-existingrustlsRUSTSEC-2026-0285 advisory, and thepr-gatefan-in over those.Before merging, the pushed head and unchanged main are checked again. After merging, the rewritten commits are checked for preserved authorship and the main tree must match the validated train exactly.
Summary by CodeRabbit
nullorundefined, preventing retained object references from being lost.Request,Response, andHeaders.