Skip to content

An unused local whose initializer can raise keeps its initializer - #4310

Merged
borisbat merged 4 commits into
masterfrom
aleksisch/unused-may-raise
Oct 26, 2026
Merged

borisbat merged 4 commits into
masterfrom
aleksisch/unused-may-raise

Conversation

@aleksisch

@aleksisch aleksisch commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Behavior change: an optimized build no longer skips a runtime error. Code that relied on a dead a[i] or x / 0 staying silent under optimization now throws, as it already did unoptimized.

Why. noSideEffects answers "writes nothing an observer can see", not "cannot throw", and every pass that deletes an evaluation read only that flag. So the optimizer removed the throw along with the value nobody wanted: an unused var u = a[10] on a two-element array, a dead store y = a[10] overwritten before any read, a[10] - a[10], a[10] * 0 and if (a[10] > 0) {} all ran to the end of main with the optimizer on and raised under options optimize = false — in the interpreter, under -jit and under -aot alike. A release build skipped an error its debug build reported, and a recover the program depends on never ran.

What changes.

  • src/ast/ast_can_raise.h asks the question once for every pass, the way ast_alias.h already shares the alias model between CSE and DSE.
  • canRaise walks a subtree, not a root: the root of a[10] + 1 is an ExprOp2 that cannot throw on its own.
  • It answers yes for ExprAt, ExprAsVariant, ExprPtr2Ref, ExprField through a pointer, / and %, and every ExprCall; the safe spellings have their own visitor hooks and stay out.
  • The deleting passes ask it: the unused-local delete, the three dead-store gates, and in const folding the operand drops, the E op E folds that collapse to a constant, the dropped conditions and the top-level pure statement drop.
  • A fold that keeps one evaluation is untouched: x & x and x | x return the left operand, which still throws. CSE is untouched for the same reason.
  • A dead local whose initializer can raise becomes a bare statement instead of staying a let, the way ast_dse.cpp keeps a dead store's rhs; keeping the variable grew the frame.
  • ExprMakeVariant with a null field value no longer dereferences it while stamping access flags.

Observable behavior.

  • optimized var u = a[10], y = a[10] then y = 5, a[10] - a[10], a[10] * 0, if (a[10] > 0) {} run to the end -> they raise, matching options optimize = false
  • optimized var u = a[10] + 1 runs to the end -> it raises
  • var unused = a + b, x & x, x | x -> unchanged, still folded

Where to look. src/ast/ast_can_raise.h and its call sites in ast_unused.cpp, ast_dse.cpp, ast_const_folding.cpp and ast_block_folding.cpp.

#nightly

Validation, claims, ledger

Validation

  • The first push of this branch was red on every Release lane: tests/lsp/test_lsp_protocol.das:489: stack overflow under -jit, 2 errors, all four platforms. Reproduced locally, bisected to ast_unused.cpp in two builds (reverting the four files passes 4/4, restoring only that one fails), and fixed by dropping the dead variable instead of keeping it. That test is one function with 115 locals and a captured lambda, so it was the first frame to blow.
  • Full JIT sweep local, the lane that caught it: 13999 tests, 13987 passed, 0 failed, 0 errors, 12 skipped.
  • Negative control, same build minus the guards: 3 of the 8 new test cases fail (E op E, the folded operand, the dropped condition) and the earlier three fail with the unused-local guard reverted. The dead-store case passes either way in this shape - its guard is justified by the pass, not by that test.
  • An over-broad first cut guarded samePureValue itself and broke tests/language/optimization_fold_shape.das ("arr[i] & arr[i] keeps a single read"). That fold keeps one evaluation, so it is sound; only the two samePureValue uses that collapse to a constant carry the question now.
  • Compile-time A/B on the single-pass version, two binaries from one commit differing only in ast_unused.cpp, interleaved over three rounds: every delta (-1.27% to +3.23%) sits under the 1.9-4.8% within-binary spread. The question is the last term of each condition, so it runs only when a pass is about to delete.
  • Interpreter suite local: 14093 tests, 14081 passed, 0 failed, 0 errors, 12 skipped. Lint and dasfmt --verify clean.
  • Gates not run here: the AOT build and full AOT suite, dupe and agent audits, and woodpecker. CI covers the first two.

Claims - stated, not tested

  • CSE needs no guard: it reuses the first evaluation, which still throws. Verified by reading ast_cse.cpp:277 and its consumers rather than by a test. A break would show as an array read losing its bounds check after CSE.
  • The operand-swap arm in the splat collapse reorders rather than drops, so it is left alone. A break would show as two throwing operands firing in the other order.

@aleksisch
aleksisch force-pushed the aleksisch/unused-may-raise branch 8 times, most recently from 27c1fc6 to 60a54bb Compare October 20, 2026 10:24
@aleksisch
aleksisch force-pushed the aleksisch/unused-may-raise branch 2 times, most recently from 3bcdcb1 to 61ec181 Compare October 22, 2026 15:40
@aleksisch
aleksisch requested a review from borisbat October 22, 2026 15:46
@aleksisch
aleksisch force-pushed the aleksisch/unused-may-raise branch 3 times, most recently from ee76322 to fc7eec7 Compare October 25, 2026 10:31
@aleksisch
aleksisch force-pushed the aleksisch/unused-may-raise branch 2 times, most recently from dc61800 to c17ffce Compare October 26, 2026 07:50
Under -q a clean run still printed the summary block, so every lint chunk
of the build wrote three lines of noise. -q now prints the summary only
when there is an issue or an error; skipped files alone do not count.
The exit code is unchanged.
terminal_checkpoint_ansi wrote "\r\n" between rows with the last cell's
style still active. When the replay scrolls, the emulator fills the new
row from the pen, so a row ending in a styled cell left that style on
every blank cell of the row scrolled in below it.

terminal_semantics.das asserted this, but its assert_snapshots_equal call
only asserts, so the optimizer deleted the call and the check never ran.
Every pass that asked whether a function is pure tested sideEffectFlags
against zero, so any bit added to SideEffects would turn all of those
checks off at once. Function::observableEffects and hasObservableEffects
read the word through one mask (userScenario, modifyExternal,
modifyArgument, accessGlobal, invoke, captureString), and the const
folder, lint, escape analysis, the builtin registration checks and the
AST printer call them. Every consumer needs the whole mask today, so
nothing folds differently;
src/ast/ARCHITECTURE_EFFECTS.md#observable-effects says why for each.
`noSideEffects` means "writes nothing an observer can see", not "cannot
throw", and every deleting pass read only that flag. So an unused
`var u = a[10]`, a dead store `y = a[10]`, `a[10] - a[10]`, `a[10] * 0` and
`if (a[10] > 0) {}` ran to the end of main with the optimizer on and raised
under `options optimize = false`.

Raising is one more side effect, `SideEffects::mayRaise`, outside the
observable-effects mask: a call that can raise still folds, rewrites and
passes escape analysis and the lint, and only a pass that deletes an
evaluation asks about it. A builtin declares it in its binding - integer `/`
and `%`, `panic`, the string conversions and character reads. A das function
gets it like every other inferred bit: getSideEffects sets it from the
function's own nodes and ORs it in from its callees, and a callee still
being analyzed counts as raising. Each expression carries `noRaise` beside
`noSideEffects`, computed by the same NoSideEffectVisitor for the whole
subtree. Both take the node rule from one place: `a[i]`, `as`, a
dereference, a field read through a pointer, `assert` / `verify` and an
invoke raise by their class, and making a block does not. A fold that keeps
one evaluation is untouched, and so is CSE. Raising builtins no longer get
`nounwind` / `willreturn` in the JIT.

`ExprMakeVariant` with a null field value no longer dereferences it while
stamping access flags.
@aleksisch
aleksisch force-pushed the aleksisch/unused-may-raise branch from c17ffce to 43efd45 Compare October 26, 2026 08:36
@borisbat
borisbat merged commit 9062ebf into master Oct 26, 2026
30 checks passed
@borisbat
borisbat deleted the aleksisch/unused-may-raise branch October 26, 2026 14:51
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.

2 participants