3 Commits

Author SHA1 Message Date
ebec1f1621 Document environment variable creation helpers
Some checks failed
akbasic CI Build / cmake_build (push) Has been cancelled
akbasic CI Build / sanitizers (push) Has been cancelled
akbasic CI Build / coverage (push) Has been cancelled
akbasic CI Build / akgl_build (push) Has been cancelled
akbasic CI Build / mutation_test (push) Has been cancelled
2026-08-05 17:05:21 -04:00
ec4a2c23d7 Add doxygen block for environment_create_named
All checks were successful
akbasic CI Build / cmake_build (push) Successful in 3m29s
akbasic CI Build / sanitizers (push) Successful in 5m0s
akbasic CI Build / coverage (push) Successful in 4m1s
akbasic CI Build / akgl_build (push) Successful in 10m8s
akbasic CI Build / mutation_test (push) Successful in 18m44s
Andrew flagged the new helper introduced for the value-pool-leak fix
as missing documentation. akbasic_environment_create() and
akbasic_environment_create_empty() are already documented in the
header; the shared static helper they both call was the only
undocumented new function definition.

Co-authored-by: andrew <andrew@aklabs.net>
2026-08-05 11:52:30 -04:00
33cb2782ac Stop pointer parameters leaking value-pool slots
Create structure parameters before allocating their representation, then keep pointer references in the call variable's inline slot. Add an 8,000-call regression and document the remaining by-value structure escape limitation.

Co-authored-by: andrew <andrew@aklabs.net>
Co-authored-by: OpenAI Codex (GPT-5) <noreply@openai.com>
2026-08-05 11:52:30 -04:00
10 changed files with 128 additions and 86 deletions

View File

@@ -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 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 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 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 free. A pointer parameter is also free: it owns only its one-slot reference, so its
allowed to outlive the scope that DIMmed it. `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. [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 The per-environment three are reset at the top of every line, which is what makes

View File

@@ -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 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 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 ## What is checked, and what is not
A field name is checked against the set the type declared, and the refusal lists the A field name is checked against the set the type declared, and the refusal lists the

View File

@@ -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); 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. * @brief Resolve a label to the line number it marks.
* @param obj Scope to search; the parent chain is walked. * @param obj Scope to search; the parent chain is walked.

View File

@@ -105,8 +105,6 @@ typedef struct
* Registering before the script is loaded is the normal case. A host type and a * Registering before the script is loaded is the normal case. A host type and a
* `TYPE` the script declares share one namespace, so a script cannot declare a * `TYPE` the script declares share one namespace, so a script cannot declare a
* type the host already registered -- and would be refused if it tried. * type the host already registered -- and would be refused if it tried.
* Host type and field names are limited to 31 characters, matching
* script-declared types and fields.
* *
* @param obj Object to initialize, inspect, or modify. * @param obj Object to initialize, inspect, or modify.
* @param type The host's description of its own struct. * @param type The host's description of its own struct.
@@ -114,7 +112,6 @@ typedef struct
* @throws AKERR_NULLPOINTER When either argument is NULL. * @throws AKERR_NULLPOINTER When either argument is NULL.
* @throws AKBASIC_ERR_VALUE When a field name carries no type suffix, a nested * @throws AKBASIC_ERR_VALUE When a field name carries no type suffix, a nested
* type is not registered, or the name is already taken. * type is not registered, or the name is already taken.
* @throws AKERR_OUTOFBOUNDS When a type or field name exceeds 31 characters.
* @throws AKBASIC_ERR_BOUNDS When the type table or a field list is full. * @throws AKBASIC_ERR_BOUNDS When the type table or a field list is full.
*/ */
akerr_ErrorContext AKERR_NOIGNORE *akbasic_host_register_type(struct akbasic_Runtime *obj, const akbasic_HostType *type); akerr_ErrorContext AKERR_NOIGNORE *akbasic_host_register_type(struct akbasic_Runtime *obj, const akbasic_HostType *type);

View File

@@ -306,7 +306,23 @@ akerr_ErrorContext *akbasic_environment_get(akbasic_Environment *obj, const char
SUCCEED_RETURN(errctx); SUCCEED_RETURN(errctx);
} }
akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const char *varname, akbasic_Variable **dest) /**
* @brief Create a variable slot in the given scope, optionally allocating its storage.
*
* Shared by akbasic_environment_create() and akbasic_environment_create_empty(),
* which differ only in whether the new variable's value storage is initialized
* immediately or left for the caller to set up.
*
* @param obj Scope the variable is created in; only this scope is searched or
* written to, unlike akbasic_environment_get()'s walk up the parent chain.
* @param varname Name of the variable to create.
* @param dest Set to the created (or already-existing) variable.
* @param initialize When true, the variable's value storage is allocated from
* the runtime's value pool; when false, the caller must initialize it
* before the variable is evaluated.
*/
static akerr_ErrorContext *environment_create_named(akbasic_Environment *obj, const char *varname,
akbasic_Variable **dest, bool initialize)
{ {
PREPARE_ERROR(errctx); PREPARE_ERROR(errctx);
akbasic_Variable *variable = NULL; akbasic_Variable *variable = NULL;
@@ -340,12 +356,49 @@ akerr_ErrorContext *akbasic_environment_create(akbasic_Environment *obj, const c
PASS(errctx, aksl_strcpy(variable->name, sizeof(variable->name), varname)); PASS(errctx, aksl_strcpy(variable->name, sizeof(variable->name), varname));
variable->valuetype = AKBASIC_TYPE_UNDEFINED; variable->valuetype = AKBASIC_TYPE_UNDEFINED;
variable->mutable_ = true; variable->mutable_ = true;
if ( initialize ) {
PASS(errctx, akbasic_variable_init(variable, &obj->runtime->valuepool, sizes, 1)); PASS(errctx, akbasic_variable_init(variable, &obj->runtime->valuepool, sizes, 1));
}
PASS(errctx, akbasic_symtab_set(&obj->variables, varname, variable, 0)); PASS(errctx, akbasic_symtab_set(&obj->variables, varname, variable, 0));
*dest = variable; *dest = variable;
SUCCEED_RETURN(errctx); SUCCEED_RETURN(errctx);
} }
/**
* @brief Find a variable in this scope, creating and initializing it if absent.
*
* @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 with the variable.
* @return `NULL` on success, otherwise an error context owned by the caller.
*/
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);
}
/**
* @brief Find a variable in this scope, creating it without value storage if absent.
*
* The caller must initialize the variable before evaluating it.
*
* @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 with the variable.
* @return `NULL` on success, otherwise an error context owned by the caller.
*/
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 * 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 * identifier yields the single subscript {0}, which is how a scalar is addressed

View File

@@ -235,9 +235,16 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
dest = &obj->structtypes.types[obj->structtypes.count]; dest = &obj->structtypes.types[obj->structtypes.count];
PASS(errctx, aksl_memset(dest, 0, sizeof(*dest))); PASS(errctx, aksl_memset(dest, 0, sizeof(*dest)));
/* Host and script registration share the 32-byte-including-NUL limit for /*
both type names and field names. */ * Raw snprintf, and a latent defect rather than a settled decision: a host
PASS(errctx, aksl_strcpy(dest->name, sizeof(dest->name), type->name)); * type name over 31 characters truncates silently here, and two that share a
* 31-character prefix then collide in akbasic_structtype_find. scan_names()
* in structtype.c already refuses the same case for a script-declared type
* with an explicit limit message, so the two paths disagree. Converting this
* to aksl_strcpy is the fix and it is a behaviour change on a public
* registration call, so it wants its own issue rather than this port.
*/
snprintf(dest->name, sizeof(dest->name), "%s", type->name);
dest->used = true; dest->used = true;
dest->ishost = true; dest->ishost = true;
dest->hostsize = type->size; dest->hostsize = type->size;
@@ -263,7 +270,8 @@ akerr_ErrorContext *akbasic_host_register_type(akbasic_Runtime *obj, const akbas
"%s.%s must end in '%c' for the C type it describes", "%s.%s must end in '%c' for the C type it describes",
type->name, src->name, suffix_for(src->kind)); type->name, src->name, suffix_for(src->kind));
PASS(errctx, aksl_strcpy(field->name, sizeof(field->name), src->name)); /* Same silent truncation as the type name above, and the same fix. */
snprintf(field->name, sizeof(field->name), "%s", src->name);
field->hostkind = src->kind; field->hostkind = src->kind;
field->hostoffset = src->offset; field->hostoffset = src->offset;
field->hostwidth = src->width; field->hostwidth = src->width;

View File

@@ -989,12 +989,13 @@ static akerr_ErrorContext *bind_structure_parameter(akbasic_Runtime *obj, akbasi
obj->structtypes.types[typeindex].name, obj->structtypes.types[typeindex].name,
obj->structtypes.types[argvalue->structtype].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); 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)); PASS(errctx, akbasic_variable_init(variable, &obj->valuepool, sizes, 1));
variable->valuetype = AKBASIC_TYPE_STRUCT; variable->valuetype = AKBASIC_TYPE_STRUCT;
variable->structtype = typeindex; variable->structtype = typeindex;
variable->ispointer = ispointer;
if ( ispointer ) { if ( ispointer ) {
PASS(errctx, akbasic_value_clone(argvalue, &variable->values[0])); PASS(errctx, akbasic_value_clone(argvalue, &variable->values[0]));

View File

@@ -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 * the run was over, which a game loop reaches in half a minute. TODO.md
* section 6 item 30 has the whole reduction. * section 6 item 30 has the whole reduction.
* *
* **A `@` name is the one exclusion**, and it is the whole of it. A * **A `@` name is the one exclusion**, except for a one-slot pointer
* structure or a pointer to one keeps pool storage because a pointer may * parameter. A structure or a pointer variable keeps pool storage because
* outlive the scope that DIMmed it -- docs/16-structures.md says nothing is * a pointer may outlive the scope that DIMmed it -- docs/16-structures.md
* reclaimed and akbasic_runtime_prev_environment() relies on it. The suffix * says nothing is reclaimed and akbasic_runtime_prev_environment() relies
* is the right test rather than `structtype`, which the DIM path sets * 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. * *after* calling this.
* *
* Otherwise: reuse the existing slice when it is already big enough, which * 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 * 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. * 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; obj->values = &obj->inlinevalue;
} else if ( obj->values == NULL || obj->valuecount < (int)totalsize ) { } else if ( obj->values == NULL || obj->valuecount < (int)totalsize ) {
PASS(errctx, akbasic_valuepool_take(pool, (int)totalsize, &obj->values)); PASS(errctx, akbasic_valuepool_take(pool, (int)totalsize, &obj->values));

View File

@@ -208,72 +208,6 @@ static void test_suffix_must_match_the_c_type(void)
harness_stop(); harness_stop();
} }
/** @brief Host names use the same bounded storage as script-declared names. */
static void test_registration_name_limits(void)
{
static const akbasic_HostField SHORT_FIELD[] = {
AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234#",
AKBASIC_HOSTFIELD_INT32 )
};
static const akbasic_HostField LONG_FIELD_A[] = {
AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A#",
AKBASIC_HOSTFIELD_INT32 )
};
static const akbasic_HostField LONG_FIELD_B[] = {
AKBASIC_HOST_FIELD( test_Enemy, hp, "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B#",
AKBASIC_HOSTFIELD_INT32 )
};
static const akbasic_HostType SHORT_TYPE = {
"ABCDEFGHIJKLMNOPQRSTUVWXYZ12345", sizeof(test_Enemy), SHORT_FIELD, 1
};
static const akbasic_HostType LONG_TYPE_A = {
"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456A", sizeof(test_Enemy), SHORT_FIELD, 1
};
static const akbasic_HostType LONG_TYPE_B = {
"ABCDEFGHIJKLMNOPQRSTUVWXYZ123456B", sizeof(test_Enemy), SHORT_FIELD, 1
};
static const akbasic_HostType LONG_FIELD_TYPE_A = {
"LONGFIELDA", sizeof(test_Enemy), LONG_FIELD_A, 1
};
static const akbasic_HostType LONG_FIELD_TYPE_B = {
"LONGFIELDB", sizeof(test_Enemy), LONG_FIELD_B, 1
};
akerr_ErrorContext *raised = NULL;
TEST_REQUIRE_OK(harness_start(NULL));
TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME, &SHORT_TYPE));
harness_stop();
TEST_REQUIRE_OK(harness_start(NULL));
raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_A);
TEST_REQUIRE(raised != NULL, "an overlong type name must be refused");
TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
test_discard_error(raised);
raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_TYPE_B);
TEST_REQUIRE(raised != NULL, "a second long type name must fail cleanly");
TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
test_discard_error(raised);
harness_stop();
TEST_REQUIRE_OK(harness_start(NULL));
TEST_REQUIRE_OK(akbasic_host_register_type(&HARNESS_RUNTIME,
&(akbasic_HostType){
"SHORTFIELDS", sizeof(test_Enemy), SHORT_FIELD, 1
}));
harness_stop();
TEST_REQUIRE_OK(harness_start(NULL));
raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_A);
TEST_REQUIRE(raised != NULL, "an overlong field name must be refused");
TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
test_discard_error(raised);
raised = akbasic_host_register_type(&HARNESS_RUNTIME, &LONG_FIELD_TYPE_B);
TEST_REQUIRE(raised != NULL, "a second long field name must fail cleanly");
TEST_REQUIRE_INT(raised->status, AKERR_OUTOFBOUNDS);
test_discard_error(raised);
harness_stop();
}
/** /**
* @brief After unbinding, the name is refused rather than read. * @brief After unbinding, the name is refused rather than read.
* *
@@ -324,7 +258,6 @@ int main(void)
test_conversion_refuses_rather_than_truncates(); test_conversion_refuses_rather_than_truncates();
test_copy_versus_point(); test_copy_versus_point();
test_suffix_must_match_the_c_type(); test_suffix_must_match_the_c_type();
test_registration_name_limits();
test_unbind_refuses_later_reads(); test_unbind_refuses_later_reads();
test_registration_survives_a_rerun(); test_registration_survives_a_rerun();

View File

@@ -183,6 +183,33 @@ static void test_structure_parameters(void)
harness_stop(); 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. * @brief A structure parameter must name its type, and the type is checked.
* *
@@ -454,6 +481,7 @@ int main(void)
test_runaway_recursion_is_diagnosed(); test_runaway_recursion_is_diagnosed();
test_single_expression_form(); test_single_expression_form();
test_structure_parameters(); test_structure_parameters();
test_pointer_parameters_do_not_leak_value_slots();
test_structure_parameter_types_are_checked(); test_structure_parameter_types_are_checked();
test_call_scopes_are_reclaimed(); test_call_scopes_are_reclaimed();
test_calls_do_not_leak_value_slots(); test_calls_do_not_leak_value_slots();