andrew/akbasic
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refuse an operand the numeric path cannot read

Browse Source 
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>
This commit is contained in:
Andrew Kesterson
2026-08-01 11:22:50 -04:00
parent 3415c9aec6
commit 8117fb564d
5 changed files with 184 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
TODO.md
View File

@@ -1527,6 +1527,42 @@ the reference's semantics reproduced faithfully; the third is the port's own.
listings read it there. Same known-failing test as item 19; the fix is a scoping decision
about where a loop counter is created, not an ordering one.
### Found while auditing `akbasic_value_clone()` for the structures work
22. ~~**The numeric operators' `else` was a catch-all, not a float branch.**~~ **Fixed.**
`math_minus`, `math_multiply` and `math_divide` were `if ( INTEGER ) … else <treat as
float>`, and that `else` 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 other 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, deliberately.** The left one picks the branch
and must be a number. The right one is read through `rval_as_int()`, which handles a
truth value on purpose — `5 - (A# == 1)` is 6, and that is the same property that lets
`AND` and `OR` double as logical operators. Refusing it on the right would have broken
every condition in the language, and `tests/language/numeric/truth_value_arithmetic.bas`
pins both halves.
**Found by asking what a structure operand would do**, since `AKBASIC_TYPE_STRUCT` would
have taken exactly this path and read a `floatval` it does not carry. The structures
work did not create this defect; it made an already-reachable one worth chasing. Same
shape as item 5 and fixed the same way, with the assertions against the value API where
the bad value can actually be constructed.
Two stale comments went with it: `src/value.c`'s file header and a duplicate block above
`rval_as_int()` both cited "TODO.md section 12", which does not exist — the defect list
is §6 — and the duplicate described the summing behaviour item 5 had already removed.
### Found while writing the error-code appendix
21. **A dependency's status code reaches `ER#` where the interpreter has one of its own.**

94
src/value.c
View File

@@ -2,11 +2,10 @@
* @file value.c
* @brief Implements the BASIC value type and its operators.
*
* A faithful port of basicvalue.go. Where the reference does something
* arithmetically odd -- adding both of the right operand's numeric fields, for
* instance -- this reproduces it, because the golden corpus encodes the observed
* behaviour and a "fix" here is a silent behaviour change. Each one is catalogued
* in TODO.md section 12.
* Ported from basicvalue.go. It used to reproduce the reference's arithmetic
* oddities deliberately, because the golden corpus encoded the observed
* behaviour; TODO.md section 0.1 retired that constraint and section 6 records
* which ones have since been fixed and why.
*/
#include <inttypes.h>
@@ -18,13 +17,6 @@
#include <akbasic/error.h>
#include <akbasic/value.h>
/*
* The reference writes `rval.intval + int64(rval.floatval)` on every integer
* operation and the mirror image on every float one. It works only because the
* unused field of a typed value is always zero. Named so the intent -- "whatever
* numeric payload the right operand is carrying" -- is legible, and so there is
* one place to change when TODO.md section 12 item 5 is fixed.
*/
/*
* Selected on the type, not summed. The reference adds both numeric fields --
* `rval.intval + int64(rval.floatval)` -- which happens to give the right answer
@@ -55,6 +47,72 @@ static double rval_as_float(akbasic_Value *rval)
return (double)rval->intval;
}
/**
* @brief What a value's type is called in a diagnostic.
*
* Indexed by akbasic_Type, so a new type added to the enum without a row here
* trips the negative-array-size assert below rather than printing an empty
* string into an error message nobody can act on.
*/
static const char *TYPE_NAMES[] = {
"an undefined value", /* AKBASIC_TYPE_UNDEFINED */
"an integer", /* AKBASIC_TYPE_INTEGER */
"a float", /* AKBASIC_TYPE_FLOAT */
"a string", /* AKBASIC_TYPE_STRING */
"a truth value" /* AKBASIC_TYPE_BOOLEAN */
};
typedef char akbasic_assert_type_names_complete
[(sizeof(TYPE_NAMES) / sizeof(TYPE_NAMES[0]) == AKBASIC_TYPE_BOOLEAN + 1) ? 1 : -1];
static const char *type_name(akbasic_Type valuetype)
{
if ( valuetype < 0 || valuetype > AKBASIC_TYPE_BOOLEAN ) {
return "a value of unknown type";
}
return TYPE_NAMES[valuetype];
}
/**
* @brief Refuse an operand the numeric path cannot read.
*
* `math_minus`, `math_multiply` and `math_divide` are shaped
* `if ( INTEGER ) ... else <treat as float>`, and **that else is a catch-all
* rather than a float branch**: it reads `floatval` from whatever it is handed.
* A truth value carries its payload in `boolvalue` and leaves `floatval` zero,
* so `(A# == 1) - 1` computed `0 - 1` into a field nothing reads, left the type
* as BOOLEAN, and printed `true` instead of -2. Multiplication and division did
* the same; only `math_plus` escaped, because it enumerates its cases and ends
* in an error.
*
* Checked here rather than in each operator so there is one rule and one
* message, and so a type added later is refused by all three at once instead of
* silently taking the float branch in each.
*
* **The two operands have different rules, and deliberately.** The left one
* decides the branch, so it must be a number. The right one is read through
* rval_as_int()/rval_as_float(), which handle a truth value on purpose -- a
* comparison yields -1 or 0 and `5 - (A# == 1)` is 6, which is the same property
* that lets `AND` and `OR` double as logical operators. Refusing a truth value
* on the right would break that, so this permits exactly what those two readers
* can actually read and nothing else.
*/
static akerr_ErrorContext *require_numeric(akbasic_Value *self, akbasic_Value *rval, const char *what)
{
PREPARE_ERROR(errctx);
FAIL_ZERO_RETURN(errctx,
(self->valuetype == AKBASIC_TYPE_INTEGER ||
self->valuetype == AKBASIC_TYPE_FLOAT),
AKBASIC_ERR_TYPE, "Cannot perform %s on %s", what, type_name(self->valuetype));
FAIL_ZERO_RETURN(errctx,
(rval->valuetype == AKBASIC_TYPE_INTEGER ||
rval->valuetype == AKBASIC_TYPE_FLOAT ||
rval->valuetype == AKBASIC_TYPE_BOOLEAN),
AKBASIC_ERR_TYPE, "Cannot perform %s on %s", what, type_name(rval->valuetype));
SUCCEED_RETURN(errctx);
}
/* Copy a string into a value's inline buffer. Truncation is an error. */
static akerr_ErrorContext *set_string(akbasic_Value *dest, const char *src)
{
@@ -448,6 +506,7 @@ akerr_ErrorContext *akbasic_value_math_minus(akbasic_Value *self, akbasic_Value
FAIL_NONZERO_RETURN(errctx,
(self->valuetype == AKBASIC_TYPE_STRING || rval->valuetype == AKBASIC_TYPE_STRING),
AKBASIC_ERR_TYPE, "Cannot perform subtraction on strings");
PASS(errctx, require_numeric(self, rval, "subtraction"));
if ( self->valuetype == AKBASIC_TYPE_INTEGER ) {
(*dest)->intval = self->intval - rval_as_int(rval);
} else {
@@ -464,6 +523,7 @@ akerr_ErrorContext *akbasic_value_math_divide(akbasic_Value *self, akbasic_Value
FAIL_NONZERO_RETURN(errctx,
(self->valuetype == AKBASIC_TYPE_STRING || rval->valuetype == AKBASIC_TYPE_STRING),
AKBASIC_ERR_TYPE, "Cannot perform division on strings");
PASS(errctx, require_numeric(self, rval, "division"));
if ( self->valuetype == AKBASIC_TYPE_INTEGER ) {
/*
* Integer division by zero is UB in C where Go panics. Neither is
@@ -510,12 +570,16 @@ akerr_ErrorContext *akbasic_value_math_multiply(akbasic_Value *self, akbasic_Val
}
buf[offset] = '\0';
PASS(errctx, set_string(*dest, buf));
SUCCEED_RETURN(errctx);
}
/*
* Not an `else if`. The reference falls through to the numeric branches even
* for a string, where self->floatval is 0 and the write is a harmless no-op.
* Kept so the port is provably faithful.
* The reference falls through to the numeric branches even for a string,
* where self->floatval is 0 and the write lands in a field nothing reads.
* That was reproduced deliberately and is now a return, because the guard
* below would otherwise refuse the string repeat this branch just did --
* and the fallthrough was only ever provably harmless, never useful.
*/
PASS(errctx, require_numeric(self, rval, "multiplication"));
if ( self->valuetype == AKBASIC_TYPE_INTEGER ) {
(*dest)->intval = self->intval * rval_as_int(rval);
} else {

16
tests/language/numeric/truth_value_arithmetic.bas Normal file
View File

@@ -0,0 +1,16 @@
10 REM A truth value carries its payload in boolvalue, not floatval. The three
20 REM numeric operators were `if ( INTEGER ) ... else <treat as float>`, and
30 REM that else was a catch-all rather than a float branch -- so a truth value
40 REM on the LEFT computed 0 - 1 into a field nothing reads, kept its BOOLEAN
50 REM type, and printed `true` instead of -2. Silent, and wrong twice over.
60 A# = 1
70 PRINT (A# == 1)
80 REM On the RIGHT it stays legal and stays -1, which is the same property that
90 REM lets AND and OR double as logical operators. Refusing it here would have
100 REM broken every condition in the language.
110 PRINT 5 - (A# == 1)
120 PRINT 5 * (A# == 1)
130 PRINT 5 + (A# == 1)
140 PRINT 4 - (A# == 2)
150 REM And on the left it is refused by name rather than computed.
160 PRINT (A# == 1) - 1

7
tests/language/numeric/truth_value_arithmetic.txt Normal file
View File

@@ -0,0 +1,7 @@
true
6
-5
4
4
? 160 : RUNTIME ERROR Cannot perform subtraction on a truth value

46
tests/value_arithmetic.c
View File

@@ -249,6 +249,52 @@ int main(void)
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);