From 5b7b7d2ed9721060bac16525f06b0530ddca3906 Mon Sep 17 00:00:00 2001 From: Andrew Kesterson Date: Fri, 31 Jul 2026 11:53:05 -0400 Subject: [PATCH] Add akbasic_runtime_global: a host variable lands in the script's root scope A host creating a variable while a script was suspended got it in whatever scope was active -- usually a FOR or GOSUB body -- and it died when the body popped, silently. Reaching for the root by hand returned NULL without raising, because environment_get only auto-creates in the active environment. Both are still true of environment_get, which is correct for what the interpreter uses it for. The README and the example now point somewhere else. Co-Authored-By: Claude Opus 5 (1M context) --- CMakeLists.txt | 1 + README.md | 44 +++---- TODO.md | 68 +++++------ examples/hostvars.c | 79 ++++++++----- include/akbasic/environment.h | 21 ++++ include/akbasic/runtime.h | 31 +++++ src/environment.c | 32 ++++- src/runtime.c | 23 ++++ tests/hostvars.c | 213 ++++++++++++++++++++++++++++++++++ 9 files changed, 425 insertions(+), 87 deletions(-) create mode 100644 tests/hostvars.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 423d8db..369c8cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -238,6 +238,7 @@ set(AKBASIC_TESTS error_codes grammar_leaves graphics_verbs + hostvars input_verbs numeric_contract parser_commands diff --git a/README.md b/README.md index 1ffb086..6c61c13 100644 --- a/README.md +++ b/README.md @@ -254,7 +254,7 @@ Yes — in both directions, for integers, floats and strings, using the same var script itself uses. There is no marshalling layer and no copy: the host and the script are looking at the same `akbasic_Value`. -`akbasic_environment_get()` finds a variable by name and creates it if it does not exist. The +`akbasic_runtime_global()` finds a variable by name and creates it if it does not exist. The type comes from the name's suffix, exactly as it does for BASIC code, so `HP#` is an integer and `NAME$` is a string. Every scalar is really a one-element array, which is why the subscript list is `{0}` with a count of 1. @@ -267,9 +267,7 @@ akerr_ErrorContext AKERR_NOIGNORE *host_set_int(akbasic_Runtime *obj, const char akbasic_Variable *variable = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, - "could not reach variable %s from the active scope", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_set_integer(variable, value, subscript, 1)); SUCCEED_RETURN(errctx); } @@ -282,8 +280,7 @@ akerr_ErrorContext AKERR_NOIGNORE *host_get_int(akbasic_Runtime *obj, const char akbasic_Value *value = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, "no variable %s", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_get_subscript(variable, subscript, 1, &value)); FAIL_NONZERO_RETURN(errctx, (value->valuetype != AKBASIC_TYPE_INTEGER), AKBASIC_ERR_TYPE, "%s is not an integer", name); @@ -310,26 +307,29 @@ CATCH(errctx, host_get_int(&SCRIPT, "SCORE#", &score)); /* collect */ The full thing, including the string and float forms, is in [`examples/hostvars.c`](examples/hostvars.c). It is built and run by every build. -### The one rule: seed before, read after +### Which scope it lands in, and why you should not care -**Do this between runs, not during one.** Seed before `akbasic_runtime_start()` and read after -the script has stopped. Poking a variable while a script is suspended part-way through a -bounded `akbasic_runtime_run()` is sharp in two ways, and both are silent: +`akbasic_runtime_global()` always resolves against the script's outermost scope, so seeding +before `akbasic_runtime_start()`, reading after the script stops, and creating a variable while +a script is suspended part-way through a bounded `akbasic_runtime_run()` all do the same, +obvious thing. -* A suspended script is usually inside a `FOR` or `GOSUB` scope, and `obj->environment` is that - scope rather than the global one. A variable *created* there dies when the scope pops — the - script reads it correctly inside the loop and gets `0` the moment the loop ends. -* Reaching for the global scope explicitly does not help. Only the currently active environment - auto-creates, so `akbasic_environment_get(root, "NEW#", &var)` with a child active hands back - `NULL` and no error, and an unchecked host dereferences it. +That is worth one paragraph of history, because the obvious-looking alternative is a trap. +`akbasic_environment_get()` resolves against `obj->environment` — whatever scope is *active* — +and a suspended script is usually inside a `FOR` or `GOSUB` body. A variable created through it +lands in that body and dies when the body pops: the script reads it correctly inside the loop +and gets `0` the moment the loop ends, with nothing raised anywhere. Reaching for the root +explicitly does not help either, because only the active environment auto-creates, so +`akbasic_environment_get(root, "NEW#", &var)` with a child active hands back `NULL` and no +error and an unchecked host dereferences it. -Reading and *updating an existing* global is safe at any time — the parent chain is searched, -so a variable that already exists in the global scope is found from anywhere. It is only -*creation* that lands in the wrong place. +Both of those are still true of `akbasic_environment_get()`, which is the right behaviour for +what the *interpreter* uses it for. They are simply not your problem: use +`akbasic_runtime_global()`. -`examples/hostvars.c` demonstrates both hazards rather than describing them. This is filed as -[`TODO.md`](TODO.md) section 6 item 17; the fix is a small `akbasic_runtime_global()` that -resolves against the root scope regardless of what is active. +[`examples/hostvars.c`](examples/hostvars.c) shows the difference rather than describing it, +and `tests/hostvars.c` asserts it — a global created from inside a `FOR` body, and from inside +a `GOSUB`, and still readable by the script afterwards. ## Where the output goes diff --git a/TODO.md b/TODO.md index ba26ce2..7cf3c12 100644 --- a/TODO.md +++ b/TODO.md @@ -895,38 +895,35 @@ question. ### Ours, not the reference's -17. **A host cannot reliably create a script variable while a script is suspended.** This one - is not inherited — it falls out of §1.4's environment pool meeting §1.6's bounded `run()`, - a combination the reference never had because its `run()` never returned. +17. ~~**A host cannot reliably create a script variable while a script is suspended.**~~ + **Fixed.** `akbasic_runtime_global(obj, name, dest)` walks `obj->environment` to the root + and finds or creates there unconditionally. `README.md` and `examples/hostvars.c` both point + at it now, and `tests/hostvars.c` asserts it: a global created from inside a `FOR` body and + from inside a `GOSUB`, still readable by the script after the body pops, plus the + seed-before / read-after path that always worked. - Exchanging variables with an embedding host works and is documented in `README.md`: the - host calls `akbasic_environment_get()` to find or create a variable and - `akbasic_variable_set_*` / `_get_subscript` to move values across. Seeding before - `akbasic_runtime_start()` and reading after the script stops is safe, and so is *updating* - an existing global at any time, because the parent chain is searched. **Creating** one - mid-run is not, in two ways, and both are silent: + This one was not inherited — it fell out of §1.4's environment pool meeting §1.6's bounded + `run()`, a combination the reference never had because its `run()` never returned. Both + failure modes were silent: a variable created through `akbasic_environment_get()` landed in + the loop's scope and died with it, so the script read it correctly inside the loop and got + `0` immediately after; and reaching for the root explicitly returned `NULL` through `dest` + **without raising**, because that function only auto-creates when + `obj->runtime->environment == obj`. - - A script suspended part-way through a bounded `akbasic_runtime_run()` is usually inside a - `FOR` or `GOSUB` scope, so `obj->environment` is that scope. A variable created through - it lands in the loop's scope and dies when the environment pops — the script reads the - value correctly inside the loop and gets `0` immediately after it. - - Reaching for the root explicitly does not help. `akbasic_environment_get` only - auto-creates when `obj->runtime->environment == obj` (`src/environment.c:242`), so with a - child active it returns `NULL` through `dest` **without raising**, and an unchecked host - dereferences it. + Three things settled while doing it, each worth keeping: - `examples/hostvars.c` demonstrates both rather than describing them, and prints what it - observes, so the day this is fixed that output changes and the example needs revisiting. - - Fix: add `akbasic_runtime_global(akbasic_Runtime *obj, const char *name, akbasic_Variable **dest)` - that walks `obj->environment` to the root and creates there unconditionally, and point the - README at it instead of at `akbasic_environment_get`. Roughly fifteen lines. The one design - question worth settling first is what it should do when the script is suspended inside a - *user function's* scope — that environment is owned by the funcdef rather than the pool - (§1.4), and "root" is still the right answer there, but it is worth being deliberate about - it rather than discovering it. Tests: create a global while suspended in a `FOR` body and - assert the script still sees it after `NEXT`; the same from inside a `GOSUB`; and the - NULL-without-error path, which should become an impossible state. + - **The design question §6 named has an answer.** Suspended inside a *user function's* + scope, the root is still right and the walk still gets there: a funcdef's environment is + initialized with the caller's as its parent (`akbasic_runtime_user_function`), so the + chain terminates at the same root as any other. + - **`akbasic_environment_create()` is the new half**, and it searches *only* the scope it + is given — no walk in either direction. Walking up would find an outer variable and put + the value somewhere other than where the caller asked for it. + - **`akbasic_environment_get()` is unchanged.** Declining to create in a non-active scope is + the right behaviour for what the interpreter uses it for; it is only wrong as a *host* + API, which is why the answer was a new entry point rather than a change to that one. + `tests/hostvars.c` asserts the old behaviour too, so a later "tidy-up" has to argue with a + test. --- @@ -1019,10 +1016,10 @@ entire test corpus and passes clean under ASan and UBSan. | Gate | Result | |---|---| -| `ctest` | 74/74 — 41 upstream golden cases, 8 local ones, 22 unit tests, 2 embedding examples, 1 known-failing | -| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 73/73 — the same, minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends` and `akgl_frontend`; the `akgl_build` CI job | +| `ctest` | 75/75 — 41 upstream golden cases, 8 local ones, 23 unit tests, 2 embedding examples, 1 known-failing | +| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 74/74 — the same, minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends` and `akgl_frontend`; the `akgl_build` CI job | | Golden corpus | 41/41 byte-exact, driven in place from the submodule — **and 41/41 again through the SDL binary**, which is most of what proves the frontend changes no output | -| ASan + UBSan | 74/74 | +| ASan + UBSan | 75/75 | | Line coverage | 93.6% (3618/3867) — above the 90% gate | | Function coverage | 97.8% (267/273) | | Warnings | none under `-Wall -Wextra` | @@ -1137,14 +1134,13 @@ What remains, in priority order: deliberately does not clone. Two apt packages against two more submodules is an easy trade. Every step was verified from a clean clone before being written down, not inferred. -5. **§6 items 16 and 17.** Items 10 and 12–15 are fixed, each with a regression test in the +5. **§6 item 16.** Items 10, 12–15 and 17 are fixed, each with a regression test in the suite that pinned it. What is left is not more of the same: - **Item 16** is a *decision*, not a fix. Making the "Reserved word in variable name" check fire costs an upstream golden case, because the reference's own `strreverse.bas` names a variable `INPUT$`. §6 states the trade; somebody has to make it. - - **Item 17** is ours: a host cannot reliably create a script variable while a script is - suspended. The fix is `akbasic_runtime_global()` and roughly fifteen lines. §6 describes - it and names the one design question to settle first. + - ~~**Item 17**~~ **Done** — `akbasic_runtime_global()`, `tests/hostvars.c`, and the README + and example both pointed at it. 6. **Mutation survivors in `src/value.c`** — ~~closed~~, with one correction to what was written here before. `tests/value_arithmetic.c` grew two functions, `test_maximum_length_string()` and `test_pool_starts_empty()`, and each was verified by diff --git a/examples/hostvars.c b/examples/hostvars.c index fe46b2e..5b101e4 100644 --- a/examples/hostvars.c +++ b/examples/hostvars.c @@ -6,10 +6,16 @@ * kept here so it is compiled and run by every build rather than rotting in a * document. * - * The safe pattern is: seed variables *before* akbasic_runtime_start(), read - * results *after* the script stops. Poking a variable while the script is - * suspended part-way through is sharp in two ways that this file demonstrates at - * the bottom -- see TODO.md section 6 item 17. + * **Use akbasic_runtime_global().** It finds or creates the variable in the + * script's outermost scope, which is what a host means every time. The obvious + * alternative, akbasic_environment_get(), lands a new variable in whatever scope + * is *active* -- and a script suspended part-way through a bounded run() is + * usually inside a FOR or GOSUB body, so the variable dies when that body pops + * and the script reads zero from then on, with nothing raised anywhere. + * + * The bottom of this file demonstrates that difference rather than describing + * it, because it was TODO.md section 6 item 17 and it is the sort of thing that + * gets rediscovered. */ #include @@ -33,7 +39,7 @@ static const char *PROGRAM = /* ------------------------------------------------------- host -> script -- */ /* - * Seed an integer the script can read. akbasic_environment_get() creates the + * Seed an integer the script can read. akbasic_runtime_global() creates the * variable if it does not exist, and the type comes from the name's suffix, so * "HP#" is an integer and "HP$" would be a string. * @@ -46,9 +52,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *host_set_int(akbasic_Runtime *obj, con akbasic_Variable *variable = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, - "could not reach variable %s from the active scope", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_set_integer(variable, value, subscript, 1)); SUCCEED_RETURN(errctx); } @@ -59,9 +63,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *host_set_string(akbasic_Runtime *obj, akbasic_Variable *variable = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, - "could not reach variable %s from the active scope", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_set_string(variable, value, subscript, 1)); SUCCEED_RETURN(errctx); } @@ -75,8 +77,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *host_get_int(akbasic_Runtime *obj, con akbasic_Value *value = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, "no variable %s", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_get_subscript(variable, subscript, 1, &value)); FAIL_NONZERO_RETURN(errctx, (value->valuetype != AKBASIC_TYPE_INTEGER), AKBASIC_ERR_TYPE, "%s is not an integer", name); @@ -91,8 +92,7 @@ static akerr_ErrorContext AKERR_NOIGNORE *host_get_string(akbasic_Runtime *obj, akbasic_Value *value = NULL; int64_t subscript[1] = { 0 }; - PASS(errctx, akbasic_environment_get(obj->environment, name, &variable)); - FAIL_ZERO_RETURN(errctx, (variable != NULL), AKERR_KEY, "no variable %s", name); + PASS(errctx, akbasic_runtime_global(obj, name, &variable)); PASS(errctx, akbasic_variable_get_subscript(variable, subscript, 1, &value)); FAIL_NONZERO_RETURN(errctx, (value->valuetype != AKBASIC_TYPE_STRING), AKBASIC_ERR_TYPE, "%s is not a string", name); @@ -105,9 +105,10 @@ static akerr_ErrorContext AKERR_NOIGNORE *host_get_string(akbasic_Runtime *obj, /* ------------------------------------------------------------- the demo -- */ /* - * The hazard, demonstrated rather than described. A script suspended part-way - * through is usually inside a FOR or GOSUB scope, and a variable created there - * dies when that scope pops. + * The difference between the two entry points, demonstrated rather than + * described. The script prints the same variable from inside a loop and from + * after it, so a variable that died with the loop's scope is visible as a zero + * on the second line rather than as an error anywhere. */ static const char *LOOPING_PROGRAM = "10 FOR I# = 1 TO 3\n" @@ -115,7 +116,24 @@ static const char *LOOPING_PROGRAM = "30 NEXT I#\n" "40 PRINT \" after loop, GIFT# = \" + GIFT#\n"; -static akerr_ErrorContext AKERR_NOIGNORE *demonstrate_scope_hazard(void) +/* Step until the script is inside the loop body, whatever line that turns out + * to be. A step processes one source-line *index*, and lines are filed under + * their BASIC line numbers, so a fixed count would be a hostage to them. */ +static akerr_ErrorContext AKERR_NOIGNORE *run_into_the_loop(void) +{ + PREPARE_ERROR(errctx); + int steps = 0; + + while ( steps < AKBASIC_MAX_SOURCE_LINES && + SCRIPT.mode != AKBASIC_MODE_QUIT && + SCRIPT.environment->parent == NULL ) { + PASS(errctx, akbasic_runtime_run(&SCRIPT, 1)); + steps += 1; + } + SUCCEED_RETURN(errctx); +} + +static akerr_ErrorContext AKERR_NOIGNORE *demonstrate_scope_rules(void) { PREPARE_ERROR(errctx); akbasic_Environment *root = NULL; @@ -126,18 +144,25 @@ static akerr_ErrorContext AKERR_NOIGNORE *demonstrate_scope_hazard(void) PASS(errctx, akbasic_runtime_load(&SCRIPT, LOOPING_PROGRAM)); PASS(errctx, akbasic_runtime_start(&SCRIPT, AKBASIC_MODE_RUN)); - /* Run far enough to be inside the loop body. */ - PASS(errctx, akbasic_runtime_run(&SCRIPT, 12)); + PASS(errctx, run_into_the_loop()); printf("suspended inside the loop? %s\n", (SCRIPT.environment != root ? "yes" : "no")); - /* Poking through the *active* scope creates the variable in the loop. */ + /* + * The right way. The variable lands in the root scope, so line 40 -- which + * runs after NEXT has popped the loop -- still sees it. + */ PASS(errctx, host_set_int(&SCRIPT, "GIFT#", 42)); - /* Reaching for the root scope while a child is active simply misses. */ + /* + * The wrong way, kept because the failure is silent and worth seeing once. + * akbasic_environment_get() only auto-creates in the *active* environment, + * so reaching for the root while a child is active returns NULL through + * dest without raising -- and an unchecked host dereferences it. + */ PASS(errctx, akbasic_environment_get(root, "UNREACHABLE#", &variable)); - printf("can the host create in the root scope mid-run? %s\n", - (variable != NULL ? "yes" : "no -- get() returns NULL, with no error")); + printf("does environment_get() create in the root mid-run? %s\n", + (variable != NULL ? "yes" : "no -- it returns NULL, with no error")); PASS(errctx, akbasic_runtime_run(&SCRIPT, 0)); SUCCEED_RETURN(errctx); @@ -172,8 +197,8 @@ int main(void) printf("host read back: SCORE# = %lld, STATUS$ = \"%s\"\n", (long long)score, status); - printf("\n--- the mid-run scope hazard (TODO.md 6.17) ---\n"); - CATCH(errctx, demonstrate_scope_hazard()); + printf("\n--- creating a variable mid-run ---\n"); + CATCH(errctx, demonstrate_scope_rules()); } CLEANUP { } PROCESS(errctx) { } HANDLE_DEFAULT(errctx) { diff --git a/include/akbasic/environment.h b/include/akbasic/environment.h index 0d0a331..8433a90 100644 --- a/include/akbasic/environment.h +++ b/include/akbasic/environment.h @@ -159,9 +159,30 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_stop_waiting(akbasic_Envi * runtime's currently active environment auto-creates. A lookup that misses in a * non-active environment returns NULL through `dest` without error, matching the * reference. + * + * **A host wanting a variable the script will still see later wants + * akbasic_runtime_global() instead.** This one lands the variable in whatever + * scope happens to be active, which during a suspended run is usually a `FOR` or + * `GOSUB` body -- and it dies when that scope pops. */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_get(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest); +/** + * @brief Find a variable in exactly this scope, creating it here if it is absent. + * + * No walk up the parent chain in either direction: the caller has said which + * scope it means, and no active-environment check stands in the way. This is + * what akbasic_runtime_global() calls against the root. + * + * @param obj The scope to search and, on a miss, to create in. + * @param varname Name including its type suffix. + * @param dest Output destination populated by the function; never NULL on success. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER When any argument is NULL. + * @throws AKBASIC_ERR_BOUNDS When the name is too long, or no variable slot is free. + */ +akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest); + /** * @brief Resolve a label to the line number it marks. * @param obj Scope to search; the parent chain is walked. diff --git a/include/akbasic/runtime.h b/include/akbasic/runtime.h index a51b24f..4d0de97 100644 --- a/include/akbasic/runtime.h +++ b/include/akbasic/runtime.h @@ -373,6 +373,37 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_start(akbasic_Runtime *obj, i */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_load(akbasic_Runtime *obj, const char *source); +/** + * @brief Find or create a variable in the script's outermost scope. + * + * **This is the entry point a host exchanging values with a script should use.** + * akbasic_environment_get() lands a new variable in whatever scope is active, + * and a script suspended part-way through a bounded akbasic_runtime_run() is + * usually inside a `FOR` or `GOSUB` body -- so the script reads the value + * correctly inside the loop and gets `0` immediately after it, with nothing + * raised anywhere. Reaching for the root by hand does not help either: that + * function only auto-creates in the *active* environment, so with a child active + * it returns NULL through `dest` without raising and an unchecked host + * dereferences it. + * + * Here the root is found by walking `obj->environment` to the environment with + * no parent, and the variable is created there unconditionally. That is the + * right answer inside a user function's scope too: a funcdef's environment is + * initialized with the caller's as its parent, so the walk terminates at the + * same root. + * + * Seeding before akbasic_runtime_start() and reading after the script stops both + * still work through this, so a host has no reason to use anything else. + * + * @param obj Object to initialize, inspect, or modify. + * @param name Variable name including its type suffix -- `A#`, `B%`, `C$`. + * @param dest Output destination populated by the function; never NULL on success. + * @return `NULL` on success, otherwise an error context owned by the caller. + * @throws AKERR_NULLPOINTER When any argument is NULL, or the runtime has no environment. + * @throws AKBASIC_ERR_BOUNDS When the name is too long, or no variable slot is free. + */ +akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_global(akbasic_Runtime *obj, const char *name, akbasic_Variable **dest); + /* --- Internal API: exposed for the scanner, parser and verb handlers only. --- */ /** diff --git a/src/environment.c b/src/environment.c index d167d14..b4263de 100644 --- a/src/environment.c +++ b/src/environment.c @@ -249,8 +249,6 @@ akerr_ErrorContext *akbasic_environment_get(akbasic_Environment *obj, const char { PREPARE_ERROR(errctx); akbasic_Environment *walk = NULL; - akbasic_Variable *variable = NULL; - int64_t sizes[1] = { 1 }; void *slot = NULL; FAIL_ZERO_RETURN(errctx, (obj != NULL && varname != NULL && dest != NULL), AKERR_NULLPOINTER, @@ -276,6 +274,36 @@ akerr_ErrorContext *akbasic_environment_get(akbasic_Environment *obj, const char SUCCEED_RETURN(errctx); } + PASS(errctx, akbasic_environment_create(obj, varname, dest)); + SUCCEED_RETURN(errctx); +} + +akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest) +{ + PREPARE_ERROR(errctx); + akbasic_Variable *variable = NULL; + int64_t sizes[1] = { 1 }; + void *slot = NULL; + akerr_ErrorContext *found = NULL; + + FAIL_ZERO_RETURN(errctx, (obj != NULL && varname != NULL && dest != NULL), AKERR_NULLPOINTER, + "NULL argument in environment create"); + + /* + * This scope only. Unlike akbasic_environment_get() there is no walk up the + * parent chain: the caller has already said *which* scope it means, and + * finding an outer one would put the variable somewhere other than where it + * was asked for. + */ + *dest = NULL; + found = akbasic_symtab_get(&obj->variables, varname, &slot, NULL); + if ( found == NULL ) { + *dest = (akbasic_Variable *)slot; + SUCCEED_RETURN(errctx); + } + found->handled = true; + IGNORE(akerr_release_error(found)); + PASS(errctx, akbasic_runtime_new_variable(obj->runtime, &variable)); FAIL_ZERO_RETURN(errctx, (strlen(varname) < sizeof(variable->name)), AKBASIC_ERR_BOUNDS, "Variable name '%s' is too long", varname); diff --git a/src/runtime.c b/src/runtime.c index 4c56dba..06ea3bb 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -35,6 +35,29 @@ akerr_ErrorContext *akbasic_runtime_new_variable(akbasic_Runtime *obj, akbasic_V FAIL_RETURN(errctx, AKBASIC_ERR_BOUNDS, "Maximum runtime variables reached"); } +akerr_ErrorContext *akbasic_runtime_global(akbasic_Runtime *obj, const char *name, akbasic_Variable **dest) +{ + PREPARE_ERROR(errctx); + akbasic_Environment *root = NULL; + + FAIL_ZERO_RETURN(errctx, (obj != NULL && name != NULL && dest != NULL), AKERR_NULLPOINTER, + "NULL argument in runtime global"); + FAIL_ZERO_RETURN(errctx, (obj->environment != NULL), AKERR_NULLPOINTER, + "Runtime has no environment; call akbasic_runtime_init() first"); + + /* + * Walk to the root rather than using obj->environment. That is the whole + * point: a script suspended part-way through a bounded run() is usually + * inside a FOR or GOSUB body, and a variable created there dies when the + * body pops -- silently, with the script reading it correctly right up + * until it stops. See the note on the declaration. + */ + for ( root = obj->environment; root->parent != NULL; root = root->parent ) { + } + PASS(errctx, akbasic_environment_create(root, name, dest)); + SUCCEED_RETURN(errctx); +} + akerr_ErrorContext *akbasic_runtime_new_function(akbasic_Runtime *obj, akbasic_FunctionDef **dest) { PREPARE_ERROR(errctx); diff --git a/tests/hostvars.c b/tests/hostvars.c new file mode 100644 index 0000000..825bf27 --- /dev/null +++ b/tests/hostvars.c @@ -0,0 +1,213 @@ +/** + * @file hostvars.c + * @brief Tests the host-facing variable exchange, from the host's side. + * + * TODO.md section 6 item 17: a host could not reliably *create* a script + * variable while a script was suspended, and neither way of trying failed + * loudly. Through akbasic_environment_get() the variable landed in whichever + * scope happened to be active -- usually a `FOR` or `GOSUB` body -- and died + * when that body popped, so the script read it correctly right up until it + * stopped. Reaching for the root by hand returned NULL through `dest` without + * raising, because that function only auto-creates in the active environment. + * + * akbasic_runtime_global() is the answer to both, and what is asserted here is + * the part a host actually cares about: a value it set mid-run is still there + * after the loop it was set from has finished. + */ + +#include "harness.h" + +/** + * @brief Step a program until it is suspended inside a nested scope. + * + * One step at a time until a child environment is active, rather than a fixed + * budget: a step processes one *source line index*, and the interpreter files + * lines under their BASIC line numbers, so `10 FOR` is eleven steps in and any + * count written here would be a hostage to the line numbers in the fixture. + * + * Bounded so a program that never nests fails the assertion below rather than + * hanging the suite. + */ +static akerr_ErrorContext AKERR_NOIGNORE *run_until_nested(const char *source) +{ + PREPARE_ERROR(errctx); + int steps = 0; + + PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, source)); + PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN)); + while ( steps < AKBASIC_MAX_SOURCE_LINES && + HARNESS_RUNTIME.mode != AKBASIC_MODE_QUIT && + HARNESS_RUNTIME.environment->parent == NULL ) { + PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 1)); + steps += 1; + } + SUCCEED_RETURN(errctx); +} + +/** @brief Set a global to an integer, the way an embedding host would. */ +static akerr_ErrorContext AKERR_NOIGNORE *set_global(const char *name, int64_t value) +{ + PREPARE_ERROR(errctx); + akbasic_Variable *variable = NULL; + int64_t zerosubscript[1] = { 0 }; + + PASS(errctx, akbasic_runtime_global(&HARNESS_RUNTIME, name, &variable)); + FAIL_ZERO_RETURN(errctx, (variable != NULL), AKBASIC_ERR_UNDEFINED, + "runtime_global returned NULL for %s", name); + PASS(errctx, akbasic_variable_set_integer(variable, value, zerosubscript, 1)); + SUCCEED_RETURN(errctx); +} + +/** + * @brief A global created while suspended inside a FOR body outlives the loop. + * + * The case that motivated the whole entry. `H#` is created from outside while + * the script sits in the loop, and line 40 -- which runs after `NEXT` has popped + * the loop's scope -- has to still see it. + */ +static akerr_ErrorContext AKERR_NOIGNORE *test_global_survives_a_for_body(void) +{ + PREPARE_ERROR(errctx); + + TEST_REQUIRE_OK(harness_start(NULL)); + PASS(errctx, run_until_nested("10 FOR I# = 1 TO 3\n" + "20 PRINT I#\n" + "30 NEXT I#\n" + "40 PRINT H#\n")); + + /* Suspended below the root, which is the state that used to break this. */ + TEST_REQUIRE(HARNESS_RUNTIME.environment != NULL, "the runtime must have a scope"); + TEST_REQUIRE(HARNESS_RUNTIME.environment->parent != NULL, + "the script should be suspended inside the FOR body"); + + PASS(errctx, set_global("H#", 42)); + PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 0)); + + /* 1 2 3 from the loop, then 42 from after it. The 42 is the assertion. */ + TEST_REQUIRE_STR(HARNESS_OUTPUT, "1\n2\n3\n42\n"); + harness_stop(); + SUCCEED_RETURN(errctx); +} + +/** @brief The same from inside a GOSUB, which is the other nesting a host meets. */ +static akerr_ErrorContext AKERR_NOIGNORE *test_global_survives_a_gosub(void) +{ + PREPARE_ERROR(errctx); + + TEST_REQUIRE_OK(harness_start(NULL)); + /* + * The QUIT is not decoration. Without a terminator the program runs off the + * end of line 20 into the subroutine again, and its RETURN then has no GOSUB + * to return to -- which is ordinary BASIC and would only obscure what is + * being asserted here. + */ + PASS(errctx, run_until_nested("10 GOSUB 100\n" + "20 PRINT H#\n" + "30 QUIT\n" + "100 PRINT \"IN\"\n" + "110 RETURN\n")); + + TEST_REQUIRE(HARNESS_RUNTIME.environment->parent != NULL, + "the script should be suspended inside the subroutine"); + PASS(errctx, set_global("H#", 7)); + PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 0)); + + TEST_REQUIRE_STR(HARNESS_OUTPUT, "IN\n7\n"); + harness_stop(); + SUCCEED_RETURN(errctx); +} + +/** + * @brief The NULL-without-error path is gone: this always creates or raises. + * + * akbasic_environment_get() against a non-active environment is the shape that + * used to bite, and it is asserted here as well -- unchanged, because it is + * still the right behaviour for *that* function and it is what makes + * runtime_global() necessary rather than merely convenient. + */ +static akerr_ErrorContext AKERR_NOIGNORE *test_global_never_returns_null(void) +{ + PREPARE_ERROR(errctx); + akbasic_Environment *root = NULL; + akbasic_Variable *variable = NULL; + + TEST_REQUIRE_OK(harness_start(NULL)); + root = HARNESS_RUNTIME.environment; + PASS(errctx, akbasic_runtime_new_environment(&HARNESS_RUNTIME)); + + /* The old way, with a child active: NULL, and nothing raised. */ + variable = NULL; + TEST_REQUIRE_OK(akbasic_environment_get(root, "NEW#", &variable)); + TEST_REQUIRE(variable == NULL, + "environment_get must still decline to create in a non-active scope"); + + /* The new way: created, in the root, every time. */ + variable = NULL; + PASS(errctx, akbasic_runtime_global(&HARNESS_RUNTIME, "NEW#", &variable)); + TEST_REQUIRE(variable != NULL, "runtime_global must create rather than return NULL"); + + /* And it is genuinely in the root, visible from the child. */ + TEST_REQUIRE_OK(akbasic_environment_get(HARNESS_RUNTIME.environment, "NEW#", &variable)); + TEST_REQUIRE(variable != NULL, "the child must see the global the host created"); + + /* Twice is the same variable, not a second one shadowing the first. */ + { + akbasic_Variable *again = NULL; + PASS(errctx, akbasic_runtime_global(&HARNESS_RUNTIME, "NEW#", &again)); + TEST_REQUIRE(again == variable, "a repeat call must find the variable it created"); + } + + TEST_REQUIRE_STATUS(akbasic_runtime_global(NULL, "A#", &variable), AKERR_NULLPOINTER); + TEST_REQUIRE_STATUS(akbasic_runtime_global(&HARNESS_RUNTIME, NULL, &variable), + AKERR_NULLPOINTER); + TEST_REQUIRE_STATUS(akbasic_runtime_global(&HARNESS_RUNTIME, "A#", NULL), + AKERR_NULLPOINTER); + + TEST_REQUIRE_OK(akbasic_runtime_prev_environment(&HARNESS_RUNTIME)); + harness_stop(); + SUCCEED_RETURN(errctx); +} + +/** @brief Seeding before the script starts, and reading after it stops, still work. */ +static akerr_ErrorContext AKERR_NOIGNORE *test_seed_and_read_back(void) +{ + PREPARE_ERROR(errctx); + akbasic_Variable *variable = NULL; + akbasic_Value *value = NULL; + int64_t zerosubscript[1] = { 0 }; + + TEST_REQUIRE_OK(harness_start(NULL)); + PASS(errctx, set_global("SEED#", 10)); + + PASS(errctx, akbasic_runtime_load(&HARNESS_RUNTIME, "10 OUT# = SEED# * 2\n" + "20 PRINT OUT#\n")); + PASS(errctx, akbasic_runtime_start(&HARNESS_RUNTIME, AKBASIC_MODE_RUN)); + PASS(errctx, akbasic_runtime_run(&HARNESS_RUNTIME, 0)); + TEST_REQUIRE_STR(HARNESS_OUTPUT, "20\n"); + + PASS(errctx, akbasic_runtime_global(&HARNESS_RUNTIME, "OUT#", &variable)); + PASS(errctx, akbasic_variable_get_subscript(variable, zerosubscript, 1, &value)); + TEST_REQUIRE_INT(value->intval, 20); + + harness_stop(); + SUCCEED_RETURN(errctx); +} + +int main(void) +{ + PREPARE_ERROR(errctx); + + ATTEMPT { + CATCH(errctx, test_global_survives_a_for_body()); + CATCH(errctx, test_global_survives_a_gosub()); + CATCH(errctx, test_global_never_returns_null()); + CATCH(errctx, test_seed_and_read_back()); + } CLEANUP { + } PROCESS(errctx) { + } HANDLE_DEFAULT(errctx) { + LOG_ERROR_WITH_MESSAGE(errctx, "host variable test failed"); + akbasic_test_failures += 1; + } FINISH_NORETURN(errctx); + + return akbasic_test_failures; +}