Skip to content

renameShadowedNames miscompiles local recursive groups: member order reversed, member RHSs renamed in the wrong scope #133

Description

@Unisay

Two related defects in renameShadowedNamesInExpr (lib/Language/PureScript/Backend/IR/Optimizer.hs), both in the RecursiveGroup branch of the Let case. Both produce wrong runtime behavior without any compile-time error.

Bug 1: member order of every local RecursiveGroup is reversed

The fold over group members conses each processed member onto an accumulator and the result is never reversed back:

RecursiveGroup (toList  recGroup) 
  (: bs) . RecursiveGroup . NE.fromList <$> foldl' g (sc, []) recGroup
 where
  g (sc', recBinds) (ann', name, expr) =
    ... (sc'', (ann', name', expr') : recBinds)   -- cons, no reverse afterwards

The sibling code path for Let groupings does compensate (NE.fromList (reverse binds')); the member list inside a group does not. So every RecursiveGroup inside an expression comes out of renameShadowedNames with its members in reverse order, unconditionally.

Member order is not cosmetic: it is the initialization order computed by the laziness transform (Language.PureScript.CoreFn.Laziness.applyLazinessTransform reorders the group so that statically orderable bindings initialize before their dependents), and the Lua codegen emits the group's assignments in member order. Reversing it undoes the laziness analysis.

Repro (full pipeline)

-- A laziness-ordered eager group: x must initialize before y,
-- because y reads x immediately (delay 0), while x only refers to y under a lambda.
let x = Name "x"; y = Name "y"
let recGroup = RecursiveGroup
      (  (noAnn, x, abstraction paramUnused (refLocal y 0))
      :| [(noAnn, y, literalObject [(PropName "foo", refLocal x 0)])] )
let uber = UberModule [] [] [(Name "main", lets (recGroup :| []) (refLocal y 0))]
uberModuleExports (optimizedUberModule uber)

The group comes out as [y, x], so the generated Lua is:

local x
local y
y = { foo = x }              -- x is still nil here
x = function() return y end

y.foo is nil at runtime. The corresponding PureScript shape is ordinary code, e.g. in a let/where block:

x = \_ -> y
y = { foo: x }

The reversal is already visible in a committed golden: test/ps/output/Golden.RecursiveBindings.Test/golden.ir has the a/b group in the reverse of the order the laziness transform emits. It happens to be harmless there because both members are lambdas, so their initialization order doesn't matter.

Top-level recursive groups are not affected (renameShadowedNames maps over top-level binding expressions individually and never touches grouping order); only Let ... RecursiveGroup inside expressions.

Bug 2: member RHSs are renamed in a scope that lacks the group's own renames

g renames each member's RHS with sc', the scope accumulated over the previous members only. But recursive-group scoping (Note [Sequential scoping of Let bindings]) makes every member visible in every member's RHS, itself included. When a group member shadows an outer binder, self-references and forward references inside the group keep the old name while the binder gets renamed, so they quietly rebind to the outer binder.

Repro

-- \x -> let rec x = f x@0 in x@0     (the member's RHS refers to itself)
renameShadowedNamesInExpr mempty $
  abstraction (paramNamed x) $
    lets (RecursiveGroup ((noAnn, x, application (exception "f") (refLocal x 0)) :| []) :| [])
         (refLocal x 0)

Actual result: the binder becomes x1 and the body reference follows, but the RHS still says Ref (Local x) 0, which now resolves to the outer lambda parameter. The recursion is severed:

Abs (ParamNamed x) (Let (RecursiveGroup ((x1, App (Exception "f") (Ref x 0)) :| [])) (Ref x1 0))

Forward references hit the same problem: with members [(y, refLocal x 0), (x, literalInt 1)] under an outer binder x, the member x is renamed to x1 but y's reference stays x and rebinds to the outer binder. A shadowing name in a local recursive group is common (go, f in nested where blocks), so this is reachable from ordinary source code.

Fix sketch

  • Bug 1: reverse the member list after the fold, symmetrically with the groupings fix-up.
  • Bug 2: process each group in two passes: first run withScopedName for all members to build the group's complete scope, then rename every member's RHS in that scope.

Both fixes should come with red-first regression tests in test/Language/PureScript/Backend/IR/Optimizer/Spec.hs (the repros above are ready to be turned into cases under the "renames shadowed names" describe block), plus an eval golden for the order-sensitive group so the runtime consequence stays pinned.

Found during a backend audit on 2026-07-02; bug 2 confirms a hypothesis first noted during the issue #37 work.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions