Skip to content

fix: preserve TCO through error values#818

Open
He-Pin wants to merge 1 commit intodatabricks:masterfrom
He-Pin:fix/tco-error-value-directional-tests
Open

fix: preserve TCO through error values#818
He-Pin wants to merge 1 commit intodatabricks:masterfrom
He-Pin:fix/tco-error-value-directional-tests

Conversation

@He-Pin
Copy link
Copy Markdown
Contributor

@He-Pin He-Pin commented May 1, 2026

Rebased onto current master at b4c667d5 (Add lazy array views for large stdlib arrays (#814)).

Motivation

error value evaluates value immediately before throwing. That value is a tail position for auto-TCO and explicit tailstrict, but resolving a TailCall inside the error expression would re-enter the trampoline from the current function body and can bring back recursive stack growth for deep error f(n - 1) chains.

error value also must not be treated as an unconditional non-recursive exit. A function like local f() = error f() has no base case; auto-TCOing it would turn the existing max-stack diagnostic into an infinite trampoline loop.

Implementation

  • Evaluate tail-position error value with tail-call support and attach a delayed error-value continuation to the returned TailCall.
  • Store delayed result continuations directly on TailCall as primitive/ref fields: optional &&/|| validation plus optional error value throw.
  • Avoid linked continuation nodes, wrapper values, and per-wrapper check objects on deep recursive paths.
  • Keep the trampoline hot path simple: TailCall.resolve carries pending checks in local parameters and returns immediately when no final check is pending.
  • Preserve continuation order:
    • inner error value preempts any outer boolean check or outer error throw
    • inner boolean validation runs before an outer error throw
    • redundant outer boolean validations collapse after the innermost boolean check
  • Update auto-TCO exit analysis so Expr.Error recursively inspects its value expression instead of always counting as a non-recursive exit.

Performance notes

  • No allocation is introduced for delayed boolean/error continuations; the only state is stored on the existing TailCall.
  • The resolve loop is tail-recursive, monomorphic in the common path, and uses null/reference checks rather than allocating small continuation objects.
  • Final result validation is skipped entirely for tail calls without delayed checks.

Tests

New golden tests cover:

  • auto-TCO and explicit tailstrict through error value
  • lazy unused arguments through a 10000-deep recursive error f(n - 1, unused) chain
  • boolean-check-before-outer-error ordering
  • inner-error-before-outer-bool ordering
  • nested boolean continuation ordering
  • a non-tail lhs negative case
  • an error value expression whose non-recursive exit is inside the error value
  • a no-exit error f() case that must keep the max-stack diagnostic

Verification

  • ./mill --no-server 'sjsonnet.jvm[3.3.7].compile'
  • ./mill --no-server 'sjsonnet.jvm[2.13.18].compile'
  • ./mill --no-server 'sjsonnet.jvm[2.12.21].compile'
  • ./mill --no-server 'sjsonnet.jvm[3.3.7].test.testOnly' sjsonnet.FileTests sjsonnet.TailCallOptimizationTests
  • ./mill --no-server 'sjsonnet.js[2.13.18].compile' 'sjsonnet.wasm[2.13.18].compile'
  • ./mill --no-server 'sjsonnet.native[2.13.18].compile'
  • ./mill --no-server '__.checkFormat'
  • ./mill --no-server '_.jvm[_].__.test'
  • ./mill --no-server '_.js[_].__.test'
  • ./mill --no-server '_.wasm[_].__.test'
  • ./mill --no-server '_.native[_].__.test'
  • git diff --check

Rebase note

The previous CI failures were caused by the stale base: EncodingModule.scala had an unused UTF_8 import on that revision. Current master uses that import, so rebasing fixes those compile failures.

@He-Pin He-Pin marked this pull request as draft May 1, 2026 04:36
@He-Pin He-Pin force-pushed the fix/tco-error-value-directional-tests branch 2 times, most recently from 115eba9 to 6f1f2ee Compare May 2, 2026 17:39
@He-Pin He-Pin marked this pull request as ready for review May 2, 2026 17:43
Keep tail-position error values and boolean-result checks on the TailCall trampoline without allocating per-wrapper continuation objects.

This preserves correct error ordering while keeping the hot resolve loop GC-light and JIT-friendly.
@He-Pin He-Pin force-pushed the fix/tco-error-value-directional-tests branch from 6f1f2ee to e2abf73 Compare May 5, 2026 07:36
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.

1 participant