Skip to content

fix(transform): closes #342 — cond ? await a() : b() evaluates BOTH branches - #356

Merged
proggeramlug merged 2 commits into
mainfrom
fix/342-ternary-await
Apr 30, 2026
Merged

fix(transform): closes #342 — cond ? await a() : b() evaluates BOTH branches#356
proggeramlug merged 2 commits into
mainfrom
fix/342-ternary-await

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

What's broken

A conditional expression with await on one side ran the awaited call regardless of the condition. The original repro from #342:

async function a(): Promise<number> { console.log('ran a'); return 1; }
function       b():        number    { console.log('ran b'); return 2; }
async function main(): Promise<void> {
  const cmd: string = 'digest';
  const r = cmd === 'fetch' ? await a() : b();
  console.log('r=' + r);
  process.exit(0);
}
main().catch((e) => { console.log(e); process.exit(1); });

Buggy output: ran a / ran b / r=2 (a ran even though the digest branch was taken).

Root cause

Not codegen — the async→generator pre-pass. The hoister blindly walks expression children looking for Expr::Await and lifts each into a let __await_N = await X placed BEFORE the containing statement. So

let r = cond ? await a() : b();

becomes

let __await_N = await a();
let r = cond ? __await_N : b();

a() runs unconditionally. --print-hir confirms it: then_expr: LocalGet(N), and the original Call got moved to state 0 of the generator state machine.

Fix

Detect Expr::Conditional whose then- or else-branch contains an await and, before any general hoisting walks into it, lift the whole conditional to a statement-level if/else with a temp local:

let __cond_await_N: any;
if (cond) { __cond_await_N = await a(); }
else      { __cond_await_N = b(); }
let r = __cond_await_N;

The recursive hoist_awaits_in_stmts call inside each branch then hoists the await to the top of its own if-arm — the position the await→yield rewrite expects. Both hoist_awaits_in_expr_full and hoist_awaits_avoiding_top_level get the early dispatch, so the lift fires whether the conditional is at let-init or in a deeper position.

Verification

All four await-shape combinations behave correctly:

Shape Before After
cond ? await a() : b() both branches only matched
cond ? a() : await b() both branches only matched
cond ? await a() : await b() both branches only matched
cond ? a() : b() (sync only) already correct unchanged

Plus the actual skelpo-listener pattern that surfaced the bug:

const code = args.command === 'fetch' ? await runFetch(args) : runDigest(args);

now runs only runDigest on a digest invocation.

cargo build --release -p perry clean.

Closes #342

…ranches

Issue #342: a conditional expression with `await` on one side ran the
awaited call regardless of the condition. Previously a `digest`-bound
codepath that wrote `args.command === 'fetch' ? await runFetch() : runDigest()`
also executed `runFetch` (printing fetch progress) before falling through
to `runDigest`.

Root cause is in the async→generator pre-pass, not the codegen. The
hoister blindly walks expression children looking for `Expr::Await` and
lifts each into a `let __await_N = await X` placed BEFORE the containing
statement. For

    let r = cond ? await a() : b();

it lifts `await a()` to `let __await_N = await a();` so the resulting
HIR is

    let __await_N = await a();
    let r = cond ? __await_N : b();

— which executes `a()` unconditionally. The HIR confirms it: dumping
with `--print-hir` shows `then_expr: LocalGet(N)` and the original call
site moved to state 0 of the generator state machine.

Fix: detect `Expr::Conditional` whose then/else branch contains an
await and, before any general hoisting walks into it, lift the whole
conditional to a statement-level if/else with a temp local:

    let __cond_await_N: any;
    if (cond) { __cond_await_N = await a(); }
    else      { __cond_await_N = b(); }
    let r = __cond_await_N;

The recursive `hoist_awaits_in_stmts` call inside each branch then
hoists the await to the top of its own if-arm — the position the
await→yield rewrite expects. Both `hoist_awaits_in_expr_full` and
`hoist_awaits_avoiding_top_level` get the early dispatch so the lift
fires whether the conditional is at let-init or in a deeper position.

Verified with all four await-shape combinations:

    cond ? await a() : b()         — only the matched branch runs (was: both)
    cond ? a() : await b()         — only the matched branch runs (was: both)
    cond ? await a() : await b()   — only the matched branch runs (was: both)
    cond ? a() : b()               — unchanged (was already correct)

Plus the actual skelpo-listener pattern that surfaced the bug:

    const code = args.command === 'fetch' ? await runFetch(args) : runDigest(args);

now runs only `runDigest` on a `digest` invocation.

`cargo build --release -p perry` clean.
@proggeramlug
proggeramlug force-pushed the fix/342-ternary-await branch from aaa8a3e to 8c16ef7 Compare April 30, 2026 16:51
@proggeramlug
proggeramlug merged commit e3b81c4 into main Apr 30, 2026
5 of 8 checks passed
@proggeramlug
proggeramlug deleted the fix/342-ternary-await branch April 30, 2026 16:53
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.

Codegen: cond ? await a() : b() evaluates BOTH ternary branches

1 participant