From fdb3421b2a72a05ab51d13e2c7dc20d04e4a4219 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Sun, 2 Aug 2026 11:06:45 -0400 Subject: [PATCH] Let C call a BASIC function with values it already has `akbasic_runtime_call_function(obj, name, args, nargs, dest)`. The argument binding is split out of the call-site handling, so `akbasic_runtime_user_function()` becomes evaluate-the-leaves and then call the same code -- and a verb that wants to hand a BASIC function four numbers has somewhere to start, which it did not before. The only entry point took a parsed AST call site, so calling a function required having been parsed as an expression. Behaviour-preserving: both suites pass unmodified. The one deliberate difference is that the AST path now evaluates *all* the arguments before binding any of them, where it used to interleave. That is the safer order and it is what by-value passing means everywhere else -- interleaved, a later argument could see an earlier one already in the callee's scope. **Finding, filed as section 6 item 41: a multi-line `DEF` called outside a running program does not run its body, and says nothing about it.** DEF TRIPLE(N#) T# = N# * 3 RETURN T# PRINT TRIPLE(14) At the REPL that prints "(UNDEFINED STRING REPRESENTATION FOR 0)". From a file the same function answers 42. The multi-line body runs by spinning a line loop guarded on `mode == AKBASIC_MODE_RUN`, which is true only of a program running from a file; in REPL mode the loop is skipped and the result is the caller's zeroed return slot. The single-expression form has no such loop, and every case in tests/user_functions.c goes through run_program and is therefore in RUN mode, which is most of why nobody had seen it. **The obvious fix is wrong and I tried it.** Widening the guard to `mode != AKBASIC_MODE_QUIT` makes the interpreter *hang* instead of answering wrongly -- `akbasic_runtime_process_line_run()` does not advance a REPL-mode runtime the way the loop assumes, so the environment never comes back. Trading a silent wrong answer for a lock-up is worse, so it is reverted, the reasoning is in a comment where the next person will try the same thing, and the fix is filed rather than guessed at. That bounds this entry point rather than blocking it: it reaches a multi-line body while a program is running, which is exactly the case a verb calling a callback is in. The new test asserts the single-expression form from C and says in a comment why the multi-line one is not asserted, so the omission is a statement rather than a gap. What is still not done is the language half -- no verb takes a function, and nothing resolves a bare word to a function rather than a label. Item 40 now says so, and says it wants a verb that needs it rather than speculation. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EwxGB6TdoVvZ11KQQME9cL --- TODO.md | 53 +++++++++++++++++++++--- include/akbasic/runtime.h | 26 ++++++++++++ include/akbasic/types.h | 6 +++ src/runtime.c | 86 ++++++++++++++++++++++++++++++++------- tests/user_functions.c | 54 ++++++++++++++++++++++++ 5 files changed, 205 insertions(+), 20 deletions(-) diff --git a/TODO.md b/TODO.md index 28211af..4d14e78 100644 --- a/TODO.md +++ b/TODO.md @@ -2298,12 +2298,17 @@ each is here so the reasoning does not have to be reconstructed. evaluation, with a scope from the pool, re-entrant since ยง6 item 26, and with recursion depth answering to `AKBASIC_MAX_ENVIRONMENTS` as a diagnosis rather than a hang. - What is missing is narrow. `akbasic_runtime_user_function()` takes an `akbasic_ASTLeaf *` - call site and evaluates argument *leaves* at `:993-1006`; split that loop so the AST path - evaluates-then-binds and a new entry point binds values it is handed. Resolving a bare word - to a function rather than a label is `akbasic_environment_get_function()` at `:960`, and - `COLLISION 1, BUMPED` is the precedent for a verb taking a bare word by name - (`src/runtime_sprite.c:543-555`). + ~~What is missing is narrow.~~ **Done**, as `akbasic_runtime_call_function()`: the + argument binding is split out of the call-site handling, so the AST path evaluates leaves + and then calls the same code a verb can call with values it already has. Bounded by item + 41 -- a multi-line body only runs while a program is running, which is the case a verb is + in and not the case a host with a stopped runtime is in. + + What is still not done is the *language* half: no verb takes a function yet, and nothing + resolves a bare word to a function rather than a label. That wants + `akbasic_environment_get_function()` and the precedent of `COLLISION 1, BUMPED` taking a + bare word by name (`src/runtime_sprite.c`) -- and a verb that actually needs it, rather + than being built on speculation. **One hazard, if it is ever wired to a collision query.** Do not pass the BASIC function through as an `akgl_CollisionVisitFunc`: it would run inside the partitioner's cell-chain @@ -2311,6 +2316,42 @@ each is here so the reasoning does not have to be reconstructed. `partitioner.remove()` rewriting the chain the walk holds a cursor into. Collect the candidates during the walk, close the walk, then dispatch. +41. **A multi-line `DEF` called outside a running program does not run its body**, and says + nothing about it. + + ```basic + DEF TRIPLE(N#) + T# = N# * 3 + RETURN T# + PRINT TRIPLE(14) + ``` + + At the REPL that prints `(UNDEFINED STRING REPRESENTATION FOR 0)`. In a program run from a + file the same function answers 42. No error either way. + + `akbasic_runtime_user_function()` (`src/runtime.c`) runs a multi-line body by handing + control to the call's environment and spinning a line loop until `RETURN` pops back out, + guarded on `mode == AKBASIC_MODE_RUN` -- which is true only of a program running from a + file. In `REPL` mode the loop is skipped entirely and the result is whatever is in the + caller's return slot, which is zero. The single-expression form has no such loop and is + unaffected, which is most of why this has gone unnoticed; and `tests/user_functions.c` + drives every case through `run_program`, so the whole suite is in RUN mode. + + **The obvious fix is wrong, and was tried.** Widening the guard to + `mode != AKBASIC_MODE_QUIT` makes the interpreter *hang* rather than answer wrongly: + `akbasic_runtime_process_line_run()` does not advance a REPL-mode runtime the way this + loop assumes, so the environment never comes back. Trading a silent wrong answer for a + lock-up is worse. The fix wants the REPL's own line cycle driving the body, which is a + change to how a call is executed rather than to a condition. + + It also bounds item 40: `akbasic_runtime_call_function()` reaches a multi-line body only + while a program is running, which is exactly the case a verb calling a callback is in and + not the case a host poking at a stopped runtime is in. + + A test belongs in `tests/user_functions.c` asserting the correct contract -- a multi-line + function answering 42 at the REPL -- registered in `AKBASIC_KNOWN_FAILING_TESTS` until it + does. + ## 7. Filing gaps against `libakgl` When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in diff --git a/include/akbasic/runtime.h b/include/akbasic/runtime.h index 2b100c5..f592327 100644 --- a/include/akbasic/runtime.h +++ b/include/akbasic/runtime.h @@ -783,6 +783,32 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_new_variable(akbasic_Runtime * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKBASIC_ERR_BOUNDS When every function slot is in use. */ +/** + * @brief Call a user-defined function with values a caller already has. + * + * The half of a call that is not parsing. akbasic_runtime_user_function() + * evaluates a parsed call site's arguments and then comes here; a **verb** that + * wants to hand a BASIC function four numbers starts here directly, which was + * not possible before -- the only entry point took an AST call site, so calling + * a function required having been parsed as an expression. + * + * The body runs the same way either way: a scope from the environment pool, as + * `GOSUB` takes, re-entrant, with recursion depth answering to + * #AKBASIC_MAX_ENVIRONMENTS. A multi-line definition re-enters the line loop + * synchronously and returns when its RETURN pops back out. + * + * @param obj The runtime. + * @param name The function's name, as `DEF` spelled it. + * @param args Values to bind, already evaluated. May be NULL when @p nargs is 0. + * @param nargs How many. Extra arguments beyond the definition's are ignored, + * which is what the AST path has always done. + * @param dest Receives the result, in the caller's scratch. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER When @p obj, @p name or @p dest is NULL. + * @throws AKBASIC_ERR_UNDEFINED When no function of that name is defined. + */ +akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_call_function(struct akbasic_Runtime *obj, const char *name, akbasic_Value **args, int nargs, akbasic_Value **dest); + akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_new_function(akbasic_Runtime *obj, akbasic_FunctionDef **dest); /** * @brief File one already-scanned source line under its line number. diff --git a/include/akbasic/types.h b/include/akbasic/types.h index 507699a..97bfad2 100644 --- a/include/akbasic/types.h +++ b/include/akbasic/types.h @@ -17,6 +17,12 @@ /* Per-environment pools */ #define AKBASIC_MAX_LEAVES 32 /* ~16 operations per source line */ +/* + * Arguments one call may carry. A line holds 32 tokens, so a call site cannot + * spell more than a handful anyway; this is the ceiling on the array a call + * builds them in, and it is checked rather than assumed. + */ +#define AKBASIC_MAX_CALL_ARGUMENTS 16 #define AKBASIC_MAX_TOKENS 32 #define AKBASIC_MAX_VALUES 64 #define AKBASIC_MAX_VARIABLES 128 diff --git a/src/runtime.c b/src/runtime.c index bef1d63..c54b7b1 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -944,21 +944,28 @@ static akerr_ErrorContext *bind_structure_parameter(akbasic_Runtime *obj, akbasi SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akbasic_runtime_user_function(akbasic_Runtime *obj, akbasic_ASTLeaf *expr, akbasic_Value **dest) +akerr_ErrorContext *akbasic_runtime_call_function(akbasic_Runtime *obj, const char *name, + akbasic_Value **args, int nargs, + akbasic_Value **dest) { PREPARE_ERROR(errctx); akbasic_FunctionDef *fndef = NULL; - akbasic_Environment *targetenv = obj->environment; - akbasic_ASTLeaf *leafptr = NULL; + akbasic_Environment *targetenv = NULL; akbasic_ASTLeaf *argptr = NULL; - akbasic_Value *argvalue = NULL; akbasic_Value *unused = NULL; akbasic_Value *result = NULL; akbasic_Value *out = NULL; akbasic_Environment *callenv = NULL; void *fnptr = NULL; + int i = 0; - PASS(errctx, akbasic_environment_get_function(obj->environment, expr->identifier, &fnptr)); + FAIL_ZERO_RETURN(errctx, (obj != NULL && name != NULL && dest != NULL), AKERR_NULLPOINTER, + "NULL argument in call_function"); + FAIL_ZERO_RETURN(errctx, (nargs == 0 || args != NULL), AKERR_NULLPOINTER, + "call_function was given %d arguments and no array", nargs); + targetenv = obj->environment; + + PASS(errctx, akbasic_environment_get_function(obj->environment, name, &fnptr)); fndef = (akbasic_FunctionDef *)fnptr; /* @@ -985,22 +992,25 @@ akerr_ErrorContext *akbasic_runtime_user_function(akbasic_Runtime *obj, akbasic_ obj->environment = targetenv; /* - * Bind arguments into the call's scope, evaluating each one in the caller's. - * Passing is by value: assignment is what copies, so a structure argument - * deep-copies and a pointer argument copies its reference. + * Bind the values into the call's scope. Passing is by value: assignment is + * what copies, so a structure argument deep-copies and a pointer argument + * copies its reference. + * + * The values arrive already evaluated, which is the whole difference between + * this and what it was. Evaluating a *leaf* here is what tied a call to + * having been parsed from a call site, and therefore to being reachable only + * from an expression -- a verb wanting to hand a function four numbers had + * nowhere to start. */ - leafptr = (expr->right != NULL ? expr->right->right : NULL); argptr = (fndef->arglist != NULL ? fndef->arglist->right : NULL); - while ( leafptr != NULL && argptr != NULL ) { - PASS(errctx, akbasic_runtime_evaluate(obj, leafptr, &argvalue)); + for ( i = 0; i < nargs && argptr != NULL; i++ ) { obj->environment = callenv; if ( argptr->leaftype == AKBASIC_LEAF_IDENTIFIER_STRUCT ) { - PASS(errctx, bind_structure_parameter(obj, callenv, argptr, argvalue)); + PASS(errctx, bind_structure_parameter(obj, callenv, argptr, args[i])); } else { - PASS(errctx, akbasic_environment_assign(callenv, argptr, argvalue, &unused)); + PASS(errctx, akbasic_environment_assign(callenv, argptr, args[i], &unused)); } obj->environment = targetenv; - leafptr = leafptr->next; argptr = argptr->next; } @@ -1030,6 +1040,19 @@ akerr_ErrorContext *akbasic_runtime_user_function(akbasic_Runtime *obj, akbasic_ */ callenv->gosubReturnLine = callenv->lineno + 1; callenv->nextline = fndef->lineno; + /* + * **The body only runs in RUN mode, and that is a defect rather than a + * rule.** A multi-line function called at the REPL falls straight past this + * loop and returns whatever is in the caller's return slot -- zero -- so + * `PRINT TRIPLE(14)` answers "(UNDEFINED STRING REPRESENTATION FOR 0)" with + * no error and no diagnostic. Recorded as TODO.md section 6 item 41. + * + * Widening it to `mode != AKBASIC_MODE_QUIT` is the obvious fix and is + * *wrong*: akbasic_runtime_process_line_run() does not advance a REPL-mode + * runtime the way this loop assumes, so the interpreter hangs instead of + * answering wrongly -- which is worse. The fix wants the REPL's own line + * cycle, and that is a larger change than a condition. + */ while ( obj->environment != targetenv && obj->mode == AKBASIC_MODE_RUN ) { PASS(errctx, akbasic_runtime_process_line_run(obj)); } @@ -1039,6 +1062,41 @@ akerr_ErrorContext *akbasic_runtime_user_function(akbasic_Runtime *obj, akbasic_ SUCCEED_RETURN(errctx); } +/** + * @brief Call a user-defined function from a parsed call site. + * + * Evaluates the arguments in the caller's scope and hands the values to + * akbasic_runtime_call_function(). The split is what lets a verb call a BASIC + * function too: everything below the evaluation is shared. + */ +akerr_ErrorContext *akbasic_runtime_user_function(akbasic_Runtime *obj, akbasic_ASTLeaf *expr, akbasic_Value **dest) +{ + PREPARE_ERROR(errctx); + akbasic_Value *args[AKBASIC_MAX_CALL_ARGUMENTS]; + akbasic_ASTLeaf *leafptr = NULL; + int nargs = 0; + + FAIL_ZERO_RETURN(errctx, (obj != NULL && expr != NULL && dest != NULL), AKERR_NULLPOINTER, + "NULL argument in user_function"); + + /* + * All of them first, then the call. Evaluating as each one is bound would + * mean a later argument could see an earlier one already in the callee's + * scope, which is not what by-value passing means anywhere else. + */ + leafptr = (expr->right != NULL ? expr->right->right : NULL); + while ( leafptr != NULL ) { + FAIL_ZERO_RETURN(errctx, (nargs < AKBASIC_MAX_CALL_ARGUMENTS), AKBASIC_ERR_BOUNDS, + "%s was called with more than %d arguments", + expr->identifier, AKBASIC_MAX_CALL_ARGUMENTS); + PASS(errctx, akbasic_runtime_evaluate(obj, leafptr, &args[nargs])); + nargs += 1; + leafptr = leafptr->next; + } + PASS(errctx, akbasic_runtime_call_function(obj, expr->identifier, args, nargs, dest)); + SUCCEED_RETURN(errctx); +} + /* ------------------------------------------------------------ line cycle -- */ int64_t akbasic_runtime_find_previous_lineno(akbasic_Runtime *obj) diff --git a/tests/user_functions.c b/tests/user_functions.c index d576914..d2d60d5 100644 --- a/tests/user_functions.c +++ b/tests/user_functions.c @@ -284,6 +284,59 @@ static void test_calls_do_not_leak_value_slots(void) harness_stop(); } +/** + * @brief A host can call a BASIC function with values it already has. + * + * The entry point a *verb* would use. Everything else here reaches a function + * through an expression the parser built; this reaches one with four numbers and + * a name, which is what a verb taking a callback would have to do and what was + * impossible until the argument binding was split out of the call site handling. + */ +static void test_call_from_c(void) +{ + akbasic_Value args[2]; + akbasic_Value *argp[2]; + akbasic_Value *result = NULL; + + TEST_REQUIRE_OK(run_program("10 DEF ADDEM(A#, B#) = A# + B#\n" + "20 DEF TRIPLE(N#)\n" + "30 T# = N# * 3\n" + "40 RETURN T#\n")); + TEST_REQUIRE_STR(HARNESS_OUTPUT, ""); + + memset(&args[0], 0, sizeof(args[0])); + memset(&args[1], 0, sizeof(args[1])); + args[0].valuetype = AKBASIC_TYPE_INTEGER; + args[0].intval = 17; + args[1].valuetype = AKBASIC_TYPE_INTEGER; + args[1].intval = 25; + argp[0] = &args[0]; + argp[1] = &args[1]; + + TEST_REQUIRE_OK(akbasic_runtime_call_function(&HARNESS_RUNTIME, "ADDEM", argp, 2, &result)); + TEST_REQUIRE(result != NULL, "a call should have produced a result"); + TEST_REQUIRE_INT(result->intval, 42); + + /* + * **The multi-line form is not asserted here, and that is a finding rather + * than an omission.** Its body is *source lines*, so the call re-enters the + * interpreter's line loop -- and that loop only runs while the runtime is in + * RUN mode. By the time a host can call this the program has stopped, so the + * body does not run and the result is the caller's zeroed return slot. + * + * The same defect is reachable without any of this: a multi-line function + * called at the *REPL* answers "(UNDEFINED STRING REPRESENTATION FOR 0)". + * TODO.md section 6 item 41. So this entry point is usable today from a verb + * during execution, which is what it was built for, and from a host only for + * the single-expression form. + */ + + /* A name nobody defined is refused rather than crashing. */ + TEST_REQUIRE_ANY_ERROR(akbasic_runtime_call_function(&HARNESS_RUNTIME, "NOSUCH", + argp, 1, &result)); + harness_stop(); +} + int main(void) { TEST_REQUIRE_OK(akbasic_error_register()); @@ -297,6 +350,7 @@ int main(void) test_structure_parameter_types_are_checked(); test_call_scopes_are_reclaimed(); test_calls_do_not_leak_value_slots(); + test_call_from_c(); return akbasic_test_failures; }