Batch: continue past compile errors with per-unit isolation#1301
Merged
Conversation
8fd4dd8 to
70e8d21
Compare
70e8d21 to
bd4bb08
Compare
bd4bb08 to
1cd0287
Compare
b73de07 to
efeeb0f
Compare
1cd0287 to
8126b54
Compare
Stacked on the backend-mapper-emits-errors refactor. `--batch` used to stop at the first failing unit. This makes it continue past compile errors, reporting each and exiting non-zero if any failed — needed for test-runner use. A compile error corrupts the shared reactive context: after any error, every later unit silently produces no output. Deferring the error is not enough — the context is still poisoned. So type the stdlib once into a warm base, then compile each unit on a throwaway copy-on-write clone of that base via SKStore.withForkedContext (added in #1304): the clone isolates the unit's typing and any error from the base and the other units, and its memory is reclaimed afterwards so large batches stay flat. Because the backend mapper now emits its outcome to /backendSink/ instead of reporting from inside the reactive computation, batch mode just reads that outcome in compile() and raises a recoverable CompileFailureException that compile_batch reports before moving to the next unit — no side-channel global, and the mapper stays mode-agnostic. skipError.sk adds a batch-mode flag so catchErrors collects (not exits) frontend errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8126b54 to
c895871
Compare
4 tasks
mbouaziz
added a commit
that referenced
this pull request
Jul 10, 2026
…fatal) skc --batch (#1301) crashes on invalid inputs that a one-shot compile reports cleanly: batching the 826 invalid_tests/ aborts at the second unit with `Invariant violation: assert` (or a silent exit 2 with no summary), losing every remaining unit. #1301's continue-on-error is only safe for typing errors (keepErrors, which the pipeline already defers — the /backendSink/ mapper checks ERRORS before codegen). It is unsafe for the earlier phases: - catchErrors (naming, expand, …) was made to collect-and-continue in batch mode, but those phases abort the whole compile via printErrorsAndExit in one-shot mode and the later phases rely on that; continuing runs them on inconsistent state and trips an invariant. - fatalError (e.g. "Unbound class") calls printErrorsAndExit directly, so in batch mode it exits the whole process mid-batch. Add SkipError.printErrorsOrRecover: one-shot prints + exits as before; batch mode raises a recoverable CompileFailureException carrying the same rendered errors, aborting just this unit. Its throwaway forked context is torn down safely by withForkedContext (#1304), so the shared base stays pristine — and this mirrors one-shot's stop-at-first-error, so per-unit error text still matches the .exp_err goldens. catchErrors and fatalError route through it; keepErrors (typing) and all one-shot behavior are unchanged. Verified: the 826 invalid_tests/ now batch cleanly in one process (0 invariant violations, all reported); per-unit error text matches one-shot skc across every category; one-shot and valid-batch paths unregressed. Unblocks #1300. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mbouaziz
added a commit
that referenced
this pull request
Jul 10, 2026
…fatal) (#1313) ## Summary `skc --batch` (continue-on-error, #1301) **crashes on invalid inputs that a one-shot `skc` handles cleanly**. Compiling the compiler's own 826 `invalid_tests/` through one `--batch` process aborts almost immediately with `Invariant violation: assert` (or a silent `exit 2` with no failure summary), losing every remaining unit. Minimal repro — `invalid_tests/typechecking/overridable7.sk` (a "cannot override a final method" error): ``` skc -O0 --no-inline -o /tmp/x invalid_tests/typechecking/overridable7.sk # clean error, exit 2 echo "/tmp/x main invalid_tests/typechecking/overridable7.sk" > m skc --batch m -O0 --no-inline # Invariant violation: assert ``` ## Root cause #1301 made batch mode *continue* past a unit's compile error so the shared context can serve later units. That is safe for **typing** errors (`keepErrors`, which the pipeline already defers — the `/backendSink/` mapper checks `ERRORS` before codegen). It is **not** safe for the earlier phases: - `catchErrors` (naming, expand, …) was changed to *collect and continue* in batch mode (delegating to `keepErrors`). But in one-shot mode those phases abort the whole compile via `printErrorsAndExit`, and the phases that run after them rely on that. Collecting the error and carrying on runs them on inconsistent state → `invariant` trips. - `fatalError` (e.g. "Unbound class") calls `printErrorsAndExit` **directly**, so in batch mode it `skipExit`s the whole process mid-batch, killing every remaining unit. ## Fix Add `SkipError.printErrorsOrRecover`: in one-shot mode it prints + exits (unchanged); in **batch mode** it raises a recoverable `CompileFailureException` carrying the same rendered errors, aborting **just this unit**. Its throwaway forked context is torn down safely by `SKStore.withForkedContext` (#1304) — the `catch` there destroys the fork's obstack even on exception — so the shared base stays pristine for the remaining units, and this exactly mirrors one-shot's "stop at the first error" semantics (so the per-unit error text still matches what generated the `.exp_err` goldens). `catchErrors` and `fatalError` now both route through it. `keepErrors` (typing) is unchanged. No one-shot behavior changes — the batch-mode branch is the only new path. ## Verified (rebuilt stage1) - **The 826 `invalid_tests/` in one `--batch` process:** was — aborted at the 2nd unit with an invariant; now — **all 826 compile-and-fail cleanly, 0 invariant violations, `Batch compilation failed: 826`**. - **Error-text parity:** for a sample spanning every category (syntax, typechecking, exhaustiveness, expand, visibility, runtime), each unit's batch error output equals its one-shot `skc` stderr (modulo a trailing newline the harness already `.trim()`s). - **No regression:** one-shot valid compile + run, one-shot invalid (prints error, exit 2, no binary), and valid `--batch` (produces + runs binaries, exit 0) all unchanged. ## Why it matters / unblocks This is a prerequisite for #1300 (batching the compiler test harness's invalid-test subprocess path): that path can only batch the invalid set once `--batch` survives every invalid unit. It's also a correctness fix in its own right — `--batch` should never crash on input a one-shot compile reports cleanly. ## Test plan - [x] 826 invalid tests batch cleanly (no invariant; all reported) - [x] per-unit error parity vs one-shot across all categories - [x] one-shot + valid-batch paths unregressed - [x] CI 🤖 Generated with [Claude Code](https://claude.com/claude-code)
3 tasks
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
Implements #1299. Stacked on #1306 (backend mapper emits errors as output) and builds on #1298 (merged).
--batchpreviously stopped at the first failing unit. This makes it continue past compile errors, reporting each and exiting non-zero if any failed — needed for test-runner-style use.The hard part is that a compile error corrupts the shared reactive context: I verified that after any error, every later unit silently produces no output (the same problem the earlier batch prototype hit). Deferring the error (collect instead of exit) is not enough — the context is still poisoned.
Approach: per-unit isolation
Type the stdlib once into a warm base context, then compile each unit on a throwaway copy-on-write clone of that base via
SKStore.withForkedContext(added in #1304): it clones the context, runs the body on the clone inside a fresh region, and reclaims the clone's memory afterwards. A unit's typing and any error mutate only its clone, so the base stays pristine and a failed unit can't affect the others; large batches don't accumulate state.Because the backend mapper now emits its outcome to
/backendSink/(via #1306) instead of reporting from inside the reactive computation, batch mode simply reads that outcome incompile()and raises a recoverableCompileFailureExceptionthatcompile_batchreports before moving to the next unit — no side-channel global, and the mapper stays mode-agnostic.skipError.skadds a batch-mode flag socatchErrorscollects (not exits) frontend errors.Verified
[good, bad, good, bad, good, good]→ every good unit (including those after failures) produces correct output; both failures reported; exit 2.withForkedContextregion payoff); ~7× faster than per-fileskc.Why a new helper, not
fork()fork()=mclone+ obstack + a named-fork registry aimed at SKDB's server pattern (and there are zero Skip-levelfork()/withContextcallers — forks are driven from the C runtime). Entering a fork to runcompile()has no fitting Skip API (withContextgives a readonly context;runWithGc(fork=…)re-enters the GC entry point).withForkedContext(#1304) is a small prelude combinator that gives exactly the read-only-base + per-call isolation + memory reclamation this needs, and the call site is a one-liner.Unblocks
With isolation in place, the test harness subprocess path (incl. invalid tests) can be batched — #1300.
Test plan
🤖 Generated with Claude Code