Close #15/#16: stamp errors with their call site; expose it to builtins

[odrobnik]

Closes #15, closes #16.

Both issues are the same gap seen from two sides: the interpreter knows where evaluation is, bridges and builtins know what went wrong, and no channel connected the two. The channel is a task-local: the expression dispatcher binds Interpreter.evaluationOffset to each node's UTF-8 offset for the duration of its evaluation, so the innermost binding always names the expression being evaluated — which, while a builtin or bridge body runs, is the call that invoked it. Task-local rather than instance state so script Task { … } concurrency can't corrupt a save/restore stack, and unwinding is automatic.

#15 — thrown errors name their line

• RuntimeError gains indirect case positioned(RuntimeError, at: Int) plus positioned(at:), which never double-wraps — the earliest (innermost, most precise) stamp wins. Existing cases, call sites, and description are untouched, so catch-and-match code keeps working.
• ScriptError gains an offset: Int? — the catchable-error vehicle from Close #11/#12: defer leading-dot member args to the callee; make bridge errors catchable #13 can now say where it was raised.
• callingBridge stamps everything it wraps with the invoking expression's offset: RuntimeErrors (both the boxed error and the signal around it), raw host errors, and pre-wrapped signals thrown by generated Foundation bridges. Script throws are stamped at the throw statement itself.
• The dispatcher and statement executor back-stop anything still unpositioned, so fatalError, division by zero, and plain .invalid diagnostics render with carets now too.

A failing flow now reads:

flow.swift:4:9: error: Data index 99 out of bounds (0..<2)
2 | let d = Data([1, 2])
3 | print("first:", d[0])
4 | let x = d[99]
  |         `- error: Data index 99 out of bounds (0..<2)

#16 — recorded failures that never become an Error

• public var currentCallOffset: Int? exposes the task-local while a builtin runs, so an assertion-shaped global registered via registerGlobal that records-and-continues (Swift Testing / XCTest semantics) can capture its call site at the moment it fires.
• public func renderSourceContext(at:message:) drives the existing caret renderer from a bare (offset, message) pair — out-of-range offsets clamp, no source in scope falls back to a plain error: line. renderRuntimeError is now a thin wrapper over it and also understands positioned ScriptErrors.

Tests

13 new tests in CallSitePositionTests cover: uncaught bridge RuntimeError / raw host error / generated-bridge subscript error rendering with file:line:col + caret, the boxed RuntimeError carrying the offset for hosts that dig it out, catchability + message unchanged (#13 regression), uncaught script throw, division by zero, fatalError, precise offsets not being overwritten, the record-and-continue assertion flow end to end, currentCallOffset nil outside evaluation, and both renderer fallbacks. Full suite: 554 tests / 61 suites green, probe conformance included.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8efeb4f329

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[odrobnik]

Reviewed this from the perspective of a downstream host — Loupe embeds the interpreter and registers its own bridges and globals, so it exercises most of the surface this PR touches. I built the PR branch against it and probed each claim below against gh/main (a771bb9), commit 1 (8efeb4f) and commit 2 (7228dba) separately, so every "changed" here is a measured difference rather than a reading of the diff.

The positioning itself is good

The core of #15/#16 works, and it needed no host changes at all to start paying off. A thrown bridge error now renders with the line and caret for free, and after capturing currentCallOffset in the assertion builtins, a recorded failure reads the same way as a thrown one:

check.swift FAILED after 3.8s
  ✗ check.swift:7:1: error: composer never appeared
     6 | // this identifier is wrong on purpose
     7 | XCTAssertTrue(composer.waitForExistence(timeout: 3), "composer never appeared")
       | `- error: composer never appeared
     9 | print("script continued past the failed expectation")

That is exactly what #16 was for. The offset also survives the shapes I expected to break it — inside a script func it blames the function body rather than the call site, inside a closure passed through map it blames the closure body, and a failing subexpression as an argument gets the caret on the operand. @TaskLocal was the right call.

Two things I'd change before merging, and one follow-up. Both of the first two are the same shape: the PR adds wrapping in two places and unwrapping in neither.

1. indirect case positioned silently breaks if case / catch RuntimeError.<case>

Introduced by commit 1. Because the dispatcher re-wraps any escaping error that doesn't already carry an offset, .invalid and .divisionByZero arrive as .positioned(.invalid(…), at: N) and stop matching. description and offset forward, so nothing signals it:

gh/main (a771bb9):
  PROBE-DIV     pattern-matched=true   shape=divisionByZero
  PROBE-GLOBAL  invalid-matched=true   shape=invalid(deliberate)
  PROBE-UNK     pattern-matched=true   shape=unknownIdentifier(nopeNotHere, at: 8)

PR head (7228dba), identical probe source:
  PROBE-DIV     pattern-matched=false  shape=OTHER(division by zero)
  PROBE-GLOBAL  invalid-matched=false  shape=OTHER(deliberate)
  PROBE-UNK     pattern-matched=true   shape=unknownIdentifier(nopeNotHere, at: 8)

Note the asymmetry in the third row: .unknownIdentifier keeps matching because it already carries an offset, so positioned(at:) no-ops on it. So which cases are wrapped depends on whether they happened to have an at: payload already — that's an accident, not a rule, and it's the kind of thing that's very hard to reason about from a host.

Two honest mitigations, both of which lower the severity a lot:

• Nothing is actually broken today. Grepping the repo, the CLI and Loupe turns up no consumer that pattern-matches a RuntimeError case — every site is as? RuntimeError reading .description/.offset. There are no tags, so there's no released contract.
• Exhaustive switches break loudly, at compile time, not silently. Only if case and catch <pattern> go quiet.

So this is about the shape you want to commit to, not a fire. Suggestion, in preference order:

1. Give .invalid and .divisionByZero an at: Int? payload, the way .unsupported and .unknownIdentifier already do, and drop .positioned entirely. Position stops being a thing that changes what the error is, and the "never double-wraps" invariant disappears with it.
2. If .positioned stays, it needs a public var underlying: RuntimeError that recursively strips it, documented as the thing hosts match on.

Worth a test either way — the 329 new lines never assert the case shape, which is why the whole suite stays green through the change.

2. Commit 2 boxes host errors from registerGlobal — and scripts can now swallow them

This one bit me for real. Loupe's XCTSkip is a registered global that throws a private ScriptSkipped sentinel, which the host catches to mark the run skipped rather than failed. After 7228dba, skipped runs started reporting as failed runs. Bridge bodies (.method, .computed, .init) were already boxed on main by #12 — this is specifically about registered globals and statics:

gh/main  a771bb9   [registerGlobal] swiftType=MySentinel   AS_MY_SENTINEL=YES
commit 1 8efeb4f   [registerGlobal] swiftType=MySentinel   AS_MY_SENTINEL=YES
commit 2 7228dba   [registerGlobal] swiftType=ScriptError  AS_MY_SENTINEL=no
                                    opaque(typeName: "Error", value: MySentinel)

.staticComputed and .staticMethod are newly boxed here too.

The part I'd flag hardest isn't the boxing, though — it's the second-order effect. Because the boxed error becomes a script-catchable value, script code can now catch and discard a host signal that previously always propagated:

7228dba  [registerGlobal-script-catch]  NO ERROR THROWN | scriptOutput=SCRIPT SWALLOWED IT
7228dba  [registerGlobal-script-try?]   NO ERROR THROWN | scriptOutput=SCRIPT try? SWALLOWED IT

On main and commit 1, both of those propagate. A host that uses an error for control flow — skip, abort, deadline, quota — can no longer rely on it reaching the host at all, because any script may wrap the call in try?. ScriptExit is the only pass-through a host can currently reach for, and it forces an exit code and discards the error.

Suggestion:

• A marker protocol the host can opt into, checked ahead of the generic catch in both wrappers: public protocol ScriptUncatchableError: Error {}. That gives hosts the affordance RuntimeError provides internally, which they can't otherwise use.

• Independently: public var hostError: (any Error)? { if case .opaque(_, let v) = value { return v as? any Error }; return nil } on ScriptError. The tell that this is missing is that the PR's own test writes the unwrap by hand at CallSitePositionTests.swift:78:

  guard case .opaque("Error", let payload) = script.value,
        let runtime = payload as? RuntimeError

  I had to write the same thing in Loupe to recover my sentinel. If the test needs it, hosts need it.

3. Follow-up: line-broken member chains blame the receiver line

Not a blocker — before this PR there was no location at all, so this is strictly better either way. But every node in a postfix chain shares the same start offset, so "innermost wins" can't discriminate, and the conventional XCUITest formatting that #15/#16 are aimed at is exactly the shape that suffers:

chain2.swift:5:1: error: no element matches windows["Main"].buttons["fault-here"]
3 | let app = XCUIApplication()
4 | 
5 | app
  | `- error: no element matches windows["Main"].buttons["fault-here"]
6 |     .windows["Main"]
7 |     .buttons["fault-here"]

The caret is on app; .tap() on line 8 is the call that failed and isn't even in the listing. Real Swift attributes to the member line here — a #line default argument on a 3-line chain reports the member's line, and a runtime trap in a broken chain reports the failing member's line, not the receiver's.

Fix looks small: anchor the stamp on the node's own token for the postfix forms — member.declName for MemberAccessExprSyntax (and for a FunctionCallExprSyntax whose callee is one), leftSquare for SubscriptCallExprSyntax — and leave positionAfterSkippingLeadingTrivia for everything else. Every chain in CallSitePositionTests.swift is on one line, so a line-broken case would be worth adding.

Things I checked that are not problems

Flagging these so they don't get chased:

• Cancellation. I suspected callingBuiltin boxing CancellationError let scripts swallow host cancellation. It reproduces, but it's not this PR: callingBridge had the byte-identical generic catch on main, and the interpreter has never had a checkCancellation anywhere, so a pure-compute script loop ignored cancellation on main too. If you want cancellation to be reliable that's its own issue, touching both wrappers.
• renderSourceContext resolving against the last-parsed tree. Also reproduces, also pre-existing — renderRuntimeError on main renders against currentSourceFile the same way, and holding an error across a second eval misattributes identically there. The new clamp is a no-op: SwiftSyntax's SourceLocationConverter already clamps out-of-range positions by documented contract, and main produces byte-identical output for offset 999999 and -5. At most the doc could add "render before evaluating another script".
• "Deferred rendering is untested" — false. recordedAssertionFailureNamesItsLine is exactly that flow. What's untested is only the cross-script variant.

Full suite passes on the PR head (569 tests). On the Loupe side, everything passes once the sentinel is unboxed, and #15 needed no host changes whatsoever.

[odrobnik]

All three addressed in 5966c2d — thank you for probing this against a real host; the Loupe sentinel regression and the case-matching asymmetry were both things the suite could never have caught from inside this repo.

1. Took your first option: .positioned is gone. .invalid and .divisionByZero now carry at: Int? payloads directly, same shape as .unsupported/.unknownIdentifier always had, so position no longer changes what the error is and if case .invalid(let msg, _) matches positioned and position-less errors alike. A static func invalid(_:) factory keeps the one-argument raise spelling working (enum cases can't take default arguments, so the issue-#15 spelling literally can't compile — the factory is the closest legal equivalent). Bare .divisionByZero patterns still match the payload case; only its two internal raise sites needed the explicit at:. The case shape is now pinned by tests (boxedRuntimeErrorCarriesThePositionToo matches .invalid(let message, let at) on a positioned error; positionedDivisionByZeroStillMatchesItsCase covers the other).

2. Both suggestions taken. public protocol ScriptUncatchableError: Error {} — conforming errors pass through callingBridge and callingBuiltin raw, exactly like ScriptExit, so a skip/deadline/quota sentinel always reaches the host and script catch/try? cannot swallow it. No position is attached to these — they're signals to the host, not diagnostics. And ScriptError.hostError unboxes the wrapped host error; the PR's own test now uses it instead of the hand-written .opaque unpack you flagged. Two new tests pin the sentinel surviving a script do/catch + try? from both a registered global and a bridge method. Note the marker also makes your sentinel immune on the bridge side, where it's been boxable since #12 — so Loupe can drop the unboxing workaround entirely rather than keeping it for .method bodies.

3. Did the chain-anchor fix now rather than as a follow-up — your sketch was right and it's ~15 lines. diagnosticAnchor(for:) anchors member accesses (and calls through them) on declName, subscripts on leftSquare, everything else at the node start. Your chain2.swift shape now renders:

chain2.swift:5:6: error: value of type 'Data' has no member 'subdata'
4 | d
5 |     .subdata(in: 0..<9)
  |      `- error: value of type 'Data' has no member 'subdata'

Single-line calls moved with it (w.mustExist() now blames col 3, the member, matching where swiftc puts member diagnostics) — existing tests updated accordingly, plus a line-broken chain test. renderSourceContext docs also gained the "render before evaluating another script" sentence. Cancellation I've left alone per your note — it predates this PR and deserves its own issue if you want reliable script cancellation.

Full suite: 561 tests green.

🤖 Addressed by Claude Code