Skip to content

refactor(codegen): migrate the literal-accumulator modules onto the Layer 1 rooting API (#7615) - #7636

Merged
proggeramlug merged 4 commits into
mainfrom
refactor/layer1-slice3
Aug 8, 2026
Merged

refactor(codegen): migrate the literal-accumulator modules onto the Layer 1 rooting API (#7615)#7636
proggeramlug merged 4 commits into
mainfrom
refactor/layer1-slice3

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Slice 3 of the Layer 1 campaign (#7615) — the literal accumulators. Four
modules, 1776 lines, 18 raw sites, 7 hazard sites:
expr/objects_arrays_lit.rs, expr/array_literal.rs, expr/object_literal.rs,
expr/array_push.rs. All four are listed in MIGRATED_MODULES; none names
expr::temp_root. Follows the template (#7617) and slices 1a (#7618),
1b (#7620) and 2 (#7627).

No new combinator

#7627 asked slice 3 to reuse with_rooted_accumulator rather than build a
second one. It does, and so does the one shape that looked like it needed a
fourth:

site combinator
[a, ...b, c]'s growing array (js_array_push_f64 / _hole / js_array_spread_append) with_rooted_accumulator(Repr::Ptr) + advance
[a, b, c]'s element group with_operands_rooted
an object literal's half-built handle, both build paths with_rooted_accumulator(Repr::Ptr)
a this-capturing method closure's deferred value with_rooted_accumulator(Repr::Boxed), nested one per property
arr.push(...src)'s operand pair with_operands_rooted (empty window, emits nothing)

The nesting is the interesting one. A method closure's value is consumed by the
this patch loop below every remaining initializer, so its root has to span
exactly the suffix of the literal that follows it, and the number of such values
is data-dependent. The three with_operands_rooted* forms cannot express that:
all three lower their own operand list up front, which for an object literal
would evaluate every property before storing any of them and reorder observable
side effects. with_rooted_accumulator is "a GC value held while more user
code is lowered", so the shape is one scope per such property, which falls out
as a small recursion over the property list (lower_by_name_props). The list it
returns is innermost-first and re-reversed, and that reverse() is pinned by a
test rather than by a comment.

What that replaces is #6951's flat Vec of raw slots that were never
released
— correct only because temp_root_truncate is a stack cut and the
object handle sat below them, i.e. correct because of an invariant stated in a
comment ("keep rooted_handle_begin ahead of this loop and the release after
the patch loop"). It also leaked the whole group on a ? from a later
initializer. Nested, the release is owned on every path out.

Live bug found, and deliberately not fixed here

arr.push(f()) and arr.push(...g()) evaluate the receiver after the
argument
. Per ES2024 the MemberExpression is evaluated to a Reference first,
so a f that reassigns arr must still push onto the array arr.push resolved
— node prints [9], perry prints [9,2]. Filed as #7634 with the
reproducer and the prescription.

It is not fixed in this slice because that evaluation order is precisely what
makes the arm rooting-free today: the receiver is an Expr::LocalGet, so
reading it after the value's arbitrary user code observes whatever an
evacuating cycle wrote back into the local's alloca / box / module global, and
operand_protection answers Reuse. Restoring spec order puts the receiver in
a window, i.e. every pointer-valued push gains a temp root — expressible with
the existing with_operands_rooted_across and needing no new combinator, but
needing a measured before/after on the pinned mini, which a behaviour-preserving
refactor has no mandate to run. Same rule that produced #7628 last slice.

Scoped honestly

Three of the four modules are translations, not repairs, and the PR claims
them as such:

array_push.rs's ledger line is vacuous on the committed source. It named
no rooting symbol before the migration and names none after, exactly like both
slice-1 modules; only the sabotage arm makes it an assertion. The audit that
earned the listing is written into that file's header: both arms lower value
first / receiver second, and nothing they emit below that point
(js_array_push_f64, js_array_concat, the header probes,
js_gc_note_slot_layout, js_write_barrier_slot, js_array_length) either
holds a pointer across a moving window or fails to consume the one it is handed.

A branch no corpus can reach

lower_object_literal's by-name path with a non-empty property list — the one
carrying the this-patch machinery this PR restructures — is not reachable
from TypeScript
. Since #809 every source-level literal containing a
Prop::Method is lowered to a source-ordered IIFE over {}
(js_object_set_method_by_name). Measured rather than assumed: over the whole
gc_root_dominance_corpus.sh corpus (129 sources, 149 modules) every emitted
js_object_alloc is (i32 0, i32 0).

A branch no corpus reaches is a branch no IR A/B can speak for, so it gets four
unit tests built directly from HIR (by_name_method_closure_tests): the by-name
path is actually selected (js_object_alloc(i32 0, i32 4) + four
js_object_set_field_by_name), both patches run below the last property store,
the two patches are applied in source order — readable off the reserved
this-slot index, because the two closures deliberately reserve different ones
— and three slots are rooted and released innermost-first with the object handle
released last. Sabotage: deleting the patches.reverse() turns the third red
with left: ["1", "0"] right: ["0", "1"].

Verification (local — the CI backlog is deep, so this is the evidence)

IR A/B, both arms built from the same package set in one target dir, the
baseline arm from c1365ed8a's copies of the five touched files:

corpus result
4 probes × 2 environments (default; and PERRY_RS4GC=0 PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_INLINE_SHADOW_SLOT=0) byte-identical
curated dominance corpus — 129 sources, 149 modules, 2452 functions 2450 identical, 2 differ
dependency-scale corpus — 81 zod modules, 64 MB of IR byte-identical

Both differences are the same one instruction pair, in the iterator-result
literal {value, done}:

-  %v645 = load i64, ptr %v9
-  %v646 = bitcast double %v640 to i64
-  invoke void @js_object_set_field(i64 %v645, i32 0, i64 %v646)
+  %v645 = bitcast double %v640 to i64
+  %v646 = load i64, ptr %v9
+  invoke void @js_object_set_field(i64 %v646, i32 0, i64 %v645)

Root-plumbing only, and in the direction the API exists to enforce: the
accumulator's re-read is now fused to the emission that consumes it, so it lands
after the value's bitcast instead of before. Identical opcode multiset, same
operands, same call. Per #7625 the emitted IR is run-to-run deterministic, so
the double-compile control is no longer needed.

The probes cover every lowering family in the four modules — verified by counting
call sites rather than by reading the source: js_object_alloc_with_shape 14,
js_object_set_field 30, js_object_alloc 2, js_gc_init_typed_shape_layout
11, js_array_alloc_literal 4, js_array_from_values 10 (PERRY_FULL_OUTLINE_IC=1),
js_array_spread_append 14, js_array_push_hole 2, js_array_clone_for_spread
2, js_array_concat 3, js_array_push_f64 94. (The first p2 probe read its
literals' fields back and every one was scalar-replaced away; the probe now makes
them escape.)

Gates. gc-root-dominance green in both gated modes on both corpora
with an empty allowlist:

  • curated: 2452 functions / 149 modules / 9846 root stores → 0 violations;
    --seeded-violations 40 at 40/40; --unrooted-allocas 0 over 7867 gc-capable
    allocas;
  • dependency-scale: 12899 functions / 81 modules / 12908 root stores → 0
    violations; 40/40 seeded; --unrooted-allocas 0 over 15242.

All four checker static audits pass (--self-test, --audit-alloc-re,
--audit-poll-capable, --audit-immovable-sources). Root-store counts are
unchanged on both corpora, which is what a byte-identical translation should
read.

Tests. cargo test -p perry-codegen --lib 699 pass (695 + the 4 new);
--doc 3 pass with both compile_fail,E0499 arms still rejecting;
cargo test -p perry-runtime --no-fail-fast 1902 pass, 0 failed, first run;
cargo check --all-targets clean.

Gap A/B, 13 family filters over both prebuilt arms (PERRY_SKIP_BUILD=1,
so the binaries are the ones the IR A/B was measured on): 140 journal verdicts
each, identical sets (diff exit 0) — 126 pass, 1 crash. The crash is
test_gap_gc_same_module_call_argument_rooting, a harness 10 s timeout present
identically on both arms; compiled and run directly the binary finishes in 0.5 s,
so it is the perry-dev profile's margin, not a fault.

Runtime instrument. All four probes byte-identical in stdout and exit code
on both arms, and again under PERRY_GC_ZEAL=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800 PERRY_GC_MOVING_LOOP_POLLS=1. The
instrument was live, not merely enabled: PERRY_GC_DIAG=1 prints 5
[gc-fromspace-protect] retired_set= lines, so copying minors actually ran and
quarantined from-space.

Ledger sabotage, per module — the assert stops at the first offender, so one
run cannot speak for four. A real, compiling temp_root_push_i64 /
temp_root_truncate pair injected into each of the four turns
migrated_modules_do_not_reach_past_the_rooting_api red and names both
planted lines; restored, the ledger is green again. It also caught a genuine
violation in this PR's own new test — an assertion message quoting
temp_root_truncate is a code line, not a comment — which is now reworded.

lint. Every step of the job enumerated from .github/workflows/test.yml
was run (27 of them). All pass except python3 benchmarks/ci_public_baseline_check.py, which is red on pristine origin/main
and has been since 2026-07-29 — #7618, #7620 and #7627 each reported the same
step.

Summary by CodeRabbit

  • Bug Fixes

    • Improved memory safety while creating array and object literals, including spread operations and closures that capture this.
    • Ensured temporary values remain properly available during complex literal construction and are released reliably on success or failure.
  • Tests

    • Added coverage for literal construction paths, closure patching, root-release ordering, unreachable paths, and garbage-collection safety checks.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR migrates array, object, spread-array, and spread-push lowering from manual temporary roots to rooting combinators. Object literals defer and patch this-capturing closures. Tests and the migration ledger cover the updated rooting behavior.

Changes

Literal rooting migration

Layer / File(s) Summary
Array and spread-array rooting
crates/perry-codegen/src/expr/array_literal.rs, crates/perry-codegen/src/expr/objects_arrays_lit.rs
Array elements and spread-array accumulators use rooted operands and accumulators. Existing allocation, append, store, and boxing paths remain in place.
Object literal rooting and closure patching
crates/perry-codegen/src/expr/object_literal.rs
Object construction uses rooted accumulators. this-capturing closures are deferred, patched in source order, and finalized after property stores. IR tests cover path selection and release ordering.
Spread-push migration tracking
crates/perry-codegen/src/expr/array_push.rs, crates/perry-codegen/src/rooting.rs, changelog.d/7636-layer1-slice3-literal-accumulators.md
ArrayPushSpread roots its operands around concatenation and write-back. The migration ledger and changelog record Slice 3 coverage and validation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Codegen
  participant RootingAPI
  participant LiteralLowering
  participant RuntimeStorage
  Codegen->>RootingAPI: lower operands with rooted scope
  RootingAPI->>LiteralLowering: provide rooted values
  LiteralLowering->>RuntimeStorage: allocate, append, or store values
  RuntimeStorage-->>RootingAPI: return updated pointers
  RootingAPI-->>Codegen: complete boxing and cleanup
Loading

Possibly related issues

Possibly related PRs

  • PerryTS/perry#6972 — Earlier temporary-rooting work that this PR extends for array and object literal lowering.
  • PerryTS/perry#7459 — Introduced the rooting API integrated by these expression-lowering paths.
  • PerryTS/perry#7617 — Migrates related expression modules to the rooting-by-construction API and updates migration tracking.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: migrating literal-accumulator codegen modules to the Layer 1 rooting API.
Description check ✅ Passed The description thoroughly covers the changes, related issues, implementation details, and extensive verification results, despite not using the template headings exactly.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/layer1-slice3

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Ralph Küpper added 3 commits August 8, 2026 13:04
…ayer 1 rooting API (#7615)

Slice 3: expr/objects_arrays_lit.rs, expr/array_literal.rs,
expr/object_literal.rs and expr/array_push.rs. No new combinator --
with_operands_rooted and with_rooted_accumulator express every shape,
including the nested per-property closure roots in the object literal.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix
The branch is unreachable from TypeScript since #809 routes every
method-bearing object literal to a source-ordered IIFE, so it gets a
unit test built from HIR instead of relying on an IR A/B that cannot
reach it.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
crates/perry-codegen/src/expr/array_push.rs (1)

723-726: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Destructure the rooted operand slice instead of indexing it.

vals[0] is the spread source and vals[1] is the receiver. The two have the same LLVM type, so a swap compiles and produces a silent miscompile. A slice pattern names them once and fails to compile if the group size changes.

♻️ Proposed refactor
-            rooting::with_operands_rooted(ctx, &[source.as_ref(), &array_expr], |ctx, vals| {
-                let blk = ctx.block();
-                let dst_handle = unbox_to_i64(blk, &vals[1]);
-                let src_handle = unbox_to_i64(blk, &vals[0]);
+            rooting::with_operands_rooted(ctx, &[source.as_ref(), &array_expr], |ctx, vals| {
+                let [src_box, dst_box] = vals else {
+                    unreachable!("with_operands_rooted returns one value per operand")
+                };
+                let blk = ctx.block();
+                let dst_handle = unbox_to_i64(blk, dst_box);
+                let src_handle = unbox_to_i64(blk, src_box);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/array_push.rs` around lines 723 - 726, Update
the rooted operand closure in the array push code to destructure vals as exactly
two named operands, binding the spread source first and the receiver second.
Replace positional indexing in the dst_handle and src_handle assignments with
those bindings, preserving their current semantic mapping and making
operand-count changes fail at compile time.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/src/expr/object_literal.rs`:
- Around line 257-283: Restructure the closure-value handling around
with_rooted_accumulator so the root scope is opened before lower_expr produces v
and remains active through js_object_set_field_by_name. Pass the rooted value
through the field store, then append the resulting closure value to rest only
after the store branch completes, while preserving cleanup on both paths.

---

Nitpick comments:
In `@crates/perry-codegen/src/expr/array_push.rs`:
- Around line 723-726: Update the rooted operand closure in the array push code
to destructure vals as exactly two named operands, binding the spread source
first and the receiver second. Replace positional indexing in the dst_handle and
src_handle assignments with those bindings, preserving their current semantic
mapping and making operand-count changes fail at compile time.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9a2d357-1504-420b-8897-a67838f9a4b7

📥 Commits

Reviewing files that changed from the base of the PR and between 0ff70fe and 8172098.

📒 Files selected for processing (6)
  • changelog.d/7636-layer1-slice3-literal-accumulators.md
  • crates/perry-codegen/src/expr/array_literal.rs
  • crates/perry-codegen/src/expr/array_push.rs
  • crates/perry-codegen/src/expr/object_literal.rs
  • crates/perry-codegen/src/expr/objects_arrays_lit.rs
  • crates/perry-codegen/src/rooting.rs

Comment on lines +257 to +283
let v = lower_expr(ctx, value_expr)?;
let key_raw = emit_interned_key_raw(ctx, &key_handle_global);
obj.call_void(
ctx,
"js_object_set_field_by_name",
&[Arg::Plain(I64, &key_raw), Arg::Plain(DOUBLE, &v)],
);

// The closure value is deferred: the patch loop reads it after every
// remaining property has been lowered, so it must survive their
// allocations AND be re-read afterwards (an evacuating cycle rewrote
// the slot; the register queued above is stale). `build` owns the
// rest of the literal, `finish` is the one place the value escapes,
// and the release happens on both paths out.
let mut rest: Vec<(String, u32)> = Vec::new();
let closure_value = rooting::with_rooted_accumulator(
ctx,
Repr::Boxed,
&v,
protect,
|ctx, _| {
rest = lower_by_name_props(ctx, obj, props, i + 1, protect)?;
Ok(())
},
|_ctx, closure_value| Ok(closure_value.to_string()),
)?;
rest.push((closure_value, this_idx));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Compare the pre-migration ordering of the closure-value root push and the by-name field store.
set -euo pipefail

FILE=crates/perry-codegen/src/expr/object_literal.rs
git log --oneline -n 5 -- "$FILE" || true
BASE=$(git rev-parse HEAD^ 2>/dev/null || echo "")
if [ -n "$BASE" ]; then
  git show "$BASE:$FILE" 2>/dev/null | rg -n -C 6 'temp_root_push_double|js_object_set_field_by_name|rooted_handle' || true
fi

# Confirm the runtime helper can allocate.
fd -t f -g 'field_get_set.rs' crates/perry-runtime | xargs -r rg -n -C 6 'js_object_set_field_by_name'

Repository: PerryTS/perry

Length of output: 5755


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=crates/perry-codegen/src/expr/object_literal.rs

echo "== File size and relevant sections =="
wc -l "$FILE"
sed -n '220,290p' "$FILE" | cat -n
sed -n '320,385p' "$FILE" | cat -n
sed -n '480,540p' "$FILE" | cat -n

echo "== Pre-migration relevant lines =="
BASE=$(git rev-parse HEAD~2 2>/dev/null || git rev-parse HEAD^ 2>/dev/null || true)
if [ -n "$BASE" ]; then
  echo "BASE=$BASE"
  git show "$BASE:$FILE" 2>/dev/null | sed -n '220,300p' | cat -n
  git show "$BASE:$FILE" 2>/dev/null | sed -n '320,385p' | cat -n
else
  echo "BASE unavailable"
fi

echo "== Runtime field_get_set definitions =="
FD_FILE=$(fd -t f -g 'field_get_set.rs' crates/perry-runtime | head -n 1 || true)
if [ -n "$FD_FILE" ]; then
  echo "$FD_FILE"
  wc -l "$FD_FILE"
  rg -n -C 12 'js_object_set_field_by_name|fn object_set_field|keys_array|keys_list' "$FD_FILE" || true
fi

echo "== All call occurrences around object set =="
rg -n -C 8 'js_object_set_field_by_name|object_set_field' crates/perry-runtime crates/perry-codegen/src/expr/object_literal.rs crates/perry-codegen/tests 2>/dev/null || true

echo "== Rooting API definitions in codegen =="
rg -n -C 8 'with_rooted_accumulator|build_root|finish_root|temp_root_push_double|push_root_on_gc_trigger' crates/perry-codegen/src 2>/dev/null || true

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate field_get_set.rs =="
fd -t f field_get_set.rs crates/perry-runtime crates/perry-codegen

FILE=$(fd -t f field_get_set.rs crates/perry-runtime | head -n 1)
if [ -z "$FILE" ]; then
  echo "field_get_set.rs not found"
  exit 0
fi

echo "== Field set definitions in $FILE =="
rg -n -C 18 'js_object_set_field_by_name|fn (object_)?set_field_by_name|keys_array|keys_list|js_array' "$FILE" || true

echo "== git status/diff for object_literal.rs =="
git diff --stat -- crates/perry-codegen/src/expr/object_literal.rs || true
git diff HEAD -- crates/perry-codegen/src/expr/object_literal.rs | sed -n '1,220p' || true

echo "== Rooted accumulator definition/usages =="
rg -n -C 4 'with_rooted_accumulator|Repr::Boxed|rooted_handle_begin|temp_root_push|push_root_on_gc_trigger|gc_collect' crates/perry-codegen/src/perry 2>/dev/null | head -n 240 || true

Repository: PerryTS/perry

Length of output: 1521


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=$(fd -t f field_get_set.rs crates/perry-runtime | head -n 1)
if [ -z "$FILE" ]; then
  echo "field_get_set.rs not found"
  exit 0
fi

echo "== field_get_set.rs size =="
wc -l "$FILE"

echo "== lines 1-260 =="
sed -n '1,260p' "$FILE" | cat -n

echo "== lines 260-420 =="
sed -n '260,420p' "$FILE" | cat -n

echo "== lines 420-620 =="
sed -n '420,620p' "$FILE" | cat -n

echo "== lines 620-760 =="
sed -n '620,760p' "$FILE" | cat -n

Repository: PerryTS/perry

Length of output: 20436


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=$(fd -t f field_get_set.rs crates/perry-runtime | head -n 1)
if [ -z "$FILE" ]; then
  FILE=$(fd -t f field_ops.rs crates/perry-runtime | head -n 1 || true)
fi
echo "FILE=$FILE"

echo "== export/import files =="
fd -t f 'field_ops.rs|mod.rs|object_literal.rs|object_literal.rs' crates/perry-runtime crates/perry-codegen | sed -n '1,80p'

echo "== field_ops.rs =="
FIELD_OPS=$(fd -t f field_ops.rs crates/perry-runtime | head -n 1)
if [ -n "$FIELD_OPS" ]; then
  wc -l "$FIELD_OPS"
  rg -n -C 20 'js_object_set_field_by_name|js_object_alloc|keys_array|js_array' "$FIELD_OPS" || true
fi

echo "== object_literal.rs relevant lines with line numbers =="
OBJ_FILE=crates/perry-codegen/src/expr/object_literal.rs
sed -n '240,285p' "$OBJ_FILE" | nl -ba -v240
sed -n '310,335p' "$OBJ_FILE" | nl -ba -v310
sed -n '355,385p' "$OBJ_FILE" | nl -ba -v355

echo "== runtime public exports relevant =="
MOD_FILE=$(fd -t f mod.rs crates/perry-runtime/src/object | head -n 1)
if [ -n "$MOD_FILE" ]; then
  rg -n -C 6 'js_object_set_field_by_name|pub use|field_ops|get_field_by_name|get_field_by_name_tail|has_property|ic_miss' "$MOD_FILE" || true
fi

Repository: PerryTS/perry

Length of output: 5485


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate js_object_set_field_by_name definition =="
rg -n -C 20 'js_object_set_field_by_name\(' crates/perry-runtime crates/perry-codegen/src/runtime_decls crates/perry-codegen/src/lower_call 2>/dev/null || true

echo "== object_literal.rs relevant snippets =="
OBJ_FILE=crates/perry-codegen/src/expr/object_literal.rs
sed -n '240,285p' "$OBJ_FILE"
echo "---"
sed -n '310,338p' "$OBJ_FILE"
echo "---"
sed -n '356,378p' "$OBJ_FILE"

echo "== field_ops snippet around by-name implementation =="
FIELD_OPS=crates/perry-runtime/src/object/field_get_set/field_ops.rs
rg -n -C 35 'pub extern "C" fn js_object_set_field_by_name|js_object_set_field_by_name' "$FIELD_OPS" "$FILE" 2>/dev/null || true
sed -n '220,305p' "$FIELD_OPS" 2>/dev/null || true

echo "== object module exports =="
MOD_FILE=crates/perry-runtime/src/object/mod.rs
rg -n -C 6 'js_object_set_field_by_name|field_ops|field_get_set|re-export' "$MOD_FILE"

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== exact declaration definitions =="
rg -n 'pub extern "C" fn js_object_set_field_by_name|fn js_object_set_field_by_name|extern "C" fn js_object_set_field_by_name' crates/perry-runtime/src crates/perry-codegen/src -g '*.rs'

echo "== exact declaration only first hit location =="
rg -l 'pub extern "C" fn js_object_set_field_by_name|fn js_object_set_field_by_name|extern "C" fn js_object_set_field_by_name' crates/perry-runtime/src crates/perry-codegen/src -g '*.rs' | head -n 20

echo "== declarations snippets =="
for f in $(rg -l 'pub extern "C" fn js_object_set_field_by_name|fn js_object_set_field_by_name|extern "C" fn js_object_set_field_by_name' crates/perry-runtime/src crates/perry-codegen/src -g '*.rs' | head -n 20); do
  echo "--- $f"
  rg -n -C 20 'pub extern "C" fn js_object_set_field_by_name|extern "C" fn js_object_set_field_by_name|js_object_set_field_by_name' "$f" | sed -n '1,220p'
done

Repository: PerryTS/perry

Length of output: 10025


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== field_set_by_name main slices =="
FILE=crates/perry-runtime/src/object/field_set_by_name.rs
wc -l "$FILE"
sed -n '1,120p' "$FILE"
sed -n '120,260p' "$FILE"
sed -n '260,420p' "$FILE"

echo "== tail allocation/rooting usage =="
rg -n -C 4 'RuntimeHandleScope::new|root_nanbox_f64|js_gc_collect|gc_collect|alloc|push|Vec|keys_array|set_keys|set_prop|overflow|closure' crates/perry-runtime/src/object/field_set_by_name.rs crates/perry-runtime/src/object/field_set_by_name/tail.rs crates/perry-runtime/src/object/field_set_by_name/write_helpers.rs

Repository: PerryTS/perry

Length of output: 50370


Root the closure value before the by-name field store.

js_object_set_field_by_name can collect after loading root handles, especially on the keys_array append path. If the closure value stored in the accumulator patch is already pinned in the accumulator slot before that call, js_object_set_field_by_name’s allocations can move the object and leave that patched reference stale. Open the accumulator/root scope before lowering v, pass v through the field store, and only append it to the patch list after the store branch.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/object_literal.rs` around lines 257 - 283,
Restructure the closure-value handling around with_rooted_accumulator so the
root scope is opened before lower_expr produces v and remains active through
js_object_set_field_by_name. Pass the rooted value through the field store, then
append the resulting closure value to rest only after the store branch
completes, while preserving cleanup on both paths.

Source: Coding guidelines

@proggeramlug
proggeramlug merged commit e4b2989 into main Aug 8, 2026
@proggeramlug
proggeramlug deleted the refactor/layer1-slice3 branch August 8, 2026 11:16
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Audit before merge — verified, merged as v0.5.1362

#7634 reproduced exactly on my own build:

a.push(f())      perry [9,3]   node [9]
b.push(...g())   perry [8,7]   node [8]

The receiver is evaluated after the argument, so the push lands on the
reassigned array. Filing it rather than fixing it was the right call — the
current order is precisely what makes the arm rooting-free, so the fix puts
every pointer-valued push into a window. That is a measured change, and a
behaviour-preserving refactor has no mandate to run it.

Object/array literal behaviour byte-identical to node including property
evaluation order. Ledger sabotage on array_literal.rs red. Root-dominance both
modes: 129/129, 0 violations, 40/40 seeded, unrooted-allocas 0 over 7,867.
Suites 698/0 codegen, 1,902/0 runtime, --all-targets clean, all six lint
scripts + file-size + fmt clean.

Three things this slice got right that I want on the record

  1. Declining to add a fourth combinator, with a written argument. The
    nested-with_rooted_accumulator choice rests on a real semantic constraint —
    every with_operands_rooted* form lowers its operand list up front, which
    for an object literal would evaluate every property before storing any and
    reorder observable side effects. Refusing an abstraction because it would
    change semantics, and saying why in the module header, is exactly the bar for
    the ~80 slices still ahead.

  2. array_push.rs's ledger line is vacuous on committed source, and you said
    so.
    Its haz: 6 are one false-positive shape (a raw i64 whose only
    follower is an Expr::LocalGet). A ledger entry that cannot fail is the
    thing this campaign exists to avoid; naming it beats quietly banking the
    count.

  3. Both probe-vacuity catches. The first object-literal probe was
    scalar-replaced away entirely — re-cut and call-site counts verified before
    trusting the A/B — and the new by-name-path tests were justified by
    measuring unreachability (across 129 sources every js_object_alloc is
    (i32 0, i32 0)) rather than asserting it, with a reverse()-deletion
    sabotage showing they discriminate. That is the same discipline gc: forcing POINTER_FREE on a pointer-bearing object strands nothing — our zeal/protect instruments do not discriminate the layout-state hazard #7635 now
    asks the layout-state family for.

The ledger catching a violation in your own new test — an assertion message
quoting temp_root_truncate counts as a code line — is a small thing that says
the gate is scanning what it claims to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant