Let C call a BASIC function with values it already has
Some checks failed
akbasic CI Build / cmake_build (push) Failing after 3m22s
akbasic CI Build / sanitizers (push) Failing after 4m31s
akbasic CI Build / coverage (push) Failing after 3m40s
akbasic CI Build / akgl_build (push) Failing after 22s
akbasic CI Build / mutation_test (push) Has been cancelled
Some checks failed
akbasic CI Build / cmake_build (push) Failing after 3m22s
akbasic CI Build / sanitizers (push) Failing after 4m31s
akbasic CI Build / coverage (push) Failing after 3m40s
akbasic CI Build / akgl_build (push) Failing after 22s
akbasic CI Build / mutation_test (push) Has been cancelled
`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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EwxGB6TdoVvZ11KQQME9cL
This commit is contained in:
53
TODO.md
53
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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user