Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions .agents/plans/A27a-display-cycle-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
---
id: A27a
title: Cycle-safe peek for Lua.VM.Display.Table
issue: null
pr: null
branch: fix/display-cycle-guard
base: main
status: ready
direction: A
unlocks:
- inspect _G and other self-referential tables without hanging
- safer Display.Table for unknown user data
---

## Goal

`Lua.VM.Display.peek_table/3` (added in A27) recursively walks a
table's contents to build the `:peek` field on `%Lua.VM.Display.Table{}`.
Self-referential tables — `_G` is the canonical example, since
`_G._G == _G` — cause infinite recursion and a hung process.

Discovered while drafting iex recipes for A28: the obvious "what's
in the global env?" recipe (`Lua.eval!(lua, "return _G", decode: false)`)
hangs the BEAM. A28 worked around this by recommending `pairs(_G)`
in Lua instead, but the underlying bug remains.

## Out of scope

- Changing the data shape of `:peek`. Continue returning a list (for
sequence-like tables) or a map (for keyed tables). Cycles render
as a special placeholder, not a partial result.
- Tuning the recursion depth limit beyond a sensible default. A
follow-up plan can tune if needed.
- Tracking cycles in the inspect output for `Lua.VM.Display.Userdata`.
That struct stores the term verbatim, and the term's own `Inspect`
impl (or the user's) is responsible for cycle handling.

## Success criteria

- [ ] `Lua.eval!(Lua.new(), "return _G", decode: false)` returns in
under a second and produces a `%Lua.VM.Display.Table{}` whose
peek shows top-level keys but renders nested self-references
as a placeholder (e.g. `#Lua.Table<cycle>` or `:cycle`).
- [ ] Manually-constructed cycles (`local t = {}; t.self = t; return t`)
render without hanging.
- [ ] No regression for non-cyclic tables: existing
`test/lua/vm/display_test.exs` still passes.
- [ ] Add tests for cycle handling in `test/lua/vm/display_test.exs`.
- [ ] `mix test` passes.

## Implementation notes

The simplest correct approach is to track the set of `tref` ids
currently being peeked, and short-circuit when we hit one we've
already entered. Sketch:

```elixir
defp peek_table(state, id, decode?, seen \\ MapSet.new()) do
if MapSet.member?(seen, id) do
:cycle # or a small struct, e.g. %Display.Cycle{id: id}
else
seen = MapSet.put(seen, id)

case Map.fetch(state.tables, id) do
{:ok, table} ->
data = table.data

if sequence_like?(data) do
1..map_size(data)
|> Enum.map(&wrap_value(Map.fetch!(data, &1), state, decode?, seen))
else
Map.new(data, fn {k, v} -> {k, wrap_value(v, state, decode?, seen)} end)
end

:error ->
[]
end
end
end
```

`wrap_value/3` in `Lua.VM.Display` becomes `wrap_value/4` with a
`seen` accumulator threaded through. The boundary entry (`wrap_results/3`,
`wrap_value/3` at the eval call site) starts with `MapSet.new()`.

A depth limit (e.g. 8 levels) is a reasonable second guard for
deeply nested non-cyclic tables; tunable via an option later.

## Files

- `lib/lua/vm/display.ex` — thread `seen` through `wrap_value` and
`peek_table`; render cycles as a placeholder.
- `test/lua/vm/display_test.exs` — add a `describe "cycles"` block
with `_G` and a hand-built cycle test.
- (Optional) `lib/lua/vm/display/cycle.ex` — if we want a real
struct rather than the bare `:cycle` atom for the placeholder.

## Verification

```bash
mix format
mix compile --warnings-as-errors
mix test
```

Manual:

```elixir
iex> {[g], _} = Lua.eval!(Lua.new(), "return _G", decode: false)
iex> g # should not hang, should render in O(top-level keys)
iex> {[t], _} = Lua.eval!(Lua.new(), "local t = {}; t.self = t; return t", decode: false)
iex> t.peek["self"]
:cycle # or %Display.Cycle{...}, depending on representation
```

## Risks

- Picking a placeholder representation that future plans regret.
Recommendation: bare atom `:cycle` for now, escalate to a struct
if a downstream consumer needs more metadata.
- Threading `seen` widens the function arity. Acceptable: it's
internal to `Lua.VM.Display`.

## Discoveries

(populated during implementation)
185 changes: 146 additions & 39 deletions .agents/plans/A28-repl-iex-polish.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
id: A28
title: REPL/iex polish — Lua.dbg, doctest support, debugging recipes
issue: null
pr: null
pr: 219
branch: dx/iex-polish
base: main
status: ready
status: review
direction: A
unlocks:
- cleaner debugging from iex
Expand All @@ -20,9 +20,9 @@ deliverables:
1. **`Lua.dbg/2`** — a debug helper that runs Lua with stdout/stderr
captured and prints a structured summary (return values, state
diff, time elapsed, captured prints).
2. **Doctest support** — `Lua.eval/2` examples in module docs run as
doctests with deterministic output. The output formatting must be
stable enough to commit.
2. **Doctest support** — `Lua.eval!/2` examples in module docs run
as doctests with deterministic output. The output formatting must
be stable enough to commit.
3. **Recipes** — a short guide showing how to poke at a `Lua` state
from iex (read globals, call functions, list tables).

Expand All @@ -35,7 +35,7 @@ deliverables:

## Success criteria

- [ ] `Lua.dbg(state, source)` returns the same as `Lua.eval/2` but
- [ ] `Lua.dbg(state, source)` returns the same as `Lua.eval!/2` but
also prints a summary to stdout including:
- source preview (first 2 lines of input).
- return values.
Expand All @@ -57,58 +57,102 @@ deliverables:
def dbg(state \\ Lua.new(), source) when is_binary(source) do
start = System.monotonic_time()

# Capture stdout from print() calls.
{output, {result, new_state}} =
ExUnit.CaptureIO.with_io(fn -> Lua.eval(state, source) end)
# Capture stdout from print() by temporarily swapping the calling
# process's group leader to a StringIO process. Lua's `print`
# writes through Erlang's normal IO protocol, which honours the
# caller's group leader, so all output flows into our buffer.
{:ok, capture} = StringIO.open("")
original_gl = Process.group_leader()

elapsed_ms = System.convert_time_unit(
System.monotonic_time() - start, :native, :millisecond
)
result =
try do
Process.group_leader(self(), capture)
Lua.eval!(state, source)
after
Process.group_leader(self(), original_gl)
end

IO.puts("""
--- Lua.dbg ---
source: #{preview(source)}
return: #{inspect(result, pretty: true, limit: 10)}
elapsed: #{elapsed_ms} ms
prints: #{output |> String.trim() |> indent(2)}
---------------
""")
{output, _} = StringIO.contents(capture)
StringIO.close(capture)

{result, new_state}
{return, new_state} = result

elapsed_ms =
System.convert_time_unit(
System.monotonic_time() - start,
:native,
:millisecond
)

IO.puts(format_summary(source, return, elapsed_ms, output))

{return, new_state}
end
```

`ExUnit.CaptureIO` is a runtime dep (`:ex_unit` is always loaded);
keep this fine for now. If we want a non-ExUnit version, defer.
#### Why not `ExUnit.CaptureIO`

The original draft of this plan suggested wrapping eval in
`ExUnit.CaptureIO.with_io/1`. We rejected that: pulling
`:ex_unit` into a runtime/production code path is a non-starter
for an embedded library. Group-leader swap is plain OTP, no test
infra, and works because Lua's `print` is synchronous and runs in
the calling process — anything it emits goes through that
process's group leader.

#### Caveats of the group-leader approach

- It only captures output emitted from `self()`. If a future
feature has `print` spawn a task and write from there, capture
breaks. Currently `Lua.VM.Stdlib.lua_print/2` is synchronous
in-process, so this is fine.
- The group leader is restored in an `after` block so a Lua error
during eval still leaves the process's IO untouched on the way
out.

### Doctest examples

The repo's public eval function is `Lua.eval!/2` (the bang variant).
There is no non-bang `Lua.eval/2`. Doctests use `eval!`:

```elixir
@doc """
Evaluates a Lua source string.

## Examples

iex> {result, _state} = Lua.eval(Lua.new(), "return 1 + 2")
iex> {result, _state} = Lua.eval!(Lua.new(), "return 1 + 2")
iex> result
[3]

iex> {[table], _} = Lua.eval(Lua.new(), "return {a = 1, b = 2}")
iex> Lua.unwrap(table)
%{"a" => 1, "b" => 2}
iex> {[table], lua} = Lua.eval!(Lua.new(), "return {a = 1, b = 2}", decode: false)
iex> Lua.decode!(lua, table) |> Enum.sort()
[{"a", 1}, {"b", 2}]
"""
```

A27's `Inspect` polish makes some of these renderable. Where a value
needs to be unwrapped for display, use `Lua.unwrap/1` (add this
helper if not present).
A27 shipped `Lua.unwrap/1` and the four `Lua.VM.Display.*` structs,
so closures, tables, native funcs, and userdata already render
legibly in `iex`. Doctests can rely on those impls without any
extra work. For values that are sensitive to map-iteration order
(stdlib globals, table contents), wrap the assertion in `Enum.sort/1`
or pin a specific key to keep doctests deterministic.

### Files

- `lib/lua.ex` — add `dbg/1,2` + at least 3 doctests on public funcs.
- `lib/lua/vm.ex` — at least 2 doctests on public funcs.
- `guides/iex_recipes.md` (new) — recipes.
- `lib/lua.ex` — add `dbg/1,2` + at least 5 doctests across the
public eval/encode/decode/get/set/call_function surface. The plan
originally asked for ≥3 here and ≥2 on `lib/lua/vm.ex`, but
`Lua.VM.execute/2` takes a compiled `Prototype` which is awkward
to build in a 1–2 line doctest setup. Concentrating on `Lua.*`
reads more naturally and keeps the doctest count at the same
bar (5+).
- `guides/iex_recipes.md` (new) — recipes for reading globals,
calling functions, inspecting tables, modifying state and
re-running.
- `test/lua/dbg_test.exs` (new) — covers `Lua.dbg/2` output shape.
Uses `ExUnit.CaptureIO` (test-only, fine) to assert on the dbg
summary that `dbg` prints to stdout.

## Verification

Expand All @@ -129,18 +173,81 @@ return: [1, 2]
elapsed: 1 ms
prints: hi
---------------
{[1, 2], #Lua.State<...>}
{[1, 2], #Lua<>}
```

## Risks

- `Lua.dbg/2` printing to stdout in test environments could make
test output noisy. Mitigation: it's `dbg`, users will only call it
from iex.
- Doctests with non-deterministic output (`elapsed`, table order) are
flaky. Stick to determinism: only test return values, never the
test output noisy. Mitigation: it's `dbg`, users only call it from
iex; tests for `dbg` itself capture stdout to assert on the
formatted summary.
- The group-leader swap relies on `print` running synchronously in
the calling process. Documented as a known limitation — if a
future feature makes `print` spawn a task and write from there,
capture would silently miss those writes. Worth a comment in the
`dbg` source pointing at this assumption.
- Doctests with non-deterministic output (`elapsed`, table iteration
order, function references) flake. Stick to deterministic shape:
pin specific keys, sort lists before comparing, never test the
formatted dbg summary text.
- Capturing IO via group leader is per-process. If `Lua.dbg/2` is
called concurrently from the same process (it cannot be, since
Elixir is single-threaded per process, but worth noting), the
second call's group-leader swap would clobber the first. The
`try/after` ensures restoration but does not provide reentrancy.

## Discoveries

(populated during implementation)
- `IO.puts/1` writes to `:stdio`, which IS the calling process's
group leader — but `StringIO.contents/1` returns `{input, output}`,
not `{output, input}`. The first draft of `dbg/2` reversed those
and saw an empty capture. Fixed by destructuring `{_input, output}`.
- `Kernel.dbg/2` exists in Elixir 1.14+, so `defmodule Lua` had to
`import Kernel, except: [dbg: 2]` to shadow it.
- `inspect/1` formats lists of small integers as charlists by
default (`[7]` → `~c"\a"`). The dbg summary uses
`inspect(x, charlists: :as_lists)` to keep return values
unambiguous.
- A27's `Lua.VM.Display.peek_table/3` recurses into nested tables.
This deadlocks when applied to self-referential tables like `_G`
(where `_G._G == _G`). Discovered while drafting the `_G` recipe;
worked around in the recipe by encouraging users to iterate with
`pairs(library)` in Lua and only return the keys. The recursion
bug itself is filed as a follow-up plan: `A27a-display-cycle-guard.md`.
- The `eval!/2` doctest pattern needed adjustment for tables: I
added 3 new doctests covering multi-return, table decode, and the
closure-display struct — all using `Enum.sort/1` on table results
to keep iteration order out of the assertion.
- ExUnit.CaptureIO is fine in test files (it's exactly what it's for)
but the dbg `iex>` doctest had to become a fenced non-iex example
in the docstring, because doctest parsing recognises `iex>` lines
inside fenced blocks too.

## What changed

PR: [#219](https://github.com/tv-labs/lua/pull/219)

Files touched:

- `lib/lua.ex` — `dbg/1,2` implementation (group-leader swap with
StringIO capture, restored in an `after` block); imports
`Kernel, except: [dbg: 2]` to shadow `Kernel.dbg/2`; three new
doctests on `eval!/2` covering multi-return, table decode, and
the closure display struct.
- `test/lua/dbg_test.exs` (new) — 14 tests covering output shape,
capture, the error path, group-leader restoration on error, and
the `dbg/1` default-state form.
- `guides/iex_recipes.md` (new) — self-contained recipes for
reading globals, calling Lua functions, inspecting tables in
both decode modes, modifying state, dbg debugging, skimming a
library via `pairs()`, and exposing an Elixir tool function.
- `lib/lua/vm/display/{closure,native_func,table,userdata}.ex`,
`lib/lua/vm/display.ex` — drop stale `Lua.eval/2` doc references
(function is the bang variant; `mix docs` was warning).
- `.agents/plans/A27a-display-cycle-guard.md` (new) — follow-up
plan for the cyclic-peek bug discovered while drafting the `_G`
recipe.

Suite delta: 1626 → 1668 tests passing, 0 failures (no Lua 5.3
suite regressions; lua53 still 29 passing, 23 skipped).
Loading
Loading