Release the scope a skipped BEGIN block's loop pushed

`akbasic_parse_for()` and `akbasic_parse_do()` create their environment while
the line is *parsed*; whether to skip it is decided afterwards, when the line is
evaluated. So a loop inside a block that was not taken pushed a scope, its body
was skipped, and the `NEXT` or `LOOP` that would have popped it was skipped too.
Nothing else ever would.

At the top level that exhausted the pool after thirty-two skips. Inside a
routine it was far more confusing: the orphan sat between the routine and its
caller, so the `RETURN` after the block reported "RETURN outside the context of
GOSUB" from a routine that plainly *was* entered by a `GOSUB` -- naming the one
construct that was not at fault, which is why it cost an evening to find.

The skip now releases what parsing pushed.

**Narrower than it first looks.** Releasing on any skip breaks
tests/reference/language/flowcontrol/nestedforloopwaitingforcommand.bas: a
zero-iteration `FOR` skips its body by the same mechanism, and there the orphan
is load-bearing -- it absorbs the inner `NEXT` so the outer `NEXT` still finds
its own `FOR`. Releasing it turns that case into "NEXT outside the context of
FOR". So the release is conditional on the skip being a *block* skip, which is
decidable because nothing inside a skipped block ever runs to arm a `NEXT` wait.
Both halves are asserted side by side in tests/structure_verbs.c, the second one
citing the golden case that caught it.

The forty-skip case names its own step budget: a skipped line is not free, and
forty passes over a five-line block cost about 2700 steps against the shared
runner's 2000.

Chapter 18's trap 3 becomes history rather than a warning, and the note in Step
5 that called `GOTO`-guarded loops "not a style choice" now says why the shape
is kept anyway.

TODO.md section 9 item 2, struck.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-01 23:55:10 -04:00
parent f5a35b4af6
commit 5d33237eed
4 changed files with 183 additions and 24 deletions

21
TODO.md
View File

@@ -2511,9 +2511,24 @@ reduced against `build/basic`, the stdio build, unless it says otherwise.
`include/akbasic/value.h`. A test belongs in `tests/user_functions.c`: call a `DEF` ten `include/akbasic/value.h`. A test belongs in `tests/user_functions.c`: call a `DEF` ten
thousand times and then `DIM` an array. thousand times and then `DIM` an array.
2. **A `BEGIN` block that is skipped leaks a scope for every loop inside it**, because `FOR` 2. ~~**A `BEGIN` block that is skipped leaks a scope for every loop inside it**, because
and `DO` create their environment at *parse* time and the skip is decided at *evaluate* `FOR` and `DO` create their environment at *parse* time and the skip is decided at
time. *evaluate* time.~~ **Done** — the skip now releases what parsing pushed
(`akbasic_runtime_interpret()`, `src/runtime.c`), taking the second of the two routes
below. `tests/structure_verbs.c` covers both loop forms inside a routine and the
forty-skip top-level case.
**It is narrower than it looks, and the golden corpus is why.** The first attempt
released the scope on *any* skip, which broke
`tests/reference/language/flowcontrol/nestedforloopwaitingforcommand.bas`: a zero-
iteration `FOR` skips its body by the same mechanism, and there the orphan is
load-bearing -- it is what absorbs the inner `NEXT` so the outer one still finds its
`FOR`. Releasing it turns that case into "NEXT outside the context of FOR". So the
release is conditional on the skip being a *block* skip, which is testable because a
skipped block can never contain a live `NEXT` wait. Both halves are now asserted side by
side in `tests/structure_verbs.c`.
The original report:
```basic ```basic
N# = 0 N# = 0

View File

@@ -343,9 +343,10 @@ The game sets `DPLAY# = 1` or `DHUD# = 1` when something changes and never draws
directly. One consequence is visible and deliberate: **the score lags the bricks by one directly. One consequence is visible and deliberate: **the score lags the bricks by one
frame**, because the field goes first. At thirty frames a second nobody can see it. frame**, because the field goes first. At thirty frames a second nobody can see it.
Those are `LABEL`s and `GOTO`s rather than `BEGIN` blocks, and again that is not a style Those are `LABEL`s and `GOTO`s rather than `BEGIN` blocks, and when this was written that
choice a `RETURN` inside a block leaves the interpreter with no `GOSUB` to return from. was not a style choice: a loop inside a block that was skipped left the interpreter with
The last trap in this chapter is exactly that. no `GOSUB` to return from. That is fixed — see trap 3 below — and the shape is kept
because it costs nothing and reads the same.
## Step 6: Draw the lettering, because text has no colour ## Step 6: Draw the lettering, because text has no colour
@@ -672,7 +673,13 @@ NEXT I#
RETURN RETURN
``` ```
### 3. A skipped `BEGIN` block containing a loop breaks the enclosing `RETURN` ### 3. A skipped `BEGIN` block containing a loop — *fixed*
This one is here as history rather than as a warning. A loop inside a block that was not
taken used to leave a scope behind, and inside a routine the orphan sat between it and
its caller — so the `RETURN` after the block reported `RETURN outside the context of
GOSUB` from a routine that plainly *was* entered by a `GOSUB`. The error named the one
construct that was not at fault, which is why it cost an evening.
```basic ```basic
T# = 0 T# = 0
@@ -689,18 +696,13 @@ RETURN
``` ```
```output ```output
? 11 : RUNTIME ERROR RETURN outside the context of GOSUB CAME BACK
``` ```
The routine plainly *was* called by a `GOSUB`. `FOR` creates its environment when the `FOR` creates its environment when the line is **parsed** and the skip is decided when it
line is **parsed**, and the block skip is decided when it is **evaluated**, so a skipped is **evaluated**; the skip now releases what parsing pushed. **The listing still guards
loop pushes a scope that its skipped `NEXT` never pops — and the orphan sits between the its loops with `GOTO`**, which is where this chapter's own history shows: written that way
routine and its caller. Outside a routine the same thing exhausts the pool of 32 after because it had to be, and left that way because it works.
thirty-two skips instead.
Guard loops with `GOTO` rather than wrapping them in a block, which is what every draw
routine in this game does.
### 4. `SSHAPE` and `GSHAPE` ignore the subscript on a string array ### 4. `SSHAPE` and `GSHAPE` ignore the subscript on a string array

View File

@@ -816,6 +816,45 @@ akerr_ErrorContext *akbasic_runtime_interpret(akbasic_Runtime *obj, akbasic_ASTL
if ( akbasic_environment_is_waiting_for_any(obj->environment) ) { if ( akbasic_environment_is_waiting_for_any(obj->environment) ) {
if ( expr->leaftype != AKBASIC_LEAF_COMMAND || if ( expr->leaftype != AKBASIC_LEAF_COMMAND ||
!akbasic_environment_is_waiting_for(obj->environment, expr->identifier) ) { !akbasic_environment_is_waiting_for(obj->environment, expr->identifier) ) {
/*
* **A skipped loop has already pushed its scope, and this is where it
* comes back.**
*
* akbasic_parse_for() and akbasic_parse_do() create the environment
* while the line is *parsed*; whether to skip is decided here, after
* parsing. So a `FOR` inside a block that is not taken pushes a scope,
* its body is skipped, and the `NEXT` that would pop it is skipped
* too. Nothing else ever will.
*
* Left behind, thirty-two of them exhausted the pool -- and inside a
* routine it was much more confusing than that: the orphan sat between
* the routine and its caller, so the `RETURN` after the block reported
* "RETURN outside the context of GOSUB" from a routine that plainly
* *was* entered by a `GOSUB`. The error named the one construct that
* was not at fault. TODO.md section 9 item 2.
*
* `loopFirstLine` is what makes this safe: it is set by both parse
* handlers and by nothing else, so it identifies the scope this very
* line pushed rather than whatever happened to be on top.
*
* **Only a block skip**, which is what the `BEND` test is for. A
* zero-iteration `FOR` skips its body the same way, and there the
* orphan is load-bearing: `tests/reference/.../nestedforloopwaiting
* forcommand.bas` nests a loop inside one that runs zero times, and
* the inner scope is what absorbs the inner `NEXT` so the outer `NEXT`
* still finds its `FOR`. Popping it there turns that case into "NEXT
* outside the context of FOR". A `FOR` inside a skipped block cannot
* be in that position, because nothing in a skipped block ever runs to
* arm a `NEXT` wait in the first place.
*/
if ( expr->leaftype == AKBASIC_LEAF_COMMAND &&
obj->environment->loopFirstLine != 0 &&
obj->environment->waitingForCommand[0] == '\0' &&
akbasic_environment_is_waiting_for(obj->environment, "BEND") &&
(strcmp(expr->identifier, "FOR") == 0 ||
strcmp(expr->identifier, "DO") == 0) ) {
PASS(errctx, akbasic_runtime_prev_environment(obj));
}
*dest = &obj->staticTrueValue; *dest = &obj->staticTrueValue;
SUCCEED_RETURN(errctx); SUCCEED_RETURN(errctx);
} }

View File

@@ -17,19 +17,31 @@
#include "harness.h" #include "harness.h"
#include "testutil.h" #include "testutil.h"
/** @brief Run a program to completion in RUN mode, from a string. */ /** @brief Run a program to completion in RUN mode, under an explicit step budget. */
static akerr_ErrorContext AKERR_NOIGNORE *run_program(const char *source) static akerr_ErrorContext AKERR_NOIGNORE *run_program_bounded(const char *source, int64_t steps)
{ {
PREPARE_ERROR(errctx); PREPARE_ERROR(errctx);
PASS(errctx, harness_start(NULL)); PASS(errctx, harness_start(NULL));
PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, source)); PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, source));
PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN)); PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN));
/* PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, steps));
* Bounded. A defect in any of these verbs is a loop that never ends, and a SUCCEED_RETURN(errctx);
* suite that hangs tells you far less than one that fails. }
*/
PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 2000)); /**
* @brief Run a program to completion in RUN mode, from a string.
*
* Bounded. A defect in any of these verbs is a loop that never ends, and a suite
* that hangs tells you far less than one that fails. 2000 steps is generous for
* every case here but the forty-skip one, which names its own: a *skipped* line
* is not free, and forty passes over a five-line block cost about 2700.
*/
static akerr_ErrorContext AKERR_NOIGNORE *run_program(const char *source)
{
PREPARE_ERROR(errctx);
PASS(errctx, run_program_bounded(source, 2000));
SUCCEED_RETURN(errctx); SUCCEED_RETURN(errctx);
} }
@@ -273,6 +285,95 @@ static void test_loop_without_do(void)
harness_stop(); harness_stop();
} }
/**
* @brief A block that is not taken gives back the scope its loop pushed.
*
* `FOR` and `DO` create their environment while the line is *parsed*; the skip
* is decided when it is *evaluated*. So a loop inside a block that is not taken
* used to push a scope whose `NEXT` or `LOOP` was skipped along with it, and
* nothing ever popped it.
*
* Inside a routine that was much worse than a leak. The orphan sat between the
* routine and its caller, so the `RETURN` after the block reported "RETURN
* outside the context of GOSUB" -- from a routine that plainly *was* entered by
* a `GOSUB`, naming the one construct that was not at fault. TODO.md section 9
* item 2, found by writing a game against the interpreter.
*/
static void test_skipped_block_releases_its_loop_scope(void)
{
/* Both loop forms, because both parse handlers push. */
TEST_REQUIRE_OK(run_program("10 T# = 0\n"
"20 GOSUB DOIT\n"
"30 PRINT \"CAME BACK\"\n"
"40 END\n"
"50 LABEL DOIT\n"
"60 IF 1 = 0 THEN BEGIN\n"
"70 FOR I# = 0 TO 2\n"
"80 T# = T# + 1\n"
"90 NEXT I#\n"
"100 BEND\n"
"110 RETURN\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "CAME BACK\n");
harness_stop();
TEST_REQUIRE_OK(run_program("10 T# = 0\n"
"20 GOSUB DOIT\n"
"30 PRINT \"CAME BACK\"\n"
"40 END\n"
"50 LABEL DOIT\n"
"60 IF 1 = 0 THEN BEGIN\n"
"70 DO\n"
"80 T# = T# + 1\n"
"90 LOOP UNTIL T# > 2\n"
"100 BEND\n"
"110 RETURN\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "CAME BACK\n");
harness_stop();
/*
* At the top level it announced itself as pool exhaustion instead, after
* thirty-two skips -- AKBASIC_MAX_ENVIRONMENTS. Forty here, so a fix that
* merely raised the bound would not pass.
*/
TEST_REQUIRE_OK(run_program_bounded("10 N# = 0\n"
"20 LABEL AGAIN\n"
"30 N# = N# + 1\n"
"40 IF 1 = 0 THEN BEGIN\n"
"50 FOR I# = 0 TO 2\n"
"60 N# = N# + 1000\n"
"70 NEXT I#\n"
"80 BEND\n"
"90 IF N# < 40 THEN GOTO AGAIN\n"
"100 PRINT \"OK \" + N#\n", 6000));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "OK 40\n");
harness_stop();
}
/**
* @brief A zero-iteration loop still keeps the scope its inner loop pushed.
*
* The counter-case, and the reason the fix above tests for a `BEND` wait rather
* than for any wait at all. A `FOR` that runs zero times skips its body by the
* same mechanism, and *there* the orphan is load-bearing: it is what absorbs the
* inner `NEXT`, so the outer `NEXT` still finds its own `FOR`. Releasing it
* turns this into "NEXT outside the context of FOR".
*
* tests/reference/language/flowcontrol/nestedforloopwaitingforcommand.bas is the
* golden case that caught exactly that, and this is the same shape asserted
* where a reader of this file will see it.
*/
static void test_zero_iteration_loop_keeps_its_inner_scope(void)
{
TEST_REQUIRE_OK(run_program("10 FOR I# = 1 TO 0\n"
"20 FOR J# = 2 TO 4\n"
"30 PRINT \"THE BODY MUST NOT RUN\"\n"
"40 NEXT J#\n"
"50 NEXT I#\n"
"60 PRINT \"SUCCESS\"\n"));
TEST_REQUIRE_STR(HARNESS_OUTPUT, "SUCCESS\n");
harness_stop();
}
int main(void) int main(void)
{ {
test_do_while(); test_do_while();
@@ -287,5 +388,7 @@ int main(void)
test_end(); test_end();
test_nested_do(); test_nested_do();
test_loop_without_do(); test_loop_without_do();
test_skipped_block_releases_its_loop_scope();
test_zero_iteration_loop_keeps_its_inner_scope();
return akbasic_test_failures; return akbasic_test_failures;
} }