Skip to content

Commit 71c5dbe

Browse files
ewowiclaude
andcommitted
A MoonLive script calls its own functions, and itself
A script's class can now define helper functions and call them, including calling itself. These are real calls: the callee gets its own frame when it runs, so one helper can call another and a function can recurse. Recursion is bounded at run time, so a runaway script leaves the picture wrong where it stopped instead of resetting the device. Perf: classic ESP32 1726144 bytes flash (+2732, +0.16% over 2f4c292), tick 2151us / 464fps unchanged; desktop tick 133us. crosshair.mlv (three functions, two calls per frame) costs 204->219us on the classic; a script with no local call emits no guard code and is byte-identical. Core - IrOp::CallScript, carrying the callee's FUNCTION NUMBER. An IR index was the first choice and is wrong: the spill pass shifts every index past its first inserted Reload, so the call named an op that no longer started a function. - IrProgram::swap() now swaps the function table. It did not, so the spill pass's remapped boundaries were discarded and the lowering opened a frame two ops early, mid-statement. - The recursion depth guard, in the CALLEE's prologue: one copy per function rather than one per call site, and none at all unless the script calls. A refusing callee returns, so there is no branch-around at the call site and no counter to restore across a call. The counter is one arena byte (kDepthSlot), zeroed per run so a refused frame cannot shrink the next frame's budget. - The guard is emitted AFTER the host arguments are parked. Both it and the epilogue reach the arena through the parked frame copy, and a refusing activation jumps straight to the epilogue. - An EMPTY function still balances the counter. Its whole body is the argument parking, so it never reached the flush point while its epilogue still decremented: two calls wrapped the byte to 255 and the next legal call was refused as too deep. Found by the Reviewer. - kAsmLabels/kAsmFixups (48/96) moved into core from three identical private copies. 16/32 was sized when a script was one routine; a class allocates a label per function, so crosshair.mlv failed with the generic "too large". Costs 640 bytes of compile-path stack (lowerWith 480 -> 1120 bytes). Light domain - moonlive/effects/crosshair.mlv: the worked example, two helpers and a tick. Platform - callLabel(Label) on all three assemblers, reusing the branch fixup machinery with a per-ISA discriminator, because a call's displacement is encoded differently from a branch's. - alignForEntry(): Xtensa requires a 4-byte-aligned `entry`. The toolchain rejects anything else outright ("unaligned entry instruction") and CALLn encodes its target in 4-byte units, so an unaligned callee is not expressible. Instructions are 2 or 3 bytes, so a function following another lands anywhere. This is what `.align 4` does in hand-written assembly. - A local call PASSES THE HOST ARGUMENTS ON. Each prologue parks buf/nLights/cpl/t/ctrls out of the argument registers into its own frame, so a bare call left the callee parking garbage and its first control read faulted at EXCVADDR 0x9. On Xtensa they go in a10..a14, since call8 rotates the window by 8. The contract now lives once in core. - The host backend's fixup kinds became a FixKind enum, matching its siblings; the dead condition-code packing went with it. Tests - Argument passing one and two calls deep, an empty function not consuming the budget, and unbounded recursion that must return AND be runnable again at full depth. Each control-checked by reverting its fix. - Per-ISA: every function in a class starts where a call can reach it. - The structural checker gained a calling class and a recursive one, so every prologue it walks carries the argument reload and the guard. Control-checked by dropping the window reserve from the frame calculation. - An assembler stack-budget tripwire, in table entries rather than host bytes. Docs/CI - moonlive/README.md: local calls, the recursion bound, and the declare-helpers-above-callers rule. - Plan-20260817 step 1 and verification items 1, 2, 5 marked done, with the three hardware-only traps recorded. Item 5 is done WITH A CAVEAT: a refused call is silent, where the plan asked for a reported error. That needs a diagnostic channel from the emitted block to the binding, which does not exist yet. Reviews - 18 findings (Fable). Fixed: the empty-function counter bug; a test named "calls itself" that contained no self-call (removed, the unbounded test pins it honestly); a comment describing the abandoned crashing design; CallScript.imm documented as an IR index in two places; the missing callLabel/alignForEntry in the assembler contract list; a redundant arena reload per prologue; the contract duplicated across three backends; dead fnSeen; host magic fixup numbers; a misplaced comment; two false comments; a duplicate test; the tripwire's loose arithmetic; the depth limit's off-by-one between code, pseudo-code and README; US spelling and em-dashes. Verified on all four boards (S3, S31, classic, P4). The classic separately ran a deliberately unbounded recursion for 110 seconds at 109fps without resetting. KPI baselines: 28 scenario values the gate rewrote under build load (up to +1048%) were reverted; the 7 kept are +1.0% to +6.7% and desktop-only. A quiet re-run reproduced the kept values and produced no over-10% deltas. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2f4c292 commit 71c5dbe

28 files changed

Lines changed: 870 additions & 130 deletions

docs/history/plans/Plan-20260817 - MoonLive scripts are classes.md

Lines changed: 102 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,9 @@ Steps 1 to 5 are the SHAPE: how a script is written. Steps 6 to 10 are the VOCAB
206206
say. The shape comes first so that every feature in the back half lands on finished ground rather
207207
than being retrofitted into a language still moving underneath it.
208208

209-
1. 🟡 **The `class` declaration and script functions, together.** PARTLY DONE: the class form
210-
ships and every script and test uses it; script-to-script CALLS and recursion are what remain,
211-
scoped at the end of this entry.
209+
1. **The `class` declaration and script functions, together.** Done: the class form ships, every
210+
script and test uses it, and a script now calls its own functions and itself. `crosshair.mlv` is
211+
the shipped example, verified on all four boards.
212212

213213
Originally: They are one change: making the
214214
declaration mandatory means there is no bare-statement-list form left, so the grammar's new top
@@ -219,24 +219,84 @@ than being retrofitted into a language still moving underneath it.
219219
Calls are real from the start, per the section above: a script calling its own function, and then
220220
calling it recursively, is the acceptance test.
221221

222-
**What recursion still needs, measured against the code as it now stands.** Per-function frames
223-
are DONE and each activation owns its frame, so the hard half is behind us. What is missing is
224-
that a script cannot yet call its own function at all: `parseCall` resolves a name against the
225-
BUILTIN table only, so `helper()` inside a class reports `unknown function`. Closing that is a
226-
defined piece of work rather than a subtlety:
227-
228-
- **A script-call IR op.** `IrOp::Call` carries an absolute host function pointer; a call to a
229-
script function is a different thing, a jump to a label inside this block.
230-
- **A relative call in each assembler.** `call(...)` takes a `const void*` host address on all
231-
three backends. A script-to-script call needs call-to-label with the same fixup machinery the
232-
branches already use, which is a new instruction per ISA (`call0`/`callx` forms on Xtensa,
233-
`jal` on RISC-V, `bl` on arm64).
234-
- **The depth guard.** A fixed render-task stack means unbounded recursion is a reset, which the
235-
robustness rule forbids, so this is a runtime counter that degrades visibly rather than a
236-
compile-time limit.
237-
238-
None of it is blocked, and none of it changes what is already verified: every shipped script,
239-
all three bindings and the moment model work on hardware without it.
222+
**Recursion is not a feature beside local calls; it IS local calls.** A recursive call is a local
223+
call whose target happens to be the running function, and the machine cannot tell the difference:
224+
it allocates a frame, jumps, returns. The proviso is that every value lives in the callee's own
225+
frame rather than a fixed location, which is exactly what the stack machine bought (the
226+
predecessor plan's table: "a fixed slot file cannot hold two activations" becomes "each activation
227+
gets its own frame"). So recursion is a TEST CASE for local calls, not separate work.
228+
229+
Two things are not free, and both are robustness rather than mechanism. A runaway recursion costs
230+
176 bytes of stack per activation on Xtensa (48 for the host-call area + 84 for 21 slots + 32 for
231+
the window reserve + alignment) against a 12 KB main-task stack, so it resets the device at
232+
roughly 64 deep: that needs a counter, and 32 is a generous limit at 46% of the budget. And on
233+
Xtensa each call8 rotates the register window, so past ~8 nested frames the hardware spills to the
234+
stack; that is correct and automatic, and the 32-byte reserve already accounts for where the
235+
spills land, so it costs memory traffic rather than correctness.
236+
237+
**What local calls took, as built.** All four pieces landed:
238+
239+
- **A script-call IR op.** `IrOp::CallScript`, carrying the callee's FUNCTION NUMBER. It first
240+
carried the callee's IR index, which the spill pass invalidates: every index past its first
241+
inserted Reload shifts, so the call named a position that no longer started a function. A
242+
function number survives any rewrite of the ops.
243+
- **A relative call in each assembler.** `callLabel(Label)` on all three, reusing the branch
244+
fixup machinery with a discriminator (`FixKind::Call` on Xtensa, `Jal` on RISC-V, kind 2 on
245+
arm64), because a call's displacement is encoded differently from a branch's.
246+
- **Function-entry alignment, which was not foreseen.** Xtensa requires a 4-byte-aligned `entry`
247+
: the toolchain rejects anything else outright ("unaligned entry instruction") and CALLn
248+
encodes its target in 4-byte units, so an unaligned callee is not expressible. Instructions are
249+
2 or 3 bytes, so a function following another lands anywhere. `alignForEntry()` pads before
250+
every prologue, which is what `.align 4` does in hand-written assembly. hpwit's `new-parser`
251+
hits the same wall and leaves it unhandled, so this is the missing piece rather than a
252+
workaround.
253+
- **The depth guard**, in the CALLEE's prologue rather than at each call site: one copy per
254+
function instead of one per call, emitted only when `hasScriptCall()`, so every shipped script
255+
carries none of it. A refusing callee returns, so there is no branch-around at the call site
256+
and no counter to restore across a call; the decrement lives in the one epilogue both paths
257+
take. Measured: 9 instructions ≈ 25 bytes per function, one arena byte, and ~5-10% on a script
258+
that calls (215-233µs vs 204µs for `crosshair.mlv` on the classic).
259+
260+
The counter is a byte in the CONTROL ARENA (`kDepthSlot`, above the system variables), not a
261+
C++ member: recursion happens entirely inside the emitted block with no C++ frame between
262+
activations, and every function already holds the arena pointer, so this costs one byte and no
263+
new argument. The host zeroes it before each run rather than trusting the block to unwind: a
264+
script that hit the limit would otherwise leak a level and shrink every later frame's budget.
265+
A stack-limit check (comparing `sp` against a bound) is the more canonical form and is cheaper
266+
still, but needs a per-platform stack-bound source; worth revisiting if the guard ever shows up
267+
in a profile.
268+
269+
- **A bigger label and fixup table.** `kMaxLabels`/`kMaxFixups` were 16/32, sized when a script
270+
was one routine, and each backend held its own private copy of both. A class allocates a label
271+
per function on top of its loop and store labels, so `crosshair.mlv` exhausted the table and
272+
failed with the generic "too large". Now `kAsmLabels`/`kAsmFixups` (48/96) in core, so the
273+
three backends cannot drift into disagreeing about which scripts compile.
274+
275+
**Cost: 640 bytes of STACK, and no flash.** Both tables are members of the assembler, which is
276+
a local in `lowerWith`, which runs on the render task. Measured on the classic ESP32 image,
277+
that frame went 480 -> 1120 bytes (4 per label, 8 per fixup), making it the largest on the
278+
compile chain: 144 + 288 + 576 + 1120 = 2128 nested, 17% of the 12 KB main task. Flash is
279+
unchanged, since these are stack arrays. Pinned by `the <ISA> assembler stays small enough to
280+
build on a render task`, a tripwire in entry counts rather than host bytes (the host's 64-bit
281+
size_t makes its Fixup 16 bytes against the device's 8, so sizeof here overstates the device).
282+
If a script ever needs more, the tables move to the heap beside the code buffer: which was
283+
moved off the stack for exactly this reason: rather than the constants going up again.
284+
285+
**Three traps, all found on hardware and none visible to the host suite:**
286+
287+
- `IrProgram::swap()` did not swap the function table, so the spill pass's remapped boundaries
288+
were discarded and the lowering opened a frame two ops early, mid-statement.
289+
- A local call passed NO arguments. Each function's prologue parks buf/nLights/cpl/t/ctrls out of
290+
the argument registers into its own frame, so a bare call left the callee parking garbage and
291+
its first control read faulted at `EXCVADDR 0x9`. On Xtensa the arguments go in a10..a14,
292+
because `call8` rotates the window by 8.
293+
- The depth guard must be emitted AFTER the host arguments are parked. Both it and the epilogue
294+
address the arena through the parked frame copy, and a refusing activation jumps straight to
295+
the epilogue: a guard placed first makes the refusal read a slot nothing wrote (SIGBUS).
296+
297+
The host backend cannot pin the argument-passing contract: its R0..R4 map onto the ABI argument
298+
registers and `bl` leaves them alone, so removing the fix fails no test there while crashing an
299+
S3. The boards are the only check for that class.
240300

241301
2.**Typed script-level members**, per *Where script-level state lives* above: a variable declared inside the
242302
class but outside any function lives in the arena, is visible in every function, is initialised
@@ -408,19 +468,36 @@ The governing risk is unchanged from the predecessor plan and is what shapes all
408468
host backend is EXECUTED by tests**, while the constraints that bite hardest are Xtensa's. Every step
409469
therefore needs a host test that proves the semantics and a bench run that proves the encoding.
410470

411-
1. **A script calling its own function, and then calling it recursively** (step 1). The recursion case
471+
1. **A script calling its own function, and then calling it recursively** (step 1). The recursion case
412472
is the one that proves a frame per activation, and it is the acceptance test for the step.
413-
2. **The frame contract, extended to script functions** (step 1). The structural checker must refuse a
473+
Done: `a function the script calls can light pixels and read the script's controls`, `arguments
474+
reach a function two calls deep`, and `a script function can call itself, each call keeping its
475+
own values` in unit_moonlive_compiler.cpp, plus `every function in a class starts where a call
476+
can reach it` per device backend. Each was control-checked by reverting its fix.
477+
2.**The frame contract, extended to script functions** (step 1). The structural checker must refuse a
414478
script function whose frame intrudes into the window-save reserve, and it must be shown FAILING on
415479
a deliberately wrong frame before it is trusted: the same control that caught the original bug.
480+
Done: the checker's case list gained a calling class and a recursive one, so every prologue it
481+
walks now carries the argument reload and the depth guard. Control-checked by dropping
482+
kWindowSaveReserve from the frame calculation, which fires the offset check as it should. The
483+
derived reserve resisted the first attempt to break it, which is the anti-drift design working:
484+
editing the static_assert alone changes nothing, because the value comes from the callx opcode.
416485
3. **A member written by one function and read by another**, and a member that survives across
417486
`tick()` calls (step 2). The second is what a stateful effect depends on and is not provable by
418487
inspection.
419488
4. **The same script at the host's real budget and a squeezed one renders identical pixels.** The
420489
predecessor plan's technique, still the only way the register work is testable off hardware, and
421490
every new construct has to keep passing it.
422-
5. **Recursion depth degrades visibly** (step 1): a script that recurses without bound reports an
423-
error and keeps the device rendering, rather than resetting it.
491+
5.**Recursion depth degrades visibly** (step 1): a script that recurses without bound keeps the
492+
device rendering rather than resetting it. Pinned by `a script that recurses without end keeps
493+
rendering instead of resetting`, which also re-runs the script to prove the counter unwinds: a
494+
leaked level per frame would silently shrink every later frame's budget. Verified on the classic
495+
ESP32 (the tightest stack): `forever.mlv` ran 110 seconds continuously at 109 fps, no reset.
496+
497+
NOT done as specified: the script does not REPORT an error. The refusal is silent, and what a
498+
user sees is the picture being wrong where the recursion bottomed out. Reporting it needs a
499+
channel from the emitted block back to the binding, which does not exist yet: worth having, and
500+
left for the step that gives scripts a diagnostic path.
424501
6. **An arena ceiling reports a compile error** (step 8), not a failed allocation at run time.
425502
7. **The bench, on all four boards**, after each step: S3 and classic (Xtensa), P4 and S31 (RISC-V),
426503
a scripted layout and a scripted effect. Exec-block sizes compared against the previous step, since

docs/metrics/repo-health.json

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,77 +1,77 @@
11
{
2-
"commit": "22ae7f50",
2+
"commit": "2f4c292f",
33
"flash": {
4-
"esp32": 1723408,
5-
"esp32p4-eth": 1613392,
4+
"esp32": 1726144,
5+
"esp32p4-eth": 1615872,
66
"esp32p4-eth-wifi": 1793760,
7-
"esp32s3-n16r8": 1762304,
7+
"esp32s3-n16r8": 1764928,
88
"esp32s3-n8r8": 1753232,
9-
"esp32s31": 2035424,
10-
"desktop": 1156776,
9+
"esp32s31": 2037728,
10+
"desktop": 1157176,
1111
"esp32-16mb": 1714608,
1212
"esp32-eth": 1324816,
1313
"esp32-wrover": 1765504,
1414
"qemu": 1318160
1515
},
1616
"perf": {
1717
"desktop": {
18-
"tick_us": 132,
19-
"fps": 7575
18+
"tick_us": 133,
19+
"fps": 7518
2020
},
2121
"esp32": {
2222
"tick_us": 2151,
2323
"fps": 464
2424
}
2525
},
2626
"loc": {
27-
"core": 18145,
27+
"core": 18392,
2828
"light": 24653,
29-
"platform": 13136,
29+
"platform": 13309,
3030
"ui": 6468,
31-
"test": 42421,
32-
"moondeck": 20830
31+
"test": 42534,
32+
"moondeck": 20835
3333
},
3434
"comments": {
3535
"core": {
36-
"lines": 6942,
37-
"ratio": 0.416
36+
"lines": 7099,
37+
"ratio": 0.42
3838
},
3939
"light": {
4040
"lines": 9613,
4141
"ratio": 0.431
4242
},
4343
"platform": {
44-
"lines": 4613,
45-
"ratio": 0.388
44+
"lines": 4727,
45+
"ratio": 0.392
4646
},
4747
"ui": {
4848
"lines": 1670,
4949
"ratio": 0.274
5050
},
5151
"test": {
52-
"lines": 7527,
52+
"lines": 7569,
5353
"ratio": 0.205
5454
},
5555
"moondeck": {
56-
"lines": 3353,
56+
"lines": 3357,
5757
"ratio": 0.184
5858
}
5959
},
6060
"tests": {
61-
"cases": 1352,
61+
"cases": 1357,
6262
"scenarios": 23
6363
},
6464
"docs": {
6565
"md_files": 179,
66-
"md_lines": 24972,
66+
"md_lines": 25049,
6767
"plans_files": 92,
6868
"backlog_lines": 3685,
6969
"lessons_lines": 503,
7070
"claude_md_lines": 135
7171
},
7272
"complexity": {
73-
"functions": 2531,
73+
"functions": 2538,
7474
"over_threshold": 158,
75-
"worst_ccn": 105
75+
"worst_ccn": 108
7676
}
7777
}

docs/metrics/repo-health.md

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,66 @@
11
# Repo health
22

3-
Measured at `22ae7f50`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.**
3+
Measured at `2f4c292f`. Generated by [`moondeck/check/repo_health.py`](../../moondeck/check/repo_health.py) on every KPI-gate run. **Do not edit by hand.**
44

55
Current state only; the trend is this file's git history (`git log -p docs/metrics/repo-health.md`). Nothing here fails a build: the numbers make growth visible, the judgment stays human.
66

77
## Firmware size
88

99
| Target | Flash |
1010
|---|---:|
11-
| desktop | 1,130 KB (0 KB) |
12-
| esp32 | 1,683 KB (+1 KB) ⚠ |
11+
| desktop | 1,130 KB (+0 KB) |
12+
| esp32 | 1,686 KB (+3 KB) ⚠ |
1313
| esp32-16mb | 1,674 KB |
1414
| esp32-eth | 1,294 KB |
1515
| esp32-wrover | 1,724 KB |
16-
| esp32p4-eth | 1,576 KB (+2 KB) ⚠ |
16+
| esp32p4-eth | 1,578 KB (+2 KB) ⚠ |
1717
| esp32p4-eth-wifi | 1,752 KB |
18-
| esp32s3-n16r8 | 1,721 KB (+1 KB) ⚠ |
18+
| esp32s3-n16r8 | 1,724 KB (+3 KB) ⚠ |
1919
| esp32s3-n8r8 | 1,712 KB |
20-
| esp32s31 | 1,988 KB (+2 KB) ⚠ |
20+
| esp32s31 | 1,990 KB (+2 KB) ⚠ |
2121
| qemu | 1,287 KB |
2222

2323
## Render performance
2424

2525
| Target | Tick | FPS |
2626
|---|---:|---:|
27-
| desktop | 132 µs (−31 µs) | 7,575 (+1,441) ✓ |
27+
| desktop | 133 µs (+1 µs) | 7,518 (−57) ⚠ |
2828
| esp32 | 2,151 µs | 464 |
2929

3030
## Code
3131

3232
| Area | Lines | Comments | Comment share |
3333
|---|---:|---:|---:|
34-
| core | 18,145 (+178) ⚠ | 6,942 | 41.6 % (+0.1 %) ⚠ |
35-
| light | 24,653 (+29) ⚠ | 9,613 | 43.1 % |
36-
| platform | 13,136 | 4,613 | 38.8 % |
34+
| core | 18,392 (+247) ⚠ | 7,099 | 42.0 % (+0.4 %) ⚠ |
35+
| light | 24,653 | 9,613 | 43.1 % |
36+
| platform | 13,309 (+173) ⚠ | 4,727 | 39.2 % (+0.4 %) ⚠ |
3737
| ui | 6,468 | 1,670 | 27.4 % |
38-
| test | 42,421 (+205) ⚠ | 7,527 | 20.5 % (+0.1 %) ⚠ |
39-
| moondeck | 20,830 | 3,353 | 18.4 % |
38+
| test | 42,534 (+113) ⚠ | 7,569 | 20.5 % |
39+
| moondeck | 20,835 (+5) ⚠ | 3,357 | 18.4 % |
4040

4141
## Tests
4242

4343
| Kind | Count |
4444
|---|---:|
45-
| unit cases | 1,352 (+6) ✓ |
45+
| unit cases | 1,357 (+5) ✓ |
4646
| scenarios | 23 |
4747

4848
## Complexity
4949

5050
| Metric | Value |
5151
|---|---:|
52-
| functions | 2,531 (+6) ✓ |
53-
| over threshold | 158 (+2) ⚠ |
54-
| worst CCN | 105 |
52+
| functions | 2,538 (+7) ✓ |
53+
| over threshold | 158 |
54+
| worst CCN | 108 (+3) ⚠ |
5555

5656
## Documentation
5757

5858
| Metric | Value |
5959
|---|---:|
6060
| markdown files | 179 |
61-
| markdown lines | 24,972 (+246) ⚠ |
61+
| markdown lines | 25,049 (+77) ⚠ |
6262
| plan files | 92 |
63-
| backlog lines | 3,685 (+31) ⚠ |
63+
| backlog lines | 3,685 |
6464
| lessons lines | 503 |
6565
| CLAUDE.md lines | 135 |
6666

moondeck/moonlive/check_encodings.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@
6060
# Control flow. `bltu`'s displacement is a single SIGNED byte (+/-127), the field that
6161
# truncated silently on a long loop body before branch relaxation.
6262
("callx8 a8", "e00800", "call: the windowed call"),
63+
# The script-to-script call. The offset field is patched later, so what is pinned here
64+
# is the OPCODE: a wrong low-six-bits would decode as an unrelated instruction, and the
65+
# first version of this used 0x25 in the wrong byte position. Assembled at a known pc
66+
# with a known target so the displacement is reproducible.
67+
("call8 . - 4", "a5ffff", "call: a function in this block, by label"),
6368
("retw.n", "1df0", "epilogue"),
6469
("l8ui a2, a11, 8", "220b08", "LoadCtrl: read a control byte from the arena"),
6570
("s8i a3, a12, 0", "324c00", "StoreElem: write one channel"),

0 commit comments

Comments
 (0)