fix: unshift dropped Let binders and resolve body scope per let* in IR DCE#148
Merged
Conversation
…R DCE (#134) Two defects in the Let handling of IR dead code elimination, both of the De Bruijn bookkeeping class of #37/#56: 1. Dropping a dead binding from a Let removed a slot from that name's De Bruijn namespace without lowering the references that skipped over it, unlike the Abs case (fixed in #56). A dangling reference either crashed the Lua codegen with UnexpectedRefBound or silently resolved to the wrong binder. The Let case of dceAnnotatedExp now walks the groupings left to right and, for every dropped binder, unshifts the rest of the Let (later grouping RHSs and the body). The unshift traversal is reused by wrapping the remainder back into a Let, so the scope threading follows Note [Sequential scoping of Let bindings] instead of being re-implemented. 2. The scope the Let body was resolved against in adjacencyListForExpr was built with a foldr, putting the *first* binding of a name at index 0 where the let* convention requires the *last*. Reachability thus marked the wrong binding live among same-name siblings. The body now uses the same scope that adjacencyListForGrouping threads through the groupings. Both defects are latent on the current corpus (no golden churn): same-name Let siblings cannot be produced from source today, and the dangling-reference path needs an inliner-created shadowing binder whose uses later die. Regression tests pin the semantics at the DCE level; an end-to-end golden can follow once a source-level reproduction for the second path is pinned down.
There was a problem hiding this comment.
Pull request overview
This PR fixes two De Bruijn index bookkeeping defects in the IR dead-code-elimination pass (Language.PureScript.Backend.IR.DCE) specifically for Let bindings, aligning both reachability analysis and binder dropping behavior with Note [Sequential scoping of Let bindings]. It adds focused regression tests to prevent reintroducing the same class of Let scoping/unshift bugs implicated in #134 (and previously #56-style failures).
Changes:
- Fix DCE’s
Letrewriting to unshift later RHSs/body when a deadLetbinder (or recursive-group member) is dropped, preventing dangling/wrong binder references. - Fix reachability graph construction so
Letbodies resolve same-name binders innermost-first (let*) by using the scope threaded through groupings (instead of a reversedfoldr-built scope). - Add four regression tests covering body-scope resolution and unshift behavior for dropped binders (including recursive groups), plus a changelog fragment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
lib/Language/PureScript/Backend/IR/DCE.hs |
Corrects Let DCE binder dropping (with tail unshift) and fixes body-scope construction for reachability to match let* semantics. |
test/Language/PureScript/Backend/IR/DCE/Spec.hs |
Adds targeted regression tests for Let body scope ordering and unshift-after-drop behavior (standalone + recursive group). |
changelog.d/20260702_130000_unisay_dce_let_scope.md |
Documents the two Let-handling fixes and their user-visible impact (crash vs silent misbinding). |
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.
Closes #134.
Two defects in the
Lethandling ofLanguage.PureScript.Backend.IR.DCE, both of the De Bruijn bookkeeping class of #37 and #56.The first: dropping a dead binding from a
Letremoved a slot from that name's De Bruijn namespace without lowering the references that skipped over it, unlike theAbscase (fixed in #56). A dangling reference either crashed the Lua codegen withUnexpectedRefBoundor silently resolved to the wrong binder. TheLetcase ofdceAnnotatedExpnow walks the groupings left to right and, for every dropped binder, unshifts the rest of theLet(later grouping RHSs and the body). Rather than re-implementing the scope threading, the fix reuses theunshifttraversal by wrapping the remainder back into aLet, so the bookkeeping follows Note [Sequential scoping of Let bindings] by construction. Dead recursive-group members get the same treatment.The second: the scope the
Letbody was resolved against inadjacencyListForExprwas built with afoldr, putting the first binding of a name at index 0 where the let* convention requires the last. Reachability thus marked the wrong binding live among same-name siblings. The body now uses the same scope thatadjacencyListForGroupingthreads through the groupings one line below.Four regression tests were written red first: body scope resolution among same-name siblings, unshift of the body after dropping a dead shadowing binder, unshift of later sibling RHSs, and unshift after dropping a dead recursive-group member.
Both defects are latent on the current corpus, and the fix produces no golden churn: same-name
Letsiblings cannot be produced from source today, and the dangling-reference path needs an inliner-created shadowing binder whose uses later die. An end-to-end golden can follow once a source-level reproduction for that path is pinned down, as noted in the issue.