pure: harden simplify_let_branching against hoisting branch-local variables - #26
Open
karthikbhargavan wants to merge 6 commits into
Open
pure: harden simplify_let_branching against hoisting branch-local variables#26karthikbhargavan wants to merge 6 commits into
karthikbhargavan wants to merge 6 commits into
Conversation
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
Co-authored-by: Alexander Bentkamp <alexander@cryspen.com>
…iables The candidate outputs collected by push_to_outs were never filtered against bound_fvars (the variables bound inside the bound expression). With at least two ok endpoints this is safe by construction, but a switch-in-let with a single ok endpoint (all other endpoints being fail/break/continue) would take that endpoint's raw tuple args verbatim and hoist them to let-bindings after the switch, where branch-local binders are out of scope. Latent today (SymbolicToPure only creates a join-let when >= 2 branches reach the join), but one producer change away from live miscompilation. Fix: drop, in push_to_outs, any candidate whose free variables intersect bound_fvars — mirroring how is_bound_before_fvar already guards scrutinees. Fixes #23 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
simplify_let_branchingrewriteslet (x, y, ...) = switch ... in nextby hoisting outputs that are identical in every branch into let-bindings after the switch. The candidate outputs (the raw endpoint tuple args) were never filtered againstbound_fvars, the set of variables bound inside the bound expression; the existingis_bound_before_fvarguard was only applied to switch scrutinees.Degenerate single-ok-endpoint case:
fail/break/continueendpoints contribute no candidates, so a switch with exactly one ok endpoint gets that endpoint's raw args verbatim (the branch-wise intersection degenerates to a single set). If such an arg mentions a branch-local binder (match binder or intermediatelet), the hoist emits a let-binding outside the switch referencing an out-of-scope variable — an unbound fvar, i.e. a crash in a later pass or invalid extracted code.The defect is latent today:
SymbolicToPureonly creates a join-let when >= 2 branches reach the join point, and with >= 2 ok endpoints the intersection cannot contain branch-local fvars (their ids differ per branch). But nothing enforced that invariant; any future producer of a single-ok-endpoint switch-in-let would silently activate the bug.Fix: in
push_to_outs, drop any candidate whose free variables intersectbound_fvars(FVarId.Set.disjoint (texpr_get_fvars e) bound_fvars). This only disables the optimization in the unsound case and mirrors howis_bound_before_fvaralready guards scrutinees.Fixes #23
Validation
dune buildanddune build @fmtpass (charon pinned atcb50ff16, v0.1.223).-backend leanby both the patched build and an unpatchedorigin/devbuild: identical exit statuses (88 ok;raw_pointersfails identically in both, pre-existing and unrelated) and byte-identical generated Lean across the whole battery.probe_pass_fires.rs(two liveifarms sharing the constant5for one output slot) still gets the hoist with the patched build —let y1 ← if c then y + 1#u32 else y + 2#u32; let x1 ← 5#u32 + 10#u32; ...— identical to the unpatched output.probe_fail_endpoint_in_switch_let.rs(switch-in-let with afail panicendpoint) also translates identically.🤖 Generated with Claude Code