Files
akbasic/tests/value_arithmetic.c
Andrew Kesterson 8117fb564d Refuse an operand the numeric path cannot read
math_minus, math_multiply and math_divide were `if ( INTEGER ) ... else <treat
as float>`, and that else was a catch-all rather than a float branch: it read
floatval from whatever it was handed. A truth value keeps its payload in
boolvalue and leaves floatval zero, so a truth value on the left computed into a
field nothing reads and kept its BOOLEAN type --

    (A# == 1) - 1   printed true, should be -2
    (A# == 1) * 3   printed true, should be -3
    (A# == 1) + 1   correctly refused

wrong in value and in type, and silent. math_plus escaped only because it
enumerates its cases and ends in an error.

The three now share one require_numeric() guard, so a type added later is
refused by all of them at once instead of quietly taking the float branch in
each. The two operands have different rules and that asymmetry is the point: the
left one picks the branch and must be a number, while the right is read through
rval_as_int(), which handles -1/0 deliberately. `5 - (A# == 1)` is 6, and that
is the same property that lets AND and OR double as logical operators, so
refusing a truth value on the right would have broken every condition in the
language.

Found by asking what a structure operand would do to this path, which is where
AKBASIC_TYPE_STRUCT is about to arrive. The structures work did not create the
defect; it made an already-reachable one worth chasing.

Two stale comments went with it. src/value.c's header and a duplicate block
above rval_as_int both cited "TODO.md section 12", which does not exist -- the
defect list is section 6 -- and the duplicate described the summing behaviour
item 5 had already removed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 11:22:50 -04:00

316 lines
13 KiB
C

/**
* @file value_arithmetic.c
* @brief Tests the arithmetic operators, including the parts that look wrong.
*
* Where the reference does something odd, the assertion here pins the reference's
* answer, not the tidy one. A "fix" that changes any of these changes observable
* BASIC behaviour and belongs in its own commit with its own TODO.md entry.
*/
#include <string.h>
#include <akbasic/error.h>
#include <akbasic/value.h>
#include "testutil.h"
static akbasic_Value A;
static akbasic_Value B;
static akbasic_Value SCRATCH;
static void set_int(akbasic_Value *v, int64_t n)
{
test_discard_error(akbasic_value_zero(v));
v->valuetype = AKBASIC_TYPE_INTEGER;
v->intval = n;
}
static void set_float(akbasic_Value *v, double n)
{
test_discard_error(akbasic_value_zero(v));
v->valuetype = AKBASIC_TYPE_FLOAT;
v->floatval = n;
}
static void set_string(akbasic_Value *v, const char *s)
{
test_discard_error(akbasic_value_zero(v));
v->valuetype = AKBASIC_TYPE_STRING;
strncpy(v->stringval, s, AKBASIC_MAX_STRING_LENGTH - 1);
}
/**
* @brief A string that exactly fills the value's inline buffer round-trips.
*
* Written because mutation testing said nothing would have noticed if it did
* not: `AKBASIC_MAX_STRING_LENGTH - 1` mutated to `+ 1` and to `- 0` survived at
* both the strncpy and the NUL terminator in set_string(), and no test in the
* suite wrote a maximum-length string. Both mutants are a real bug of that shape
* -- one truncates a string that fits, the other writes one byte past the end of
* a 256-byte buffer -- and the boundary is the only place either shows.
*
* Concatenation rather than a direct set, because set_string() is static and
* concatenation is how every string in a BASIC program actually gets built.
*
* **The two operands are deliberately different lengths and different letters.**
* A first attempt joined a full-length string to an empty one, and the
* truncating mutant survived it: math_plus clones self into the scratch before
* writing, so the byte a short copy failed to write was already the right one.
* The parts have to sum to the limit without either of them being the answer.
*/
static void test_maximum_length_string(void)
{
enum { LIMIT = AKBASIC_MAX_STRING_LENGTH - 1, HEAD = LIMIT - 55, TAIL = 55 };
akbasic_Value *out = NULL;
char head[AKBASIC_MAX_STRING_LENGTH];
char tail[AKBASIC_MAX_STRING_LENGTH];
char joined[AKBASIC_MAX_STRING_LENGTH];
memset(head, 'X', HEAD);
head[HEAD] = '\0';
memset(tail, 'Z', TAIL);
tail[TAIL] = '\0';
/* memcpy rather than snprintf: the lengths are known here and -Wformat-truncation
* cannot see that HEAD + TAIL is exactly the buffer's capacity. */
memcpy(joined, head, HEAD);
memcpy(joined + HEAD, tail, TAIL);
joined[HEAD + TAIL] = '\0';
/* Exactly the limit: every byte lands, and the terminator is where it should be. */
set_string(&A, head);
set_string(&B, tail);
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(strlen(out->stringval), LIMIT);
TEST_REQUIRE_STR(out->stringval, joined);
TEST_REQUIRE_INT(out->stringval[LIMIT - 1], 'Z');
TEST_REQUIRE_INT(out->stringval[LIMIT], '\0');
/* One character past it is an error, not a silent truncation. */
set_string(&A, joined);
set_string(&B, "!");
TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_VALUE);
/* The same boundary through the repeat operator, which has its own copy. */
memset(head, 'Q', LIMIT / 2);
head[LIMIT / 2] = '\0';
set_string(&A, head);
set_int(&B, 2);
TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(strlen(out->stringval), (LIMIT / 2) * 2);
TEST_REQUIRE_INT(out->stringval[((LIMIT / 2) * 2) - 1], 'Q');
}
/**
* @brief A freshly initialised value pool is empty, and its storage is zeroed.
*
* Also a mutation-driven test: `memset(obj, 0, ...)` mutated to `memset(obj, 1,
* ...)`, the memset deleted outright, and `obj->next = 0` mutated to `= 1` all
* survived, because nothing asserted what an initialised pool looks like. The
* deleted-memset mutant is the one that matters -- a pool over stale stack
* memory hands a BASIC program somebody else's array contents.
*/
static void test_pool_starts_empty(void)
{
static akbasic_ValuePool pool;
akbasic_Value *slice = NULL;
int i = 0;
/* Dirty every byte first, so a missing memset cannot pass by luck. */
memset(&pool, 0xAB, sizeof(pool));
TEST_REQUIRE_OK(akbasic_valuepool_init(&pool));
TEST_REQUIRE_INT(pool.next, 0);
for ( i = 0; i < AKBASIC_MAX_ARRAY_VALUES; i++ ) {
if ( pool.values[i].valuetype != 0 || pool.values[i].intval != 0 ||
pool.values[i].stringval[0] != '\0' ) {
TEST_REQUIRE(false, "pool slot %d was not zeroed by init", i);
break;
}
}
/* The first take starts at slot zero and the bump advances by exactly count. */
TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 4, &slice));
TEST_REQUIRE(slice == &pool.values[0], "the first take must start at slot zero");
TEST_REQUIRE_INT(pool.next, 4);
TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 1, &slice));
TEST_REQUIRE(slice == &pool.values[4], "the second take must start where the first ended");
TEST_REQUIRE_INT(pool.next, 5);
/* Re-initialising takes it back to empty rather than merely rewinding. */
TEST_REQUIRE_OK(akbasic_valuepool_init(&pool));
TEST_REQUIRE_INT(pool.next, 0);
TEST_REQUIRE_STATUS(akbasic_valuepool_init(NULL), AKERR_NULLPOINTER);
TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, 0, &slice), AKBASIC_ERR_BOUNDS);
TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, AKBASIC_MAX_ARRAY_VALUES + 1, &slice),
AKBASIC_ERR_BOUNDS);
}
int main(void)
{
akbasic_Value *out = NULL;
char rendered[AKBASIC_MAX_STRING_LENGTH];
TEST_REQUIRE_OK(akbasic_error_register());
/* Integer arithmetic. */
set_int(&A, 7);
set_int(&B, 3);
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 10);
TEST_REQUIRE_OK(akbasic_value_math_minus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 4);
TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 21);
TEST_REQUIRE_OK(akbasic_value_math_divide(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 2); /* truncating, as C and Go both do */
/* Float arithmetic. */
set_float(&A, 1.20);
set_float(&B, 0.4);
TEST_REQUIRE_OK(akbasic_value_math_divide(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_OK(akbasic_value_to_string(out, rendered, sizeof(rendered)));
/* tests/language/arithmetic/float.txt says 3.000000, not 2.999999. */
TEST_REQUIRE_STR(rendered, "3.000000");
/* String concatenation, including the mixed-type forms. */
set_string(&A, "SORTED IN ");
set_int(&B, 4);
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_STR(out->stringval, "SORTED IN 4");
set_string(&A, "X=");
set_float(&B, 2.5);
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_STR(out->stringval, "X=2.500000");
/* String multiplication is repetition. */
set_string(&A, "ab");
set_int(&B, 3);
TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_STR(out->stringval, "ababab");
/*
* math_plus clones like every other operator, mutable operand or not.
*
* This used to assert the opposite. The reference mutates `self` in place
* when it is mutable, which made `A# + 1` modify `A#` whenever the left
* operand came out of a variable -- TODO.md section 6 item 4. It was left
* alone because NEXT's loop increment relied on the mutation to advance the
* counter; NEXT writes the result back itself now, so the asymmetry is gone
* and this asserts that it stays gone.
*/
set_int(&A, 10);
A.mutable_ = true;
set_int(&B, 5);
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE(out == &SCRATCH, "math_plus must use the scratch even for a mutable operand");
TEST_REQUIRE_INT(A.intval, 10);
TEST_REQUIRE_INT(SCRATCH.intval, 15);
set_int(&A, 10);
A.mutable_ = false;
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE(out == &SCRATCH, "math_plus on an immutable value must use the scratch");
TEST_REQUIRE_INT(A.intval, 10);
/*
* The unused numeric field is ignored, not added in.
*
* The reference reads a right-hand operand as `intval + int64(floatval)`,
* which gives the right answer only for as long as whichever field is unused
* happens to be zero -- TODO.md section 6 item 5, filed as a landmine
* because no program can reach it. It is reachable from here: a value is a
* plain struct and nothing stops one carrying both. Without this the fix is
* untested, and the old code passes every other test in the tree.
*/
set_int(&A, 2);
set_int(&B, 5);
B.floatval = 3.0; /* stale, from whatever B held before */
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 7);
set_float(&A, 2.0);
set_float(&B, 5.0);
B.intval = 3; /* stale the other way round */
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_FEQ(out->floatval, 7.0);
/* And a truth value used as a number is -1, which is what AND/OR rely on. */
set_int(&A, 0);
TEST_REQUIRE_OK(akbasic_value_set_bool(&B, true));
TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, -1);
/* Type errors. */
set_string(&A, "text");
set_int(&B, 1);
TEST_REQUIRE_STATUS(akbasic_value_math_minus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_divide(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_invert(&A, &SCRATCH, &out), AKBASIC_ERR_TYPE);
/*
* A left operand the numeric path cannot read is refused, not computed.
*
* `math_minus`, `math_multiply` and `math_divide` were `if ( INTEGER ) ...
* else <treat as float>`, and that else was a catch-all: it read `floatval`
* from whatever it was handed. A truth value keeps its payload in
* `boolvalue` and leaves `floatval` zero, so the operation computed into a
* field nothing reads and left the type alone -- `(A# == 1) - 1` rendered
* `true` rather than -2. Silent, and wrong in both value and type.
*
* `math_plus` never had it, because it enumerates its cases and ends in an
* error. These assert the other three now agree with it.
*/
TEST_REQUIRE_OK(akbasic_value_set_bool(&A, true));
set_int(&B, 1);
TEST_REQUIRE_STATUS(akbasic_value_math_minus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_divide(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
/* An undefined value is refused for the same reason and by the same guard. */
TEST_REQUIRE_OK(akbasic_value_zero(&A));
TEST_REQUIRE_STATUS(akbasic_value_math_minus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
TEST_REQUIRE_STATUS(akbasic_value_math_divide(&A, &B, &SCRATCH, &out), AKBASIC_ERR_TYPE);
/*
* But a truth value on the *right* stays legal, and that asymmetry is the
* point: the left operand picks the branch and must be a number, while the
* right is read through rval_as_int(), which handles -1/0 on purpose. It is
* the same property that lets AND and OR double as logical operators, so
* refusing it here would break `5 - (A# == 1)`, which is 6.
*/
set_int(&A, 5);
TEST_REQUIRE_OK(akbasic_value_set_bool(&B, true));
TEST_REQUIRE_OK(akbasic_value_math_minus(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, 6);
TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, -5);
/* And a string repeat is not caught by the numeric guard in front of it. */
set_string(&A, "xy");
set_int(&B, 2);
TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
TEST_REQUIRE_STR(out->stringval, "xyxy");
/* Division by zero raises rather than trapping or panicking. */
set_int(&A, 1);
set_int(&B, 0);
TEST_REQUIRE_STATUS(akbasic_value_math_divide(&A, &B, &SCRATCH, &out), AKBASIC_ERR_VALUE);
/* Unary minus. */
set_int(&A, 42);
TEST_REQUIRE_OK(akbasic_value_invert(&A, &SCRATCH, &out));
TEST_REQUIRE_INT(out->intval, -42);
/* nil rval is rejected everywhere. */
TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, NULL, &SCRATCH, &out), AKERR_NULLPOINTER);
test_maximum_length_string();
test_pool_starts_empty();
return akbasic_test_failures;
}