feat(vm): plumb per-call source lines through the compiled dispatcher#355
feat(vm): plumb per-call source lines through the compiled dispatcher#355davydog187 wants to merge 7 commits into
Conversation
… boundary `Lua.VM.ProtectedCall.error_value/1` had no clause for `Lua.VM.ArgumentError`, so it fell through to `Exception.message/1` — which calls `ErrorFormatter.format/3` and embeds ANSI escape codes plus the `at <source>:<line>:` header in the returned reason. PR #337 plugged this for the `:value`-bearing exceptions (`RuntimeError`, `TypeError`, `AssertionError`) but `ArgumentError` builds its message from individual fields and has no `:value` slot, so it was missed. Affected both `Lua.call_function/3` and `pcall`/`xpcall`: any stdlib bad-argument raise (e.g. `pairs("asdf")`) returned the rendered string instead of the bare §6.1 error value. Promote `ArgumentError.build_base/5` to a public `raw_message/1` and add a clause to `error_value/1` that returns it. Regression tests added for both protected-call boundaries. Fixes #342.
`Lua.VM.ProtectedCall` is `@moduledoc false`, so the autolink from `ArgumentError.raw_message/1`'s public doc tripped `mix docs --warnings-as-errors` on CI.
`Lua.call_function!/3` raises `Lua.RuntimeException` whose `:original`
field carries the underlying VM exception verbatim. That's the escape
hatch for Elixir callers who want to pattern-match on the structured
error (e.g. `%Lua.VM.ArgumentError{function_name: "pairs", arg_num: 1,
expected: "table", got: "string"}`) instead of parsing the terse §6.1
string that `call_function/3` returns.
The contract was already implemented (RuntimeException.exception/1 line
66 stores `original: error`) but wasn't pinned by a test, so a future
refactor that wrapped or stringified the underlying exception would have
silently regressed it.
The dispatcher's call opcodes now carry the source line of their call
site, baked in at encode time. `dispatcher_call_function/6` publishes
that line via `current_position/0` so raise sites inside stdlib (e.g.
`error()`'s §6.1 prefix, `pairs("asdf")`'s bad-argument check) attribute
to the right line instead of suppressing the prefix.
Previously the encoder stripped `:source_line` opcodes to avoid one
no-op dispatch per source line (~228k extra cycles for fib(25)) and the
dispatcher published `{nil, source}` at native-call boundaries to avoid
mis-attribution. This was the deferred "B5d-v2" work referenced by both
the bytecode-encoder comment and a relaxed `:compiled` assertion in
`pcall_error_value_test.exs`.
The fix:
- Encoder tracks `current_line` while walking instructions; `:source_line`
opcodes update it but stay stripped. `annotate_line/2` appends the
line as a final slot to `:call_one`/`:call_zero`/`:call_multi` opcode
tuples only. Other opcodes pass through unchanged.
- Dispatcher's three call-opcode patterns gain a `line` slot and pass it
to `Executor.dispatcher_call_function/6`.
- Executor publishes `{line, source}` at the native-call boundary and
uses the same line for the nil-call / non-function-call type errors.
- Nested bodies (inside `:test`, loops, generic_for) work because the
encoder seeds a fresh `current_line` per nested encode and the nested
bodies carry their own `:source_line` opcodes, so a call inside an
`if`/`for` body still gets its own line baked in.
Tests:
- Flipped the `:compiled` branches in `pcall_error_value_test.exs` and
`pcall_state_preservation_test.exs` that previously asserted
suppression — both engines now match `^test\.lua:\d+: boom$`.
- Added regressions in `call_function_error_value_test.exs` and
`bytecode_test.exs` pinning that calls in both outer and nested
positions carry the right line.
- Updated the `pcall(function() error("nope") end)` doctest in README
to reflect the §6.1 prefix that now flows through the compiled engine.
Benchmarks (manual, Apple M1 Pro):
- fib(30) main → 0.63 ips · branch → 0.73 ips (no regression)
- closures main → 1.37 K ips · branch → 1.40 K ips (no regression)
Hot path is unchanged: per-instruction lookup is gone, and call opcodes
gain one tuple slot read at the same dispatch step they already did.
Follow-up: line attribution for non-call dispatcher raise sites (binops,
indexing, concat) is still deferred — file as "B5d-v3" if needed.
🤖 Ultracode review — round 1Major
Minor
Nit
|
The compiled :generic_for path hard-coded line 0 when invoking the iterator, so a native iterator that raised mid-step (e.g. `for x in error, "deep", nil do end`) leaked `test.lua:0:` while the interpreter attributed `test.lua:2:` — a line-accuracy/parity break. - Bake the for-statement line into the :generic_for opcode via annotate_line/2, mirroring the call opcodes. - Thread it through the dispatcher's :generic_for dispatch and the :cps_generic_for resume marker into dispatcher_call_value/5, which passes it to call_value/5 so the native-iterator boundary publishes the right position. - Pin exact lines in the dual-engine error()-prefix tests (test.lua:2 / test.lua:4) instead of a loose :\d+: regex, and add compiled-vs-interpreted byte-equality tests covering both error() and a native-iterator raise. - Refresh the stale pcall_error_value moduledoc and the dispatcher encode_list/3 comment to match the plumbed-line reality.
🤖 Ultracode review — round 2[MAJOR]
[MINOR]
[NIT]
|
The bytecode encoder stripped `:source_line` opcodes into a rolling
`current_line` baked into call opcodes, but every nested-body
`encode_list` call hardcoded the seed to 0. A while/repeat condition
body carries no `:source_line` opcode of its own, so a native call
raising inside the condition (e.g. `while error('x') do end`) baked
`:0:` into the §6.1 error VALUE, diverging from the interpreter, which
threads the enclosing line through the body. Route body-carrying opcodes
(`:test`, `:numeric_for`, `:while_loop`, `:repeat_loop`, `:generic_for`)
through `encode/2` so their nested bodies inherit the in-scope
`current_line`.
Also pass `nil` (not the truthy literal `0`) from the dispatcher's
non-call raise bridges (binop / compare / unop / index / set / concat /
self) so `TypeError`/`RuntimeError.exception/1` fall back to
`current_position/0` instead of rendering a bogus `:0:` in the rich
render and `to_map`. The pcall error value was already byte-identical
across engines for these; this aligns the rich render too. Per-opcode
line baking for these sites remains deferred to B5d-v2.
Adds while/repeat condition-body regressions asserting both engines
agree with no `:0:` leak.
🤖 Ultracode review — round 3
|
…e after nested native catch
1 participant
Summary
The compiled (dispatcher) engine now attributes source lines to native-call raise sites. Previously the encoder stripped
:source_lineopcodes for perf anddispatcher_call_function/5published{nil, source}at native-call boundaries to avoid mis-attribution — sopairs(\"asdf\")errors anderror(\"boom\")calls in compiled chunks omitted the line in their §6.1 prefix.The fix bakes the line directly into the three call opcode tuples (
:call_one/:call_zero/:call_multi) at encode time. The dispatcher's hot path is unchanged (the same single tuple-read per call);dispatcher_call_function/6extracts the line and publishes it viacurrent_position/0so stdlib raise sites attribute correctly.This is the deferred "B5d-v2" work referenced by both
lib/lua/compiler/bytecode.ex:129and the relaxed:compiledassertions inpcall_error_value_test.exs.After
(Line
2is now populated where it was previously omitted.)Design
line_mapon the prototype. Avoids the nested-body problem (calls insideif/forbodies sit in their own PC-space) and keeps tuple-read cost constant.bytecode.ex):encode_list/3trackscurrent_linewhile walking instructions.:source_lineupdates the line but stays stripped.annotate_line/2appends the line to call opcodes only — other opcodes pass through unchanged.dispatcher.ex): each call-opcode pattern gains alineslot, passes it toExecutor.dispatcher_call_function/6.executor.ex): publishes{line, source}at the native-call boundary; uses the same line for nil-call / non-function-call type errors that previously hardcodedline: 0.Out of scope
Line attribution for non-call dispatcher raise sites (binops, indexing, concat) is still deferred. Same gap, much wider surface — follow-up PR if needed.
Benchmarks (manual, Apple M1 Pro)
Hot path unchanged: same tuple-read per call.
Test plan
:compiledbranch suppression inpcall_error_value_test.exsandpcall_state_preservation_test.exs— both engines now match^test\.lua:\d+: boom$.call_function_error_value_test.exsassertingcall_function!/3rich render containsregression.lua:2:for both outer and nested call positions.pcall(function() error(\"nope\") end)— the §6.1 prefix now flows through the compiled engine.mix test— full suite green (2263 passed, 19 skipped, +4 new tests).mix docs --warnings-as-errorsclean.mix formatclean.