Fail-loud runtime failures report where they happened (#153) - #158
Open
assapir wants to merge 1 commit into
Open
Fail-loud runtime failures report where they happened (#153)#158assapir wants to merge 1 commit into
assapir wants to merge 1 commit into
Conversation
A checked `arr[i]` that is out of bounds, negative, or NaN, and a violated
`Text.replace` / `replaceAll` / `repeat` contract, now report the expression that broke
the contract instead of a bare stderr line:
demo.ql:4:11: index 7 out of bounds for an array of size 3
|
4 | value = items[wanted]
| ^^^^^^^^^^^^^
Same frame as a compile error and a failing assertion, so every located failure a
program can produce reads the same way, and a program with several array reads no
longer leaves you hunting for which one it was. Exit codes are unchanged (1 for a bad
index, 101 for a `Text` contract) and a redirected report stays plain.
The location is compiled in: each fallible intrinsic takes a pointer to the read-only
`Site` constant codegen already materializes for assertions, so a native build reports
exactly what the JIT does with no debug info and no unwinder. On the success path this
is free for indexing (the site is built only in the cold failure branch) and one
argument for the text methods.
`@readStdin` moves onto the same mechanism, replacing the parallel one it arrived with
— a pre-rendered `path:line:col` string threaded through a `read_sites` map in the
deferral pass — which is deleted along with its accessors and walker. It gains what the
map could not do: a location for an `@readStdin` inside an imported module.
Three renderers now draw this frame (the compiler's, `core.test`'s in Quilon, and the
runtime's), so `tests/fail_loud_location_test.rs` pins them to the same output, and
also holds the runtime's hand-written `QlSite` to the layout codegen actually emits —
a drifted field order would otherwise read a text pointer as a line number rather than
fail to build.
CI now runs `cargo test`/`clippy`/`build` with `--workspace`: without it the runtime
crate's own unit tests were never compiled there, so a change to an intrinsic's
signature could (and did) leave them broken with CI green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 #153.
A fail-loud runtime failure now says where it happened, in the frame a compile error and a failing assertion already use:
Covered: checked
arr[i](out of bounds, negative, fractional-invalid, NaN) and theText.replace/replaceAll/repeatcontracts. Exit codes are unchanged (1for a bad index,101for aTextcontract), and a redirected report stays plain.How
The plumbing from #152 is reused rather than duplicated: a
#[repr(C)] QlSitein the newquilon-rt/src/report.rsmirrors the built-inSiterecord, one sharedfail_at(site, message, code)draws the frame, and each fallible intrinsic takes a trailing*const QlSite— the pointer to the read-only constantsite_valuealready emits. So:lea, never executed). For the three text methods it is one argument per call (two instructions where the call crosses the register limit)..at(i)is untouched — the non-aborting form still answersOk/NotOk.@readStdinmoves onto the same mechanism. It arrived (in #149) with a parallel one: a pre-renderedpath:line:colstring threaded through aread_sites: HashMap<Span, String>map filled by the driver, reported unframed. That map, its two accessors,read_call_sites, and a walker are deleted. The conversion is strictly better — the old map only recorded root-file spans, so an@readStdininside an imported module had no location at all — and costs one user-visible message change (@readStdin at X failed: …→ a framed@readStdin failed: …). Flagging it because it touches recently-merged work: say so and I'll split it out.Tests, docs, example
tests/fail_loud_location_test.rs(new): pins all three renderers — compile error, assertion (composed in Quilon), runtime check (composed in Rust) — to the same frame, since drift between them is the obvious failure mode. It also holds the runtime's hand-writtenQlSiteto the layout codegen actually emits: nothing else connects them, and a drifted field order would make the runtime read a text pointer as a line number rather than fail to build. Plus native-vs-JIT byte equality.tests/index_checks_test.rs,tests/text_methods_test.rs: each failure kind reports its own location; two reads in one program report different lines; a redirected report carries no ANSI.examples/index_out_of_bounds.ql: fails on purpose, registered inEXPECT_RUNTIME_FAILURE(whose gate also checks the example's header documents the exact report it prints), and linked from the indexing docs.docs/LANGUAGE.md: the fail-loud principle, the indexing section, thereplace/repeatcontract paragraph, and the error-messages section.CHANGELOG.md.benches/compile_speed.rsgains anarray_readscorpus: no existing corpus contains a singlearr[i], so the cost profile this change scales up was invisible to the bench.One CI fix, found the hard way
CI ran
cargo test/clippy/buildwithout--workspace, soquilon-rt's own unit tests were never compiled there. This change broke six of them (an intrinsic gained a parameter) and the gate stayed green locally and in CI until a review caught it. All three steps now pass--workspace.Escalations — not decided here
core.cli'sgetEnvreportscore.cli:29:20: …, which is honest and useless to the caller. Assertions solve this by forwarding a trailingSite; a runtime intrinsic has no forwarding path. Fixing it is a design question (what wouldarr[i]inside a library function know about its caller?), so it is not in this PR. This is the next thing the mechanism needs.replaceAll("-", "+")the runtime abort is statically unreachable, yet the 56-byte constant and its two relocations are still emitted. Passing a null site in exactly those cases means mirroring the checker's literal predicate in codegen — and getting it slightly wrong silently loses a location, which is worse than the bytes. Left alone deliberately.--strip-locationsbuild flag would be the answer if it matters.quilon-rt/src/scheduler.rs's fiber-stack and reactorexpects (bare Rust panics reachable from any@primitive), and__allocreturningGC_mallocwith no null check — a silent failure in the same module whose other fail-loud path this PR located.write/nowshadowing find is deliberately not folded in — codegen intercepts those names unconditionally, so a user function with one of them is silently replaced. Fixing it means deciding what should happen when a user defineswrite, which is user-visible design. It stays noted on Runtime fail-loud failures should report file:line:column too #153's sibling discussion.Full gate green:
cargo fmt --check,cargo clippy --workspace --all-targets --all-features -D warnings,cargo test --workspaceunderRUSTFLAGS=-D warnings(38 binaries), native-AOT gates under both linkers./simplifyrun — it produced the CI gap, the layout-parity test, an unnecessaryunsafe impl Send, and three stale comments.🤖 Generated with Claude Code