diff --git a/docs/14-architecture.md b/docs/14-architecture.md index e1b39a0..cc24a9f 100644 --- a/docs/14-architecture.md +++ b/docs/14-architecture.md @@ -380,8 +380,10 @@ no free, so anything drawn from it is spent for the life of the run — and scop returns a variable's *slot* without returning its storage. A scalar therefore does not draw from it at all: `akbasic_variable_init()` points a one-element non-`@` variable at its own `inlinevalue`, which is what makes a local, a `FOR` counter and a `DEF` parameter -free. Arrays and structures still spend, deliberately, because a pointer into a record is -allowed to outlive the scope that DIMmed it. +free. A pointer parameter is also free: it owns only its one-slot reference, so its +`inlinevalue` dies with the call while the target remains in the caller's storage. Arrays, +structures and by-value structure parameters still spend, deliberately, because a pointer +into a record is allowed to outlive the scope that DIMmed it. [Chapter 13](13-differences.md) states the same budget from a BASIC programmer's side. The per-environment three are reset at the top of every line, which is what makes diff --git a/docs/16-structures.md b/docs/16-structures.md index 0343040..b4e686a 100644 --- a/docs/16-structures.md +++ b/docs/16-structures.md @@ -266,6 +266,13 @@ The function saw 99; the caller still has 5. To change a caller's record on purp a pointer — `DEF POKEIT(P@ AS PTR TO CRATE)` — and reach through it with `->`. Neither is a special rule: both fall out of the parameter being assigned like any other variable. +A pointer parameter's own reference is kept in the call variable's inline slot, so repeated +calls do not spend the value pool. Its target remains the caller's structure. A by-value +structure parameter is different: its copied slots remain in the value pool because +`POINT Q@ AT B@` may retain a pointer to that copy after the function returns. Until escape +analysis can distinguish that case, **do not call a by-value structure-parameter function in +a loop**; bind a host global and rebind it per instance instead. + ## What is checked, and what is not A field name is checked against the set the type declared, and the refusal lists the diff --git a/include/akbasic/environment.h b/include/akbasic/environment.h index debbb2d..4895e81 100644 --- a/include/akbasic/environment.h +++ b/include/akbasic/environment.h @@ -246,6 +246,16 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_collect_subscripts(akbasi */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest); +/** + * @brief Create a variable slot without allocating value storage. + * + * Used when the caller knows the variable's representation before its first + * initialization, such as a structure parameter. The caller must initialize + * the variable before evaluating it. + */ +akerr_ErrorContext AKERR_NOIGNORE *akbasic_environment_create_empty(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/src/environment.c b/src/environment.c index 77a184a..27ac19d 100644 --- a/src/environment.c +++ b/src/environment.c @@ -306,7 +306,8 @@ akerr_ErrorContext *akbasic_environment_get(akbasic_Environment *obj, const char SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest) +static akerr_ErrorContext *environment_create_named(akbasic_Environment *obj, const char *varname, + akbasic_Variable **dest, bool initialize) { PREPARE_ERROR(errctx); akbasic_Variable *variable = NULL; @@ -340,12 +341,31 @@ akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const c PASS(errctx, aksl_strcpy(variable->name, sizeof(variable->name), varname)); variable->valuetype = AKBASIC_TYPE_UNDEFINED; variable->mutable_ = true; - PASS(errctx, akbasic_variable_init(variable, &obj->runtime->valuepool, sizes, 1)); + if ( initialize ) { + PASS(errctx, akbasic_variable_init(variable, &obj->runtime->valuepool, sizes, 1)); + } PASS(errctx, akbasic_symtab_set(&obj->variables, varname, variable, 0)); *dest = variable; SUCCEED_RETURN(errctx); } +akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest) +{ + PREPARE_ERROR(errctx); + + PASS(errctx, environment_create_named(obj, varname, dest, true)); + SUCCEED_RETURN(errctx); +} + +akerr_ErrorContext *akbasic_environment_create_empty(akbasic_Environment *obj, const char *varname, + akbasic_Variable **dest) +{ + PREPARE_ERROR(errctx); + + PASS(errctx, environment_create_named(obj, varname, dest, false)); + SUCCEED_RETURN(errctx); +} + /* * Evaluate an lvalue's subscript list, if it has one, into `subscripts`. A bare * identifier yields the single subscript {0}, which is how a scalar is addressed diff --git a/src/runtime.c b/src/runtime.c index 27a0a29..08ab6e2 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -989,12 +989,13 @@ static akerr_ErrorContext *bind_structure_parameter(akbasic_Runtime *obj, akbasi obj->structtypes.types[typeindex].name, obj->structtypes.types[argvalue->structtype].name); - PASS(errctx, akbasic_environment_create(callenv, param->identifier, &variable)); + PASS(errctx, akbasic_environment_create_empty(callenv, param->identifier, &variable)); sizes[0] = (ispointer ? 1 : obj->structtypes.types[typeindex].slotcount); + /* A pointer parameter's own reference cannot escape its call scope. */ + variable->ispointer = ispointer; PASS(errctx, akbasic_variable_init(variable, &obj->valuepool, sizes, 1)); variable->valuetype = AKBASIC_TYPE_STRUCT; variable->structtype = typeindex; - variable->ispointer = ispointer; if ( ispointer ) { PASS(errctx, akbasic_value_clone(argvalue, &variable->values[0])); diff --git a/src/variable.c b/src/variable.c index f3d06d0..b207db9 100644 --- a/src/variable.c +++ b/src/variable.c @@ -99,18 +99,21 @@ akerr_ErrorContext *akbasic_variable_init(akbasic_Variable *obj, akbasic_ValuePo * the run was over, which a game loop reaches in half a minute. TODO.md * section 6 item 30 has the whole reduction. * - * **A `@` name is the one exclusion**, and it is the whole of it. A - * structure or a pointer to one keeps pool storage because a pointer may - * outlive the scope that DIMmed it -- docs/16-structures.md says nothing is - * reclaimed and akbasic_runtime_prev_environment() relies on it. The suffix - * is the right test rather than `structtype`, which the DIM path sets + * **A `@` name is the one exclusion**, except for a one-slot pointer + * parameter. A structure or a pointer variable keeps pool storage because + * a pointer may outlive the scope that DIMmed it -- docs/16-structures.md + * says nothing is reclaimed and akbasic_runtime_prev_environment() relies + * on it. A pointer parameter owns only its reference slot; the target is + * owned by the caller, so bind_structure_parameter() marks it before this + * call and lets that slot use inline storage. The suffix is the right test + * for every other case rather than `structtype`, which the DIM path sets * *after* calling this. * * Otherwise: reuse the existing slice when it is already big enough, which * makes a re-DIM to the same or a smaller size free; growing takes fresh * slots and abandons the old ones, as documented on akbasic_ValuePool. */ - if ( totalsize == 1 && lastchar != '@' ) { + if ( totalsize == 1 && (lastchar != '@' || obj->ispointer) ) { obj->values = &obj->inlinevalue; } else if ( obj->values == NULL || obj->valuecount < (int)totalsize ) { PASS(errctx, akbasic_valuepool_take(pool, (int)totalsize, &obj->values)); diff --git a/tests/user_functions.c b/tests/user_functions.c index d2d60d5..0216eff 100644 --- a/tests/user_functions.c +++ b/tests/user_functions.c @@ -183,6 +183,33 @@ static void test_structure_parameters(void) harness_stop(); } +/** + * @brief Pointer parameters do not consume value-pool slots per call. + * + * The pointer's parameter variable owns only one reference to the caller's + * record, so that reference can live in the variable's inline slot. The target + * remains in the caller's storage. One pointer parameter and 8,000 calls make + * one leaked slot per call fail against the 4,096-slot value pool. + */ +static void test_pointer_parameters_do_not_leak_value_slots(void) +{ + TEST_REQUIRE_OK(run_program_bounded("10 TYPE CRATE\n" + "20 W#\n" + "30 END TYPE\n" + "40 DIM A@ AS CRATE\n" + "50 DIM P@ AS PTR TO CRATE\n" + "60 POINT P@ AT A@\n" + "70 DEF POKEIT(P@ AS PTR TO CRATE)\n" + "80 P@->W# = P@->W# + 1\n" + "90 RETURN P@->W#\n" + "100 FOR I# = 1 TO 8000\n" + "110 R# = POKEIT(P@)\n" + "120 NEXT I#\n" + "130 PRINT A@.W#\n", 1000000)); + TEST_REQUIRE_STR(HARNESS_OUTPUT, "8000\n"); + harness_stop(); +} + /** * @brief A structure parameter must name its type, and the type is checked. * @@ -347,6 +374,7 @@ int main(void) test_runaway_recursion_is_diagnosed(); test_single_expression_form(); test_structure_parameters(); + test_pointer_parameters_do_not_leak_value_slots(); test_structure_parameter_types_are_checked(); test_call_scopes_are_reclaimed(); test_calls_do_not_leak_value_slots();