diff --git a/TODO.md b/TODO.md index 5cee555..5f8d236 100644 --- a/TODO.md +++ b/TODO.md @@ -767,54 +767,89 @@ asserts the correct contract. Until fixed, the correct-contract test lives in `stopWaiting`, leaving the parent waiting on a `NEXT` that will never come. 9. **`basicruntime.go:149`** — `newVariable()` and `BasicRuntime.variables[MAX_VARIABLES]` are dead; variables live in the environment's map. Do not port them. -10. **`basicgrammar.go:224`** — `newLiteralInt` selects base 8 whenever the lexeme starts with - `0`. *Consequence:* `PRINT 08` is a parse error and `PRINT 010` prints 8. Commodore BASIC - has no octal literals. Fix: base 10 unless prefixed `0x`. +10. ~~**`basicgrammar.go:224`** — `newLiteralInt` selects base 8 whenever the lexeme starts + with `0`.~~ **Fixed.** Base 10 unless prefixed `0x`; a leading zero in a listing is padding, + not a radix. `PRINT 010` prints 10 and `PRINT 08` parses. Nothing in the upstream corpus + had a leading-zero literal, which is why nothing there noticed. Regression tests in + `tests/grammar_leaves.c` and `tests/language/numeric/octal_literal.bas`, which was rewritten + from pinning the defect to pinning the fix. 11. **`basicruntime.go:121` / `basicparser_commands.go:124`** — environments are never freed. Addressed structurally by §1.4; no separate fix needed, but the C pool **will** exhaust where Go merely leaked, so `tests/environment_scope.c`'s exhaustion assertion matters. ### Found during the port -Five more, none of which the reference's own corpus catches. Each is reproduced faithfully, -pinned by an assertion in the relevant unit test, and asserted *correctly* in -`tests/known_reference_defects.c`. +Five more, none of which the reference's own corpus catches. **Four are now fixed**, each with +a regression test in the suite that pinned it; the fifth is described below and is not a coding +question. -12. **`basicparser.go:329` — `subtraction()` returns after one operator where `addition()` - loops, so `1 - 2 - 3` computes `1 - 2` and abandons the rest of the line.** The remaining - `- 3` is left in the token stream and the statement loop picks it up as a second statement, - which evaluates and is discarded. *Consequence:* a chain of two or more subtractions - silently produces the wrong answer. This is the highest-impact item on the list — it is a - wrong result, not a refused one. The same shape appears in `exponent()`, so `2 ^ 3 ^ 2` - has it too. Fix: make both loop the way `addition()` does. Pinned in - `tests/parser_expressions.c`. +12. ~~**`basicparser.go:329` — `subtraction()` returns after one operator where `addition()` + loops.**~~ **Fixed.** `1 - 2 - 3` computed `1 - 2` and abandoned the rest of the line; the + remaining `- 3` was left in the token stream, picked up by the statement loop as a second + statement, evaluated and discarded. A wrong answer, silently, which made this the + highest-impact item on the list. `exponent()` has the identical shape — a `for` loop with a + `return` inside it, in both the reference and the port — so `2 ^ 3 ^ 2` had it too. Both + now fold left, which is what a C128 does with a run of same-precedence operators. + Regression tests in `tests/parser_expressions.c`. -13. **`basicparser.go:565` — a unary-minus argument inflates a function's arity count.** The - counter walks the `.right` chain, and a unary leaf keeps its operand on `.right`, so - `ABS(-9)` is counted as two arguments and refused with "function ABS takes 1 arguments, - received 2". *Consequence:* no builtin can be called with a negative literal. The corpus - hides it — `sgn.bas` assigns `-1` to a variable first. Fix: count only the top-level - argument chain, not whatever a leaf hangs off `.right`. Pinned in - `tests/runtime_evaluate.c`. + Worth recording for whoever reads the reference next: `exponent()` there is + `for self.match(CARAT) { ... return expr, nil }` (`basicparser.go:519-539`), so the loop is + a loop in shape only. `subtraction()` is the same at `:357-377`. Neither is a + transcription slip in the port. -14. **`basicscanner.go:272` — `matchNextChar` returns without setting a token type when it - cannot peek past the end of the line, so a comparison operator in the final column is - silently dropped.** `A# =` produces one token, not two. *Consequence:* a line ending in a - bare `<`, `>` or `=` loses it, and the parse error that follows points at the wrong place. - Fix: set `falsetype` before returning on the peek failure. Pinned in - `tests/scanner_tokens.c`. +13. ~~**`basicparser.go:565` — a unary-minus argument inflates a function's arity count.**~~ + **Fixed**, and the fix is structural rather than a smarter counter. The counter walked + `.right`, which is where a unary leaf keeps its operand *and* where an argument list chains + its arguments; the two meanings were indistinguishable, so `ABS(-9)` counted as two + arguments and was refused with "function ABS takes 1 arguments, received 2". No builtin + could be called with a negative literal at all. The corpus hid it — `sgn.bas` assigns `-1` + to a variable first. -15. **`basicscanner.go:318` — hex literals do not survive the scanner.** `matchNumber` allows - `x` through but not the hex digits after it, so `0xff` lexes as `0x` and `ff` is scanned - as a separate identifier. *Consequence:* the base-16 branch in `newLiteralInt` - (`basicgrammar.go:224`) is unreachable, and the README's hex support does not exist. Fix: - once `x` has been seen, accept `[0-9a-fA-F]`. Pinned in `tests/scanner_tokens.c`. + **A unary leaf's operand now hangs off `.left`.** A unary leaf had no other use for that + field, so the two meanings separate for the cost of one nobody was using — and counting + then needs no special case at all. That also fixes the half of the defect the original + entry did not mention: chaining the *next* argument through `.right` overwrote the operand, + so `MOD(-7, 3)` destroyed its own first argument. Three readers moved (`src/runtime.c`, + `src/grammar.c`'s renderer, `src/runtime_commands.c`'s `LIST` range parser) and that is the + whole change. Regression tests in `tests/runtime_evaluate.c`, both halves. + + **The same collision remains for array subscripts**, which is why §4 still lists + "array references in parameter lists" as outstanding: an identifier keeps its subscript + list on `.right` too. The same move — onto `.left` — is very likely the fix, and it is not + made here because it wants its own commit and its own tests. + +14. ~~**`basicscanner.go:272` — `matchNextChar` returns without setting a token type when it + cannot peek past the end of the line.**~~ **Fixed.** A comparison operator in the final + column was silently dropped: `A# =` produced one token, not two, and the parse error that + followed pointed somewhere else entirely. `falsetype` is now set before returning on the + peek failure. Regression tests in `tests/scanner_tokens.c`, for `=`, `<` and `>`. + +15. ~~**`basicscanner.go:318` — hex literals do not survive the scanner.**~~ **Fixed.** + `matchNumber` let `x` through but not the hex digits after it, so `0xff` lexed as `0x` + followed by an identifier `ff`, the base-16 branch in `newLiteralInt` was unreachable, and + the README's hex support did not exist. Once the `x` is seen the run continues over hex + digits. Accepted anywhere in a number run rather than only after a leading `0`, which is + the reference's rule and is worth keeping: it is what makes `1x2` one malformed token that + the line-number conversion diagnoses by name. Regression tests in `tests/scanner_tokens.c` + and `tests/language/numeric/octal_literal.bas`. 16. **`basicscanner.go:349` — the "Reserved word in variable name" check never fires.** The lexeme still carries its type suffix when the keyword tables are searched, so `PRINT$` misses every table and becomes an ordinary string variable. *Consequence:* the diagnostic - is dead code and `PRINT$ = 1` is accepted. Fix: strip the suffix before the lookup. Pinned - in `tests/scanner_tokens.c`. + is dead code and `PRINT$ = 1` is accepted. + + **Deliberately not fixed, and this is the interesting one.** The fix is four lines — strip + the suffix before the lookup — and it works. It also breaks a golden case: the reference's + own `deps/basicinterpret/tests/examples/strreverse.bas` names a variable `INPUT$`, and with + the check working that line is refused. + + A real C128 refuses it too, so the reference *program* is the wrong one here, not the + diagnostic. But the corpus is the acceptance contract (§1.8), it lives in a submodule this + repository may not edit, and reproducing it byte for byte is goal 1's headline claim. The + two cannot both hold. Fixing this means deciding that the diagnostic is worth one + permanently-excluded upstream case, which is a call for the author rather than for whoever + is next through this file. Until then it stays pinned in + `tests/known_reference_defects.c`, which is now the only thing left in there. ### Ours, not the reference's @@ -1052,8 +1087,14 @@ 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 12–17** — the defects the port uncovered. Item 12 is the one that produces a - *wrong answer* rather than a refused one, and should be fixed first. +5. **§6 items 16 and 17.** Items 10 and 12–15 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. 6. **Mutation survivors in `src/value.c`.** The harness is in place (`scripts/mutation_test.py`, the `mutation` CMake target, and a CI job on `src/symtab.c`), and a partial run over `src/value.c` turned up test gaps worth closing. These are *not* equivalent mutants; each diff --git a/include/akbasic/grammar.h b/include/akbasic/grammar.h index 0817896..6c02fc0 100644 --- a/include/akbasic/grammar.h +++ b/include/akbasic/grammar.h @@ -200,15 +200,21 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_leaf_new_binary(akbasic_ASTLeaf *obj, /** * @brief Build a unary-operator leaf. * - * The operand hangs off `.right`, which is why the arity counter in the parser - * miscounts a negative literal argument -- see TODO.md section 6 item 13. + * **The operand hangs off `.left`, not `.right`, and that is deliberate.** The + * reference puts it on `.right` (basicgrammar.go's newUnary), which is also + * where an argument list chains its arguments -- so `ABS(-9)` counted as two + * arguments and was refused, and a unary argument followed by a comma had its + * operand overwritten by the next argument. TODO.md section 6 item 13. A unary + * leaf has no other use for `.left`, so moving it there separates the two + * meanings for the cost of one field nobody was using. + * * @param obj Leaf to initialize; drawn from a pool by the caller. * @param op Operator token. - * @param right Operand. + * @param operand The expression the operator applies to. * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER When `right` is NULL. + * @throws AKERR_NULLPOINTER When `operand` is NULL. */ -akerr_ErrorContext AKERR_NOIGNORE *akbasic_leaf_new_unary(akbasic_ASTLeaf *obj, akbasic_TokenType op, akbasic_ASTLeaf *right); +akerr_ErrorContext AKERR_NOIGNORE *akbasic_leaf_new_unary(akbasic_ASTLeaf *obj, akbasic_TokenType op, akbasic_ASTLeaf *operand); /** * @brief Build a function-call leaf. * @param obj Leaf to initialize; drawn from a pool by the caller. diff --git a/src/grammar.c b/src/grammar.c index fc425f0..6433c8a 100644 --- a/src/grammar.c +++ b/src/grammar.c @@ -185,14 +185,19 @@ akerr_ErrorContext *akbasic_leaf_new_binary(akbasic_ASTLeaf *obj, akbasic_ASTLea SUCCEED_RETURN(errctx); } -akerr_ErrorContext *akbasic_leaf_new_unary(akbasic_ASTLeaf *obj, akbasic_TokenType op, akbasic_ASTLeaf *right) +akerr_ErrorContext *akbasic_leaf_new_unary(akbasic_ASTLeaf *obj, akbasic_TokenType op, akbasic_ASTLeaf *operand) { PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL leaf in new_unary"); - FAIL_ZERO_RETURN(errctx, (right != NULL), AKERR_NULLPOINTER, "nil pointer arguments"); + FAIL_ZERO_RETURN(errctx, (operand != NULL), AKERR_NULLPOINTER, "nil pointer arguments"); PASS(errctx, akbasic_leaf_init(obj, AKBASIC_LEAF_UNARY)); - obj->right = right; + /* + * .left, not .right. An argument list chains its arguments through .right, + * so an operand there is indistinguishable from a second argument: ABS(-9) + * counted as two and was refused. See the note on the declaration. + */ + obj->left = operand; obj->operator_ = op; SUCCEED_RETURN(errctx); } @@ -268,16 +273,14 @@ akerr_ErrorContext *akbasic_leaf_new_literal_int(akbasic_ASTLeaf *obj, const cha FAIL_ZERO_RETURN(errctx, (lexeme[0] != '\0'), AKBASIC_ERR_VALUE, "Empty integer literal"); /* - * The reference selects base 8 for any lexeme starting with '0', so `010` - * parses as 8 and `08` is an error. Commodore BASIC has no octal literals - * and this is TODO.md section 12 item 10 -- but it is reproduced here, - * because "port the behaviour first" is the rule and the fix is its own - * commit with its own test. + * Base 10 unless the lexeme is prefixed `0x`. The reference selects base 8 + * for *any* lexeme starting with '0' (basicgrammar.go:224), so `PRINT 010` + * printed 8 and `PRINT 08` was a parse error -- TODO.md section 6 item 10. + * Commodore BASIC has no octal literals, and a leading zero in a listing is + * padding, not a radix. */ if ( strlen(lexeme) > 2 && strncmp(lexeme, "0x", 2) == 0 ) { base = 16; - } else if ( lexeme[0] == '0' ) { - base = 8; } PASS(errctx, akbasic_leaf_init(obj, AKBASIC_LEAF_LITERAL_INT)); PASS(errctx, aksl_strtoll(lexeme, NULL, base, &value)); @@ -366,7 +369,7 @@ akerr_ErrorContext *akbasic_leaf_to_string(akbasic_ASTLeaf *self, char *dest, si snprintf(dest, len, "NOT IMPLEMENTED"); break; case AKBASIC_LEAF_UNARY: - PASS(errctx, akbasic_leaf_to_string(self->right, sub1, sizeof(sub1))); + PASS(errctx, akbasic_leaf_to_string(self->left, sub1, sizeof(sub1))); snprintf(dest, len, "(%s %s)", operator_to_str(self->operator_), sub1); break; case AKBASIC_LEAF_BINARY: diff --git a/src/parser.c b/src/parser.c index 75bf216..05de302 100644 --- a/src/parser.c +++ b/src/parser.c @@ -349,10 +349,15 @@ akerr_ErrorContext *akbasic_parser_relation(akbasic_Parser *obj, akbasic_ASTLeaf } /* - * subtraction and exponent return after one operator where addition, - * multiplication and division loop. That asymmetry is the reference's, not a - * transcription slip: `1 - 2 - 3` parses as `1 - (2 - 3)` there. Reproduced, - * because the golden corpus encodes the observed associativity. + * Loops, where the reference returns after one operator (basicparser.go:329). + * That asymmetry was reproduced faithfully at first and is TODO.md section 6 + * item 12: `1 - 2 - 3` computed `1 - 2` and abandoned the rest of the line, + * which the statement loop then picked up as a second statement and discarded. + * A wrong answer, silently -- the highest-impact item on that list. + * + * Left-associative, matching addition, multiplication and division here and + * matching a C128, which evaluates a run of same-precedence operators left to + * right. */ static akerr_ErrorContext *subtraction(akbasic_Parser *obj, akbasic_ASTLeaf **dest) { @@ -363,15 +368,16 @@ static akerr_ErrorContext *subtraction(akbasic_Parser *obj, akbasic_ASTLeaf **de akbasic_Token *operator_ = NULL; PASS(errctx, addition(obj, &left)); - if ( akbasic_parser_match1(obj, AKBASIC_TOK_MINUS) ) { + while ( akbasic_parser_match1(obj, AKBASIC_TOK_MINUS) ) { PASS(errctx, akbasic_parser_previous(obj, &operator_)); PASS(errctx, addition(obj, &right)); + if ( expr != NULL ) { + left = expr; + } PASS(errctx, akbasic_parser_new_leaf(obj, &expr)); PASS(errctx, akbasic_leaf_new_binary(expr, left, operator_->tokentype, right)); - *dest = expr; - SUCCEED_RETURN(errctx); } - *dest = left; + *dest = (expr != NULL ? expr : left); SUCCEED_RETURN(errctx); } @@ -469,15 +475,21 @@ static akerr_ErrorContext *exponent(akbasic_Parser *obj, akbasic_ASTLeaf **dest) akbasic_Token *operator_ = NULL; PASS(errctx, function_call(obj, &left)); - if ( akbasic_parser_match1(obj, AKBASIC_TOK_CARAT) ) { + /* + * Loops for the same reason subtraction does -- `2 ^ 3 ^ 2` abandoned the + * second operator before this. Left-associative, which is a C128: 2^3^2 is + * 64 there, not the 512 a right-associative reading gives. + */ + while ( akbasic_parser_match1(obj, AKBASIC_TOK_CARAT) ) { PASS(errctx, akbasic_parser_previous(obj, &operator_)); PASS(errctx, function_call(obj, &right)); + if ( expr != NULL ) { + left = expr; + } PASS(errctx, akbasic_parser_new_leaf(obj, &expr)); PASS(errctx, akbasic_leaf_new_binary(expr, left, operator_->tokentype, right)); - *dest = expr; - SUCCEED_RETURN(errctx); } - *dest = left; + *dest = (expr != NULL ? expr : left); SUCCEED_RETURN(errctx); } diff --git a/src/runtime.c b/src/runtime.c index 699b16c..7ad77d1 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -453,7 +453,8 @@ akerr_ErrorContext *akbasic_runtime_evaluate(akbasic_Runtime *obj, akbasic_ASTLe SUCCEED_RETURN(errctx); case AKBASIC_LEAF_UNARY: - PASS(errctx, akbasic_runtime_evaluate(obj, expr->right, &rval)); + /* .left: a unary leaf's operand, kept clear of the argument chain. */ + PASS(errctx, akbasic_runtime_evaluate(obj, expr->left, &rval)); PASS(errctx, akbasic_environment_new_value(obj->environment, &scratch)); if ( expr->operator_ == AKBASIC_TOK_MINUS ) { PASS(errctx, akbasic_value_invert(rval, scratch, dest)); diff --git a/src/runtime_commands.c b/src/runtime_commands.c index d561bfb..739efeb 100644 --- a/src/runtime_commands.c +++ b/src/runtime_commands.c @@ -377,8 +377,8 @@ static akerr_ErrorContext *parse_line_range(akbasic_Runtime *obj, akbasic_ASTLea } if ( expr->right->leaftype == AKBASIC_LEAF_UNARY && expr->right->operator_ == AKBASIC_TOK_MINUS ) { - /* -n: from the start through n. */ - PASS(errctx, akbasic_runtime_evaluate(obj, expr->right->right, &value)); + /* -n: from the start through n. A unary leaf's operand is on .left. */ + PASS(errctx, akbasic_runtime_evaluate(obj, expr->right->left, &value)); FAIL_NONZERO_RETURN(errctx, (value->valuetype != AKBASIC_TYPE_INTEGER), AKBASIC_ERR_TYPE, "Expected a line number range"); *endidx = value->intval; diff --git a/src/scanner.c b/src/scanner.c index 885d948..97d6453 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -103,6 +103,15 @@ static bool match_next_char(akbasic_Runtime *obj, char cm, akbasic_TokenType tru char nc = '\0'; if ( !peek(obj, &nc) ) { + /* + * Nothing left to peek at, so the operator is whatever it is on its + * own. The reference returns here *without* setting a type + * (basicscanner.go:272), which silently drops a comparison operator in + * the final column of a line: `A# =` produced one token, not two, and + * the parse error that followed pointed somewhere else entirely. + * TODO.md section 6 item 14. + */ + obj->tokentype = falsetype; return false; } if ( nc == cm ) { @@ -144,6 +153,7 @@ static akerr_ErrorContext *match_number(akbasic_Runtime *obj) char nc = '\0'; int64_t lineno = 0; long long converted = 0; + bool hex = false; obj->tokentype = AKBASIC_TOK_LITERAL_INT; while ( !is_at_end(obj) ) { @@ -156,8 +166,27 @@ static akerr_ErrorContext *match_number(akbasic_Runtime *obj) SUCCEED_RETURN(errctx); } obj->tokentype = AKBASIC_TOK_LITERAL_FLOAT; - } else if ( !isdigit((unsigned char)c) && c != 'x' ) { - /* 'x' is allowed through so 0x-prefixed hex reaches the parser. */ + } else if ( c == 'x' && !hex ) { + /* + * An 'x' in a number run. The reference lets it through and then + * stops at the first hex digit after it (basicscanner.go:318), so + * `0xff` lexed as `0x` followed by an identifier `ff`, the parser's + * base-16 branch was unreachable, and the README's hex support did + * not exist. TODO.md section 6 item 15. Once the 'x' is seen, keep + * going over hex digits. + * + * Accepted anywhere in the run rather than only after a leading `0`, + * which is the reference's rule and is worth keeping: it is what + * makes `1x2` one malformed token that the line-number conversion + * then diagnoses by name, instead of two tokens that fail somewhere + * less helpful. + */ + hex = true; + } else if ( hex ) { + if ( !isxdigit((unsigned char)c) ) { + break; + } + } else if ( !isdigit((unsigned char)c) ) { break; } obj->current += 1; @@ -231,6 +260,12 @@ static akerr_ErrorContext *match_identifier(akbasic_Runtime *obj) } PASS(errctx, get_lexeme(obj, lexeme, sizeof(lexeme))); + /* + * Searched with the type suffix still attached, which is the reference's + * rule (basicscanner.go:349) and is why the "Reserved word in variable name" + * branch below is unreachable -- TODO.md section 6 item 16, which explains + * why it is still here rather than fixed. + */ PASS(errctx, akbasic_verb_lookup(lexeme, &verb)); ATTEMPT { diff --git a/tests/grammar_leaves.c b/tests/grammar_leaves.c index 125cd46..49081c8 100644 --- a/tests/grammar_leaves.c +++ b/tests/grammar_leaves.c @@ -36,14 +36,20 @@ int main(void) TEST_REQUIRE_INT(LEAVES[1].literal_int, 255); /* - * A leading zero selects base 8, so 010 is 8 and 08 will not parse. - * Commodore BASIC has no octal literals; this is TODO.md section 12 item 10, - * reproduced deliberately and pinned here so the eventual fix is a - * deliberate change to this assertion. + * A leading zero is padding, not a radix -- TODO.md section 6 item 10's + * regression test. The reference selects base 8 for any lexeme starting with + * '0' (basicgrammar.go:224), so 010 was 8 and 08 would not parse at all. + * Commodore BASIC has no octal literals. */ TEST_REQUIRE_OK(akbasic_leaf_new_literal_int(&LEAVES[1], "010")); + TEST_REQUIRE_INT(LEAVES[1].literal_int, 10); + TEST_REQUIRE_OK(akbasic_leaf_new_literal_int(&LEAVES[1], "08")); TEST_REQUIRE_INT(LEAVES[1].literal_int, 8); - TEST_REQUIRE_ANY_ERROR(akbasic_leaf_new_literal_int(&LEAVES[1], "08")); + TEST_REQUIRE_OK(akbasic_leaf_new_literal_int(&LEAVES[1], "0")); + TEST_REQUIRE_INT(LEAVES[1].literal_int, 0); + /* 0x still selects base 16, which is the one prefix that means anything. */ + TEST_REQUIRE_OK(akbasic_leaf_new_literal_int(&LEAVES[1], "0x10")); + TEST_REQUIRE_INT(LEAVES[1].literal_int, 16); TEST_REQUIRE_OK(akbasic_leaf_new_literal_float(&LEAVES[2], "2.5")); TEST_REQUIRE_FEQ(LEAVES[2].literal_float, 2.5); diff --git a/tests/known_reference_defects.c b/tests/known_reference_defects.c index 7ce352f..46d99a2 100644 --- a/tests/known_reference_defects.c +++ b/tests/known_reference_defects.c @@ -10,72 +10,37 @@ * When one of these is fixed, this test starts passing and CTest reports * "unexpectedly passed". That is the cue to split the fixed assertion out into * the corresponding suite in AKBASIC_TESTS and strike the item from TODO.md - * section 12. + * section 6. + * + * Five of the six this file was written with are gone that way. What is left is + * the one whose fix is not a coding question but a decision, and it is described + * where it is asserted. */ #include "harness.h" int main(void) { - akbasic_ASTLeaf *leaf = NULL; - akbasic_Value *out = NULL; - char rendered[AKBASIC_MAX_STRING_LENGTH]; - TEST_REQUIRE_OK(harness_start(NULL)); /* - * 12.12 -- subtraction stops after one operator, so `1 - 2 - 3` parses as - * `1 - 2` and abandons the rest of the line. It should fold left, the way - * addition does. - */ - TEST_REQUIRE_OK(harness_parse("A# = 1 - 2 - 3", &leaf)); - TEST_REQUIRE_OK(akbasic_leaf_to_string(leaf, rendered, sizeof(rendered))); - TEST_REQUIRE_STR(rendered, "( A# (- (- 1 2) 3))"); - - /* - * 12.13 -- a unary-minus argument inflates the arity count, because the - * counter walks .right and a unary leaf keeps its operand there. ABS(-9) - * should be one argument. - */ - TEST_REQUIRE_OK(akbasic_environment_zero(HARNESS_RUNTIME.environment)); - TEST_REQUIRE_OK(harness_parse("A# = ABS(-9)", &leaf)); - TEST_REQUIRE_OK(akbasic_runtime_evaluate(&HARNESS_RUNTIME, leaf, &out)); - TEST_REQUIRE_INT(out->intval, 9); - - /* - * 12.14 -- a comparison operator in the final column of a line is dropped, - * because matchNextChar returns without setting a token type when it cannot - * peek past the end. - */ - TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "A# =", NULL, 0)); - TEST_REQUIRE_INT(HARNESS_RUNTIME.environment->nexttoken, 2); - - /* - * 12.15 -- hex literals do not survive the scanner. matchNumber lets 'x' - * through but not the digits after it, so the parser's base-16 branch is - * unreachable. - */ - TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT 0xff", NULL, 0)); - TEST_REQUIRE_STR(HARNESS_RUNTIME.environment->tokens[1].lexeme, "0xff"); - - /* - * 12.16 -- "Reserved word in variable name" never fires, because the lexeme - * still carries its type suffix when the verb table is searched. PRINT$ - * should be refused as a variable name. + * 6.16 -- "Reserved word in variable name" never fires, because the lexeme + * still carries its type suffix when the verb table is searched, so PRINT$ + * misses every table and becomes an ordinary string variable. + * + * Still here rather than fixed, and the reason is worth knowing before + * anybody "just fixes it": stripping the suffix before the lookup is four + * lines and it works, and it costs a golden case. The reference's own + * examples/strreverse.bas names a variable INPUT$ -- which a real C128 would + * refuse too, so the reference program is the wrong one. But the corpus is + * the acceptance contract and it lives in a submodule this repository may + * not edit, so the diagnostic and the corpus cannot both be right. TODO.md + * section 6 records the trade rather than making it silently. */ HARNESS_RUNTIME.hasError = false; TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT$ = 1", NULL, 0)); TEST_REQUIRE(HARNESS_RUNTIME.hasError, "PRINT$ should be refused as a variable name"); - /* - * 12.10 -- a leading zero selects base 8, so `PRINT 010` prints 8 and - * `PRINT 08` will not parse. Commodore BASIC has no octal literals. - */ - TEST_REQUIRE_OK(akbasic_environment_zero(HARNESS_RUNTIME.environment)); - TEST_REQUIRE_OK(harness_parse("A# = 010", &leaf)); - TEST_REQUIRE_OK(akbasic_runtime_evaluate(&HARNESS_RUNTIME, leaf, &out)); - TEST_REQUIRE_INT(out->intval, 10); - harness_stop(); return akbasic_test_failures; } diff --git a/tests/language/numeric/octal_literal.bas b/tests/language/numeric/octal_literal.bas index 3f5687e..473c4c2 100644 --- a/tests/language/numeric/octal_literal.bas +++ b/tests/language/numeric/octal_literal.bas @@ -1,6 +1,9 @@ -10 REM The reference selects base 8 for any lexeme starting with 0, so 010 is 8 -20 REM and 08 is a parse error. Commodore BASIC has no octal literals -- this is -30 REM TODO.md section 6 item 10, reproduced deliberately until it is fixed on -40 REM purpose. Pinned here so the eventual fix has to change a golden file. -50 PRINT 010 -60 PRINT 08 +10 REM A leading zero is padding, not a radix. The reference selects base 8 for +20 REM any lexeme starting with 0, so 010 printed 8 and 08 was a parse error -- +30 REM TODO.md section 6 item 10, fixed. Commodore BASIC has no octal literals. +40 REM 0x is the one prefix that changes the base, and it now reaches the scanner +50 REM whole: that was section 6 item 15. +60 PRINT 010 +70 PRINT 08 +80 PRINT 0xff +90 PRINT 0x10 + 1 diff --git a/tests/language/numeric/octal_literal.txt b/tests/language/numeric/octal_literal.txt index 6402eb4..aa4cfc6 100644 --- a/tests/language/numeric/octal_literal.txt +++ b/tests/language/numeric/octal_literal.txt @@ -1,3 +1,4 @@ +10 8 -? 60 : PARSE ERROR trailing junk "8" after the number in "08" - +255 +17 diff --git a/tests/parser_expressions.c b/tests/parser_expressions.c index dc1764c..a6969b7 100644 --- a/tests/parser_expressions.c +++ b/tests/parser_expressions.c @@ -73,19 +73,18 @@ int main(void) /* AND and OR sit below comparison. */ expect_tree("A# = 1 == 1 AND 2 == 2", "( A# (AND (= 1 1) (= 2 2)))"); - /* Exponent. */ - expect_tree("A# = 2 ^ 3", "( A# (^ 2 3))"); - /* - * Addition loops over its operator; subtraction returns after one. So - * `1 + 2 + 3` folds left, while `1 - 2 - 3` parses only `1 - 2` and leaves - * `- 3` in the token stream for the statement loop to pick up as a second - * statement. That silently computes the wrong answer -- TODO.md section 12 - * item 12 -- and is pinned here so the fix is a deliberate change to this - * assertion and not an accident. + * Every same-precedence run folds left, which is what a C128 does and what + * TODO.md section 6 item 12 was about: the reference returns after one + * operator in subtraction() and exponent(), so `1 - 2 - 3` computed `1 - 2` + * and left `- 3` in the token stream for the statement loop to evaluate and + * discard. A wrong answer, silently. These four are the regression test. */ expect_tree("A# = 1 + 2 + 3", "( A# (+ (+ 1 2) 3))"); - expect_tree("A# = 1 - 2 - 3", "( A# (- 1 2))"); + expect_tree("A# = 1 - 2 - 3", "( A# (- (- 1 2) 3))"); + expect_tree("A# = 10 - 1 - 2 - 3", "( A# (- (- (- 10 1) 2) 3))"); + expect_tree("A# = 2 ^ 3", "( A# (^ 2 3))"); + expect_tree("A# = 2 ^ 3 ^ 2", "( A# (^ (^ 2 3) 2))"); /* An array reference parses as an identifier carrying a subscript list. */ TEST_REQUIRE_OK(harness_parse("A#(2) = 1", &leaf)); diff --git a/tests/runtime_evaluate.c b/tests/runtime_evaluate.c index ce91e78..698f39e 100644 --- a/tests/runtime_evaluate.c +++ b/tests/runtime_evaluate.c @@ -109,18 +109,29 @@ int main(void) test_discard_error(e); } - /* - * Built-in functions dispatch through the table. - * - * Note the arguments are never negative *literals*: a unary-minus leaf uses - * its .right for the operand, and the parser counts arity by walking .right, - * so `ABS(-9)` reports two arguments and is rejected. TODO.md section 12 - * item 13. The golden corpus sidesteps it the same way -- sgn.bas assigns - * -1 to a variable first. - */ + /* Built-in functions dispatch through the table. */ expect_int("N# = -9", -9); expect_int("A# = ABS(N#)", 9); expect_int("A# = SGN(N#)", -1); + + /* + * A negative *literal* argument, which is TODO.md section 6 item 13's + * regression test. The reference hangs a unary leaf's operand off .right, + * which is also where an argument list chains its arguments, so `ABS(-9)` + * counted as two arguments and was refused outright -- no builtin could be + * called with a negative literal at all. The corpus hid it: sgn.bas assigns + * -1 to a variable first, which is what the three lines above do. + */ + expect_int("A# = ABS(-9)", 9); + expect_int("A# = SGN(-5)", -1); + /* + * And with a second argument after the unary one, which is the other half of + * the same defect: chaining the next argument through .right overwrote the + * operand the unary leaf was keeping there. + */ + expect_int("A# = SHL(-1, 0)", -1); + expect_int("A# = MOD(-7, 3)", -1); + expect_int("A# = INSTR(\"HELLO\", \"L\")", 2); expect_int("A# = LEN(\"HELLO\")", 5); expect_int("A# = SHL(1, 4)", 16); expect_int("A# = SHR(16, 4)", 1); diff --git a/tests/scanner_tokens.c b/tests/scanner_tokens.c index 9d090fc..09a2a0e 100644 --- a/tests/scanner_tokens.c +++ b/tests/scanner_tokens.c @@ -43,13 +43,7 @@ int main(void) expect_token(6, AKBASIC_TOK_RIGHT_PAREN, ")"); expect_token(7, AKBASIC_TOK_COMMA, ","); - /* - * Two-character comparisons, and the one-character fallbacks. The trailing - * space is not decoration: matchNextChar cannot peek past the end of the - * line, and when it fails it returns without setting a token type, so a - * comparison operator in the final column is silently dropped. TODO.md - * section 12 item 14. - */ + /* Two-character comparisons, and the one-character fallbacks. */ TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "<= <> >= < > == = ", NULL, 0)); TEST_REQUIRE_INT(env()->nexttoken, 7); expect_token(0, AKBASIC_TOK_LESS_THAN_EQUAL, NULL); @@ -60,9 +54,25 @@ int main(void) expect_token(5, AKBASIC_TOK_EQUAL, NULL); expect_token(6, AKBASIC_TOK_ASSIGNMENT, NULL); - /* Without the trailing space the final operator vanishes. */ + /* + * An operator in the final column survives, with no trailing space to help + * it. matchNextChar cannot peek past the end of the line, and the reference + * returns from that without setting a token type (basicscanner.go:272), so + * `A# =` produced one token and the parse error that followed pointed + * somewhere else. TODO.md section 6 item 14; this is its regression test. + */ TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "A# =", NULL, 0)); - TEST_REQUIRE_INT(env()->nexttoken, 1); + TEST_REQUIRE_INT(env()->nexttoken, 2); + expect_token(0, AKBASIC_TOK_IDENTIFIER_INT, "A#"); + expect_token(1, AKBASIC_TOK_ASSIGNMENT, "="); + + /* The same for the other two, each alone in the final column. */ + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "A# <", NULL, 0)); + TEST_REQUIRE_INT(env()->nexttoken, 2); + expect_token(1, AKBASIC_TOK_LESS_THAN, "<"); + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "A# >", NULL, 0)); + TEST_REQUIRE_INT(env()->nexttoken, 2); + expect_token(1, AKBASIC_TOK_GREATER_THAN, ">"); /* Literals. */ TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT 1.5", NULL, 0)); @@ -70,13 +80,25 @@ int main(void) expect_token(1, AKBASIC_TOK_LITERAL_FLOAT, "1.5"); /* - * A hex literal does not survive the scanner: matchNumber allows 'x' through - * but not the hex digits after it, so "0xff" lexes as "0x" and the "ff" is - * scanned as an identifier. The parser's base-16 branch is therefore - * unreachable. TODO.md section 12 item 15. + * A hex literal survives the scanner whole. The reference lets 'x' through + * but stops at the hex digits after it (basicscanner.go:318), so "0xff" + * lexed as "0x" plus an identifier "ff", the parser's base-16 branch was + * unreachable and the README's hex support did not exist. TODO.md section 6 + * item 15; this is its regression test. */ TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT 0xff", NULL, 0)); - expect_token(1, AKBASIC_TOK_LITERAL_INT, "0x"); + TEST_REQUIRE_INT(env()->nexttoken, 2); + expect_token(1, AKBASIC_TOK_LITERAL_INT, "0xff"); + + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT 0xDEAD", NULL, 0)); + expect_token(1, AKBASIC_TOK_LITERAL_INT, "0xDEAD"); + + /* And the hex run ends where it should, rather than eating what follows. */ + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT 0x10 + 1", NULL, 0)); + TEST_REQUIRE_INT(env()->nexttoken, 4); + expect_token(1, AKBASIC_TOK_LITERAL_INT, "0x10"); + expect_token(2, AKBASIC_TOK_PLUS, "+"); + expect_token(3, AKBASIC_TOK_LITERAL_INT, "1"); TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT \"HELLO\"", NULL, 0)); expect_token(1, AKBASIC_TOK_LITERAL_STRING, "HELLO");