Guarantee distinct Box.box allocations per runtime evaluation (fixes #10171)#10174
Closed
lukewilliamboswell wants to merge 4 commits into
Closed
Guarantee distinct Box.box allocations per runtime evaluation (fixes #10171)#10174lukewilliamboswell wants to merge 4 commits into
lukewilliamboswell wants to merge 4 commits into
Conversation
Fixes the mechanism behind #10171: the checker's hoisted-root compile-time evaluation interned Box.box results as shared static data, so a closed pure expression producing a Box (e.g. the body of `fresh = || Box.box(0)`) became one allocation returned by every runtime call, across all specializations and backends. Hosts receive a box's payload pointer as the value's representation and refcount it, so allocation identity is observable at the host ABI. Add a `varMayContainBoxAllocation` walk consulted by the keep-gate `hoistedRootIsIntrinsicallyKept`: a root whose settled value type may contain a Box stays runtime code. The walk pierces opaque nominal backings (wrapping a box in an opaque type does not hide its identity) and treats function types as conservatively box-bearing, because a stored function constant carries capture VALUES that types cannot see. Boxes remain hoistable as compile-time intermediates when the stored result is box-free, and top-level bindings are unaffected (one binding is one value, so its single shared static allocation is the declared semantics). Test assets: the issue's repro wired into test/fx (new `same_box_u64!` hosted function comparing payload addresses), plus variants covering plain functions, generic associated methods, cross-specialization sharing, closure captures behind opaque nominals, and the top-level sharing semantics; specs assert distinct allocations on both interpreter and dev backends. Ten new checker tests pin selection behavior, including the positive box-intermediate case. The CGlue golden header picks up the new hosted function. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Backstop for the checker prune: `finishConstRoot` now walks the stored const value tree (`constNodeContainsBox`, including closure capture values) before upgrading a hoisted template to `.stored_const`. In Debug builds a box-bearing hoisted root panics — the corpus is the tripwire proving the checker gate is total. In Release builds an expr-shaped root skips the upgrade and stays `.eval_template`, which re-lowers inline at its use site: one evaluation per dynamic execution, preserving Box.box freshness even if a future checker gap let a root through. Binding-shaped roots deliberately keep the stored const — reads go through the binding, so per-read re-evaluation would break local binding sharing. Top-level `.constant` roots are untouched. Verified by temporarily disabling the checker gate: the tripwire fires in both the test harness and a real `roc build`, and with the panic also disabled the Release fallback alone restores distinct allocations for the issue repro. Publication in the eval test harness never carried the checker's selected hoisted roots (the options field silently defaulted to empty), unlike the production drivers — which is why hoisted-const interning was invisible to the whole eval suite. That is now an explicit `HoistedRootPublication` choice: `.omit` remains the harness default because the lir_inline structural tests pin lowering shapes for deliberately closed programs that faithful publication folds away entirely; `.publish` opts in via `parseAndCanonicalizeProgramWithBuiltinPublishingHoistedRoots`, used by a new integration test asserting no published hoisted template ever stores a box-carrying value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `LowLevel.resultIdentityObservable`, true for box_box, box_alloc_zeroed, and box_prepare_update: their results are heap cells whose allocation identity is observable across the host ABI, and the language guarantees a distinct allocation per dynamic execution whose result escapes. No pass may merge two executions, hoist one across its executing scope, or share a result as static data. No merging pass exists today (backends lower box allocation to opaque allocator calls that LLVM cannot CSE) — this property is the written axiom any future CSE/GVN or transparent-allocation lowering must consult. Str/List/Dict/ Set allocations, ptr_alloca, and erased-callable creation are deliberately excluded, with reasons documented on the property. The box_reuse pass header now ties its rewrite's legality to the axiom (it recycles only the consumed, uniquely-owned input box — free-then-malloc, never two live boxes at one address). Also remove hoist_roots.zig's `selection_algorithm_version`: it had zero consumers, and cached artifacts are already segregated by the compiler-version cache directory, so it invalidated nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…authors Box.box's builtin doc now states the guarantee: each runtime call allocates a fresh heap cell, separate calls never share an allocation, and platforms may rely on a live box's address as a stable identity — with top-level constants as the one deliberate exception (one binding, one program-lifetime allocation with a pinned static refcount). glue/README.md gains the host-ABI side of the contract: live RocBox addresses are stable identities hosts may key on; Str and List addresses carry no such guarantee and may be shared or interned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
Richard suggested an alternative approach so this change is likely no longer required. |
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.
Fixes #10171.
Implements Position B from the issue discussion:
Boxis a heap cell whose address is its identity — each runtime evaluation ofBox.boxwhose result escapes yields a distinct allocation. Roc is strict, so evaluation count is operationally defined and the guarantee is crisp. This lands the guarantee as a durable axiom rather than a spot fix.Root cause
The checker's hoisted-roots mechanism compile-time-evaluates closed, pure, effect-free expressions in function bodies and rematerializes them as shared static data.
Box.boxresults passed every gate, so a zero-arg helperfresh = || Box.box(0)compiled to "return one static box" — every runtime call, across all specializations and all backends (dev/LLVM/interpreter), returned one allocation. Hosts receive a box's payload pointer as the value's representation and refcount it (roc-signals keys graph tokens on it), so allocation identity is observable at the host ABI. Full mechanism trace with permalinks: #10171 (comment)Design — four layers, one per commit
varMayContainBoxAllocationinCheck.zig, consulted byhoistedRootIsIntrinsicallyKept(the single prune choke point every kept root passes). A root whose settled value type may contain aBoxstays runtime code. The walk pierces opaque nominal backings (wrapping a box in an opaque type does not hide its identity — the roc-signalsNode.Tokenshape) and treats function types as conservatively box-bearing, because stored function constants carry capture values that types cannot see. Boxes remain hoistable as compile-time intermediates when the stored result is box-free; top-level bindings are untouched (one binding = one value; its single shared static allocation is the declared semantics).finishConstRootwalks the stored value tree (constNodeContainsBox, including closure captures) before upgrading a hoisted template tostored_const. Debug builds panic — the test corpus is the tripwire proving the checker gate is total. Release builds skip the upgrade for expr-shaped roots (falls back to the existing.eval_templateper-use inline re-evaluation, which preserves freshness); binding-shaped roots keep the stored const because per-read re-evaluation would break local binding sharing.LowLevel.resultIdentityObservable(true forbox_box,box_alloc_zeroed,box_prepare_update) documents the rule for any future CSE/GVN or transparent-allocation lowering; no merging pass exists today.box_reuse.zigdocuments why its dead-box recycling is legal under the axiom. Deadselection_algorithm_versionremoved.Box.box's builtin doc states the guarantee;glue/README.mdstates the host-ABI contract (liveRocBoxaddresses are stable identities;Str/Listaddresses carry no such guarantee).Behavior after this PR
fresh()called twice (any context, incl. generic methods, across specializations)Box.box(0)per call site executionVerification
test/fxwith a newsame_box_u64!hosted function; specs assert the table above on both--opt=interpreterand--opt=dev.roc build; with the panic also disabled, the Release fallback alone restores distinct allocations. (Both temporary edits reverted; this is recorded here in lieu of an un-writable test for an intentionally unreachable branch.)run-test-zig3532/3532 · lir-inline 119/119 ·run-test-cli692 run / 0 failed (238 platform specs) ·run-test-eval1588/1588 (interpreter/dev/wasm) · snapshots zero diffs · fx coverage and Builtin format checks green.Notes for reviewers
HoistedRootPublicationchoice intest_helpers.zig:.omitdefault preserves the 13 lir-inline structural tests whose deliberately-closed programs would fold away entirely under faithful publication;.publishopts in and is used by a new integration test asserting no published hoisted template stores a box. Flipping the default (and rewriting those 13 closed programs to take runtime inputs) is a candidate follow-up.test/glue/fx_platform_cglue_expected.h) is regenerated because the fx platform gained thesame_box_u64!hosted function.fresh()calls are only protected from this compiler's merging by the layered guards plus the written axiom; the durable design guidance for identity tokens (documented in the issue thread) remains an effectful constructor or host-minted handles if the language ever wants to relax the axiom.🤖 Generated with Claude Code