Fix generator teardown leaks, add RETURN-in-GEN and LOOP conditions on DO EACH
Review findings and follow-ups from PR #61 review: - runtime_generator.c: akbasic_runtime_release_generator() now releases the forGeneratorEnv of every scope it walks through. Abandoning a generator that was itself suspended inside a FOR EACH over another generator stranded the inner generator's pool slot; a loop doing so exhausted the twelve-slot pool and died far from the cause. - runtime.c/runtime.h: new akbasic_runtime_unwind_to_environment(), the shared teardown for the error unwinds in pump_generator() and call_function() -- both previously bare prev_environment() loops with the same suspended-generator blindness. - runtime_commands.c: bare RETURN standing in a GEN's own frame ends the generator exactly as END GEN does -- a GEN is a function at heart. RETURN with a value there is refused (values leave a GEN only through EMIT). The no-frame error message now says "GOSUB, DEF, or GEN". - runtime_structure.c: LOOP WHILE/UNTIL composes with DO EACH -- checked after each trip with the loop variable still holding that trip's value; a condition that stops the loop abandons the generator exactly as EXIT does. Previously the condition was silently ignored, while the verb reference documented it as working. - parser_commands.c: trailing tokens after the generator call on a FOR EACH/DO EACH line are refused at parse. Previously they sat unparsed and blew up only after the loop completed, when the parent scope resumed the line mid-statement -- an error at the loop's end pointing at its start. - tests/generators.c: pool-exhaustion tests for the nested-abandonment and LOOP-condition paths, RETURN semantics tests, and a direct test of the unwind primitive. Three new golden pairs cover RETURN, LOOP conditions and the misplaced-condition parse error. - docs: RETURN and LOOP-condition semantics in 04-control-flow.md and 11-verb-reference.md; corrected the self-recursion analogy (functions are re-entrant here). TODO.md 1.10 records the generator design decisions the code comments were already citing, plus the zero-arg parameter-list limitation. MAINTENANCE.md gains the abandoned-generators invariant those comments also cited. Co-Authored-By: Andrew Kesterson <andrew@aklabs.net> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -337,6 +337,19 @@ akerr_ErrorContext *akbasic_parse_do(akbasic_Parser *parser, akbasic_ASTLeaf **d
|
||||
|
||||
PASS(errctx, parse_each_clause(parser, &var, &callexpr));
|
||||
|
||||
/*
|
||||
* Same guard as akbasic_parse_for()'s EACH branch, with the likely
|
||||
* mistake named: a condition belongs on the LOOP, where it is
|
||||
* checked against each emitted value, not here on the DO.
|
||||
*/
|
||||
peeked = akbasic_parser_peek(parser);
|
||||
FAIL_NONZERO_RETURN(errctx,
|
||||
(peeked != NULL &&
|
||||
peeked->tokentype != AKBASIC_TOK_UNDEFINED &&
|
||||
peeked->tokentype != AKBASIC_TOK_COLON),
|
||||
AKBASIC_ERR_SYNTAX,
|
||||
"DO EACH takes its WHILE/UNTIL on the LOOP, and nothing else here");
|
||||
|
||||
newenv->isDoLoop = true;
|
||||
newenv->isEachLoop = true;
|
||||
newenv->loopFirstLine = firstline;
|
||||
@@ -1098,6 +1111,21 @@ akerr_ErrorContext *akbasic_parse_for(akbasic_Parser *parser, akbasic_ASTLeaf **
|
||||
|
||||
PASS(errctx, parse_each_clause(parser, &var, &callexpr));
|
||||
|
||||
/*
|
||||
* Nothing may follow the generator call but another statement.
|
||||
* Without this, a stray clause sits unparsed on the line and only
|
||||
* blows up after the whole loop has run, when the parent scope
|
||||
* resumes the line mid-statement -- an error at the loop's end
|
||||
* pointing at its beginning.
|
||||
*/
|
||||
peeked = akbasic_parser_peek(parser);
|
||||
FAIL_NONZERO_RETURN(errctx,
|
||||
(peeked != NULL &&
|
||||
peeked->tokentype != AKBASIC_TOK_UNDEFINED &&
|
||||
peeked->tokentype != AKBASIC_TOK_COLON),
|
||||
AKBASIC_ERR_SYNTAX,
|
||||
"FOR EACH takes nothing after the generator call");
|
||||
|
||||
newenv->isEachLoop = true;
|
||||
newenv->loopFirstLine = firstline;
|
||||
/*
|
||||
|
||||
@@ -208,6 +208,38 @@ akerr_ErrorContext *akbasic_runtime_prev_environment(akbasic_Runtime *obj)
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
akerr_ErrorContext *akbasic_runtime_unwind_to_environment(akbasic_Runtime *obj, akbasic_Environment *target)
|
||||
{
|
||||
PREPARE_ERROR(errctx);
|
||||
akbasic_Environment *popped = NULL;
|
||||
|
||||
FAIL_ZERO_RETURN(errctx, (obj != NULL && target != NULL), AKERR_NULLPOINTER,
|
||||
"NULL argument in unwind_to_environment");
|
||||
/*
|
||||
* Stops early at the root rather than failing on it: every caller is an
|
||||
* error-unwind path, where "release whatever there is" beats raising a
|
||||
* second failure on top of the one being cleaned up after.
|
||||
*/
|
||||
while ( obj->environment != target && obj->environment->parent != NULL ) {
|
||||
popped = obj->environment;
|
||||
obj->environment = popped->parent;
|
||||
/*
|
||||
* An EACH loop scope on its way out takes its suspended generator with
|
||||
* it -- the generator is a *child* of the scope, off the parent chain,
|
||||
* and this walk is the only thing that will ever see it again. Guarded
|
||||
* on `used` because a generator that was being pumped when the failure
|
||||
* hit is *on* the chain being unwound, already released by the time
|
||||
* the walk reaches the loop scope that references it.
|
||||
*/
|
||||
if ( popped->forGeneratorEnv != NULL && popped->forGeneratorEnv->used ) {
|
||||
PASS(errctx, akbasic_runtime_release_generator(obj, popped->forGeneratorEnv));
|
||||
}
|
||||
popped->forGeneratorEnv = NULL;
|
||||
PASS(errctx, akbasic_runtime_release_environment(obj, popped));
|
||||
}
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------- lifecycle -- */
|
||||
|
||||
akerr_ErrorContext *akbasic_runtime_zero(akbasic_Runtime *obj)
|
||||
@@ -1166,10 +1198,11 @@ akerr_ErrorContext *akbasic_runtime_call_function(akbasic_Runtime *obj, const ch
|
||||
* Give them back, or a host absorbing script errors drains the
|
||||
* twelve-slot environment pool after twelve dead calls and every
|
||||
* call after that fails for a reason nobody can see in the script.
|
||||
* The unwind, not a bare prev_environment() loop, because a body that
|
||||
* died inside a FOR EACH leaves a suspended generator hanging off the
|
||||
* loop scope, and only the unwind knows to take it down too.
|
||||
*/
|
||||
while ( obj->environment != targetenv && obj->environment->parent != NULL ) {
|
||||
IGNORE(akbasic_runtime_prev_environment(obj));
|
||||
}
|
||||
IGNORE(akbasic_runtime_unwind_to_environment(obj, targetenv));
|
||||
} PROCESS(errctx) {
|
||||
} FINISH(errctx, true);
|
||||
PASS(errctx, akbasic_environment_new_value(targetenv, &out));
|
||||
|
||||
@@ -161,8 +161,22 @@ akerr_ErrorContext *akbasic_cmd_return(akbasic_Runtime *obj, akbasic_ASTLeaf *ex
|
||||
SUCCEED_TRUE(obj, dest);
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
/*
|
||||
* A GEN is a function at heart, and RETURN ends it the way it ends a DEF
|
||||
* or a GOSUB: early, cleanly, from its own frame. What a generator's
|
||||
* RETURN cannot do is carry a value -- values leave a GEN one at a time,
|
||||
* through EMIT, and there is no caller waiting on a return slot.
|
||||
*/
|
||||
if ( obj->environment->isGenerator ) {
|
||||
FAIL_NONZERO_RETURN(errctx, (expr != NULL && expr->right != NULL), AKBASIC_ERR_STATE,
|
||||
"A GEN yields values through EMIT; RETURN here takes none");
|
||||
PASS(errctx, akbasic_runtime_prev_environment(obj));
|
||||
obj->environment->forGeneratorEnv = NULL;
|
||||
SUCCEED_TRUE(obj, dest);
|
||||
SUCCEED_RETURN(errctx);
|
||||
}
|
||||
FAIL_ZERO_RETURN(errctx, (obj->environment->gosubReturnLine != 0), AKBASIC_ERR_STATE,
|
||||
"RETURN outside the context of GOSUB");
|
||||
"RETURN outside the context of GOSUB, DEF, or GEN");
|
||||
|
||||
if ( expr != NULL && expr->right != NULL ) {
|
||||
PASS(errctx, akbasic_runtime_evaluate(obj, expr->right, &result));
|
||||
|
||||
@@ -68,9 +68,7 @@ akerr_ErrorContext *akbasic_runtime_pump_generator(akbasic_Runtime *obj, akbasic
|
||||
* among the scopes just released.
|
||||
*/
|
||||
if ( obj->environment != loopenv ) {
|
||||
while ( obj->environment != loopenv && obj->environment->parent != NULL ) {
|
||||
IGNORE(akbasic_runtime_prev_environment(obj));
|
||||
}
|
||||
IGNORE(akbasic_runtime_unwind_to_environment(obj, loopenv));
|
||||
loopenv->forGeneratorEnv = NULL;
|
||||
}
|
||||
} PROCESS(errctx) {
|
||||
@@ -103,6 +101,18 @@ akerr_ErrorContext *akbasic_runtime_release_generator(akbasic_Runtime *obj, akba
|
||||
while ( walk != NULL ) {
|
||||
isgen = walk->isGenerator;
|
||||
next = walk->parent;
|
||||
/*
|
||||
* A scope between the resume point and the call frame may be an EACH
|
||||
* loop with its *own* generator suspended off to the side. Releasing
|
||||
* the loop scope without releasing that generator strands it in the
|
||||
* pool -- the walk goes through parents and a suspended generator is a
|
||||
* child. Guarded on `used` so a generator already released as part of
|
||||
* some enclosing teardown is not released twice.
|
||||
*/
|
||||
if ( walk->forGeneratorEnv != NULL && walk->forGeneratorEnv->used ) {
|
||||
PASS(errctx, akbasic_runtime_release_generator(obj, walk->forGeneratorEnv));
|
||||
}
|
||||
walk->forGeneratorEnv = NULL;
|
||||
PASS(errctx, akbasic_runtime_release_environment(obj, walk));
|
||||
if ( isgen ) {
|
||||
break;
|
||||
|
||||
@@ -149,11 +149,27 @@ akerr_ErrorContext *akbasic_cmd_loop(akbasic_Runtime *obj, akbasic_ASTLeaf *expr
|
||||
akbasic_Environment *loopenv = obj->environment;
|
||||
|
||||
PASS(errctx, akbasic_environment_stop_waiting(obj->environment, "LOOP"));
|
||||
if ( loopenv->forGeneratorEnv != NULL ) {
|
||||
/*
|
||||
* A condition on the LOOP composes with EACH: it is checked after each
|
||||
* trip through the body, with the loop variable still holding that
|
||||
* trip's value, before the generator is pumped for the next one. A
|
||||
* condition that says stop abandons the generator exactly as EXIT does.
|
||||
*/
|
||||
again = true;
|
||||
arg = (expr != NULL ? expr->right : NULL);
|
||||
if ( arg != NULL ) {
|
||||
kind = (int)arg->literal_int;
|
||||
PASS(errctx, loop_continues(obj, arg->left, kind, &again));
|
||||
}
|
||||
if ( !again && loopenv->forGeneratorEnv != NULL ) {
|
||||
PASS(errctx, akbasic_runtime_release_generator(obj, loopenv->forGeneratorEnv));
|
||||
loopenv->forGeneratorEnv = NULL;
|
||||
}
|
||||
if ( again && loopenv->forGeneratorEnv != NULL ) {
|
||||
obj->environment = loopenv->forGeneratorEnv;
|
||||
PASS(errctx, akbasic_runtime_pump_generator(obj, loopenv));
|
||||
}
|
||||
again = (loopenv->forGeneratorEnv != NULL);
|
||||
again = (again && loopenv->forGeneratorEnv != NULL);
|
||||
} else {
|
||||
PASS(errctx, akbasic_environment_stop_waiting(obj->environment, "LOOP"));
|
||||
/*
|
||||
|
||||
Reference in New Issue
Block a user