Give each DEF call its own environment, so recursion returns
The function's environment was owned by the funcdef and re-initialised on every
call, which made a function not re-entrant and cost two silent defects:
DEF DBL(N#) = N# * 2
PRINT DBL(10) + DBL(1) was 4, should be 22
The result was a pointer into the funcdef's own environment, so the second call
overwrote the first before the operator saw it -- both operands became the last
call's answer. Two *different* functions in one expression were fine, which is
most of why it was invisible.
DEF FACT(N#)
IF N# <= 1 THEN RETURN 1
RETURN N# * FACT(N# - 1)
PRINT FACT(5) never returned
The recursive call re-initialised the environment the outer call was still
using, so the loop waiting for control to come back could not see it. No error,
no bound, no diagnostic -- the one place in this interpreter that looped forever
rather than raising.
A call takes an environment from the pool now, exactly as GOSUB does. The result
is copied into a caller-scope scratch before that environment goes back, because
handing back a pointer into the callee is what made two calls collide and would
now be a pointer into a released slot as well. RETURN parks its result on the
*parent* rather than on the environment it is about to release, so nothing reads
a freed slot to find it.
Recursion depth answers to AKBASIC_MAX_ENVIRONMENTS like every other nesting, so
too deep is "Environment pool exhausted" -- a diagnosis where there was none.
akbasic_FunctionDef.environment goes with it, as dead state.
One thing this exposed but did not cause, measured against a stashed build and
recorded rather than fixed: a statement containing a failed multi-line DEF call
still completes and prints a junk value. It is visible more often now only
because runaway recursion reaches it where it used to hang.
tests/language/functions/recursion.bas deliberately does not pin that answer.
Chapter 16 loses its "walk a list with a loop, not a recursive DEF" caveat and
gains the one that is still true: a function cannot take a structure parameter
yet, so it reaches a record by name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
58
TODO.md
58
TODO.md
@@ -1573,6 +1573,34 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
listings read it there. Same known-failing test as item 19; the fix is a scoping decision
|
||||
about where a loop counter is created, not an ordering one.
|
||||
|
||||
### Found while fixing the DEF recursion hang
|
||||
|
||||
25. **A statement containing a failed multi-line `DEF` call still completes, and prints a
|
||||
junk value.** The call's error is reported and the run stops, but the *enclosing*
|
||||
statement carries on with whatever `*dest` was left holding:
|
||||
|
||||
```
|
||||
10 DEF BAD(N#)
|
||||
20 RETURN N# / 0
|
||||
30 PRINT 99
|
||||
40 PRINT BAD(1)
|
||||
```
|
||||
|
||||
prints `99`, the division-by-zero line, and then
|
||||
`(UNDEFINED STRING REPRESENTATION FOR 0)`.
|
||||
|
||||
**Pre-existing, and measured as such**: identical before and after the re-entrancy
|
||||
fix, on a build stashed back to compare. It is visible more often now only because
|
||||
runaway recursion reaches it where it used to hang instead.
|
||||
|
||||
The cause is that `akbasic_runtime_process_line_run()` swallows a script's error by
|
||||
design -- goal 3 -- so the loop inside `akbasic_runtime_user_function()` sees the run
|
||||
end normally and hands back a value nobody should use. The fix is for the multi-line
|
||||
path to notice the run did not return through `RETURN` and refuse rather than produce
|
||||
a value; it is not done here because it is a decision about what a failed call
|
||||
*evaluates to*, and `tests/language/functions/recursion.bas` deliberately does not
|
||||
pin the current answer.
|
||||
|
||||
### Found while building structures
|
||||
|
||||
23. ~~**A host could not run one script twice.**~~ **Fixed.**
|
||||
@@ -1591,6 +1619,30 @@ the reference's semantics reproduced faithfully; the third is the port's own.
|
||||
index a field already recorded keeps pointing at the same type.
|
||||
`tests/hoststruct.c` pins it.
|
||||
|
||||
26. ~~**A `DEF` call was not re-entrant, which cost a wrong answer and a hang.**~~
|
||||
**Fixed.** The function's environment was owned by the funcdef and re-initialised on
|
||||
every call, so a second call trampled the first:
|
||||
|
||||
- **Two calls in one expression aliased one slot.** `DBL(10) + DBL(1)` was 4 rather
|
||||
than 22 -- both operands became the last call's answer, because the result was a
|
||||
pointer into the funcdef's own environment. Two *different* functions were fine,
|
||||
which is most of why it was invisible. Same severity class as §6 item 12, and the
|
||||
same shape: a silent wrong answer from a two-line program.
|
||||
- **Recursion never came back.** The recursive call re-initialised the environment the
|
||||
outer call was still using, so the loop waiting for control could not see it return.
|
||||
No error, no bound, no diagnostic -- the one place in this interpreter that looped
|
||||
forever instead of raising.
|
||||
|
||||
A call takes an environment from the pool now, exactly as `GOSUB` does, and the result
|
||||
is copied into a caller-scope scratch before that environment goes back. `RETURN`
|
||||
parks its result on the *parent* rather than on the environment about to be released,
|
||||
so nothing reads a freed slot. Recursion depth answers to `AKBASIC_MAX_ENVIRONMENTS`
|
||||
like every other nesting, so too deep is `Environment pool exhausted` -- a diagnosis
|
||||
where there was none.
|
||||
|
||||
`akbasic_FunctionDef.environment` is gone with it, as dead state.
|
||||
`tests/user_functions.c` and `tests/language/functions/recursion.bas`.
|
||||
|
||||
### Found while auditing `akbasic_value_clone()` for the structures work
|
||||
|
||||
22. ~~**The numeric operators' `else` was a catch-all, not a float branch.**~~ **Fixed.**
|
||||
@@ -1712,12 +1764,12 @@ requirement; exactly one case has diverged on purpose since, and
|
||||
|
||||
| Gate | Result |
|
||||
|---|---|
|
||||
| `ctest` | 104/104 — 41 reference golden cases, 20 local ones, 39 unit tests, 3 embedding examples, and `docs_examples` |
|
||||
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 104/104 on a machine with a display; 103 passed + 1 skipped headless. The same set minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends`, `akgl_frontend`, `docs_screenshots` and `akgl_typing` — the last of which is the skip, and the `akgl_build` CI job is where it skips |
|
||||
| `ctest` | 106/106 — 41 reference golden cases, 20 local ones, 39 unit tests, 3 embedding examples, and `docs_examples` |
|
||||
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 106/106 on a machine with a display; 105 passed + 1 skipped headless. The same set minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends`, `akgl_frontend`, `docs_screenshots` and `akgl_typing` — the last of which is the skip, and the `akgl_build` CI job is where it skips |
|
||||
| `docs_examples` | Every fenced block in `README.md`, `MAINTENANCE.md` and `docs/` executed and byte-compared: 49 programs, 9 transcripts, 57 output comparisons, 2 excerpts, 2 shell blocks and 8 figures in the default build; 65 programs and 56 output comparisons in the AKGL one. The C-snippet count reads 0 when the harness is run by hand without `--cflags-file`; CTest passes it. `MAINTENANCE.md` documents the fence-tag convention |
|
||||
| `docs_screenshots` | 8/8 figures re-rendered and byte-identical to the checked-in PNGs. AKGL build only — rendering a picture needs the SDL half |
|
||||
| Golden corpus | 41/41 byte-exact from `tests/reference/` — **and 41/41 again through the SDL binary**, which is most of what proves the frontend changes no output |
|
||||
| ASan + UBSan | 104/104 |
|
||||
| ASan + UBSan | 106/106 |
|
||||
| Line coverage | 94.8% (6648/7013) — above the 90% gate |
|
||||
| Function coverage | 98.4% (441/448) |
|
||||
| Warnings | none under `-Wall -Wextra` |
|
||||
|
||||
Reference in New Issue
Block a user