Skip to content

fix(vm): pass raw error values through pcall and add §6.1 position prefix#335

Merged
davydog187 merged 4 commits into
mainfrom
fix/pcall-error-value-passthrough
Jun 4, 2026
Merged

fix(vm): pass raw error values through pcall and add §6.1 position prefix#335
davydog187 merged 4 commits into
mainfrom
fix/pcall-error-value-passthrough

Conversation

@davydog187
Copy link
Copy Markdown
Contributor

pcall/xpcall pass the raw Lua error value through; error() adds the §6.1 position prefix

Plan: .agents/plans/A48-pcall-error-value-passthrough.md
Closes #334

Goal

Per Lua 5.3 §6.1, error(value) raises an arbitrary Lua value and pcall returns it as-is as its second result; xpcall hands it to the message handler untouched. Previously extract_error_message/1 stringified every non-string value (error({code = 1})"table: 0x..."). Additionally, string messages now gain the reference source:line: position prefix (error("boom")"file:1: boom"), suppressed by level == 0 — all without disturbing host-facing error rendering.

How

  • lua_error captures the level argument and builds the prefixed view into a new Lua-facing-only :lua_value field on RuntimeError; :value stays raw, so message/to_map/stringify (and the (error object is a TYPE value) host formatting) are provably untouched — no doubled location.
  • extract_error_message/1error_value/1: lua_value → raw value (matched on key presence, so error(nil)/error(false) pass through) → Exception.message fallback for ArgumentError/plain Elixir exceptions.
  • Dispatcher parity: native calls inside compiled closures publish a {nil, source} position, so the prefix is suppressed rather than attributed to a stale outer line (the dispatcher strips per-instruction lines at encode time). The interpreter attributes correctly.

Success criteria

  • New dual-engine matrix test/lua/vm/pcall_error_value_test.exs (26 cases, both engines): table/number/boolean/false/nil passthrough, prefix shape + source-first ordering, level-0 suppression, xpcall raw handler value, erroring-handler fallback to the original raw value, TypeError/bad-argument regression guards — all green (red-first, commit 424c65a)
  • mix test: 2234 passed, 0 failures; host-message canaries unchanged (error_gallery_test.exs, integration_test.exs:1382, lua_test.exs:940, runtime_exception_test.exs:34-35)
  • mix test --only lua53: 17 passed / 12 skipped — identical to main baseline
  • mix format + mix compile --warnings-as-errors clean (mix credo is not a task in this repo — see Discoveries)

Changes

CHANGELOG.md                                   | 13 ++++
lib/lua/vm/executor.ex                         | 17 ++++-
lib/lua/vm/runtime_error.ex                    | 11 +++-
lib/lua/vm/stdlib.ex                           | 69 ++++++++++++++++-----
test/lua/vm/pcall_error_value_test.exs         | 285 +++++++++++++++++++++
test/lua/vm/pcall_state_preservation_test.exs  | 24 +++++--

Discoveries

  • The position-suppression edit means native raises (assert, bad-argument) inside nested compiled closures now render a source-only host header (at <eval>:) where they previously had no location header at all ({nil, nil} position). Verified via before/after canary: strictly added information, no wrong line, no test pinned the old bytes; ErrorFormatter.format_location(source, nil) is an explicit supported clause.
  • The plan's verification listed mix credo, but credo is not a dependency of this repo.
  • Follow-up needed (issue not yet filed): a dispatcher native-call line bridge so error() inside compiled closures gets a real line (collapsing the engine-conditional prefix assertions) and error(msg, 2) caller attribution becomes implementable.

Out of scope (intentional)

  • error() level >= 2 (needs per-frame line numbers; treated as level 1)
  • Position prefix on internal raises (TypeError/ArgumentError — host ErrorFormatter already renders location; many tests pin exact strings)
  • Correct compiled-closure prefix line under the dispatcher (suppressed instead; see Discoveries)
  • Unskipping test/lua53_tests/errors.lua (blocked on an unrelated load() parse-error-prefix mismatch; its level-0 and error()-nil cases are reproduced in the new unit test)
  • Any change to host-facing rendering

Verification

mix format                          # clean
mix compile --warnings-as-errors    # clean
mix test test/lua/vm/pcall_error_value_test.exs   # 26 passed
mix test                            # 2234 passed, 0 failures, 19 skipped
mix test --only lua53               # 17 passed, 12 skipped (== main baseline)

Red matrix over both engines: pcall/xpcall must return the raised Lua
value as-is (table, number, boolean, nil), error() must prefix string
messages with source:line: (level 0 suppresses), and internal
TypeError/ArgumentError messages must stay strings.

Plan: A48
…efix

error(value) now raises the value verbatim and pcall/xpcall hand it back
to Lua untouched (table, number, boolean, nil) instead of stringifying.
String messages gain the source:line: prefix per Lua 5.3 §6.1, honoring
level 0 suppression; the prefixed view rides a new Lua-facing :lua_value
field on RuntimeError so host rendering keeps reading the raw :value and
never doubles the location. Under the dispatcher, native calls inside
compiled closures publish a {nil, source} position so the prefix is
omitted rather than attributed to a stale line.

Plan: A48
Closes #334
Comment thread lib/lua/vm/executor.ex
# sites that read `current_position/0` (e.g. `error()`'s §6.1 position
# prefix) then omit the line rather than attribute a wrong one. Restored
# after the call so nested invocations don't leak.
prev_pos = Process.get(@position_key, @unset)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process dictionary is really a hack. I think this was from a previous design choice made for performance that we need to revisit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will evaluate this later

@davydog187 davydog187 merged commit 9c3f2b2 into main Jun 4, 2026
5 checks passed
@davydog187 davydog187 deleted the fix/pcall-error-value-passthrough branch June 4, 2026 19:42
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.

pcall returns stringified error objects instead of passing the error value through

1 participant