diff --git a/TODO.md b/TODO.md index 306f8e8..ba26ce2 100644 --- a/TODO.md +++ b/TODO.md @@ -565,11 +565,30 @@ control), `MONITOR` (no machine-language monitor). Also on the queue, from the reference's own defect list: -- **Multiple statements per line** (`10 PRINT A$ : REM ...`). The `COLON` token exists - (`basicscanner.go:40`) and nothing consumes it. This changes the parser's statement loop and - should be done before group A, because `DO`/`LOOP` bodies read badly without it. -- **Array references in parameter lists** (`READ A$(0), B#`) currently fail to parse. Fix in - `argumentList`. +- ~~**Multiple statements per line**~~ **Done.** `akbasic_parser_parse()` consumes a run of + `COLON` tokens before each statement and hands the caller a NULL leaf when nothing followed + them, which is how an empty statement — a trailing separator, or `::` — stays legal rather + than becoming an error. `tests/parser_commands.c` counts statements per line; + `tests/language/statements/multiple_per_line.bas` asserts what actually runs. + + **One thing about it needed a decision rather than a transcription**, and it is recorded as + deviation 29 in §5: BASIC 7.0 scopes everything after `THEN` to the condition, so + `IF C THEN A : B` must run neither `A` nor `B` when `C` is false. The parser takes exactly one + statement per arm, so the rest of the line reaches the statement loop as ordinary statements + and the branch has to tell the loop whether to run them. + + **A known limitation, not a defect:** a whole `FOR`/`NEXT` on one line + (`FOR I# = 1 TO 3 : PRINT I# : NEXT I#`) does not loop. The block structure is the + reference's `waitingForCommand` model (§1.6), which skips forward *by source line* to the verb + it is waiting for, so a `NEXT` on the same line as its `FOR` is never reached. Fixing it means + restructuring control flow to work on statements rather than lines, which is a deliberate + piece of work and wants its own commit. Group A's `DO`/`LOOP` will meet the same wall. +- **Array references in parameter lists** (`READ A$(0), B#`) currently fail to parse. This is + the *same collision* §6 item 13 was, one field along: an identifier keeps its subscript list + on `.right`, which is also where an argument list chains its arguments. That item was fixed by + moving a unary leaf's operand to `.left`; the same move very likely works here, since an + identifier leaf has no other use for the field either. Fix in `akbasic_parser_primary` and + `akbasic_leaf_first_subscript`. --- @@ -727,6 +746,29 @@ deviations from the reference's *program*: `main.go` and the SDL half of driven through the SDL binary, which is where most of the frontend's real coverage comes from. +### Deviations in statement separation + +29. **A branch decides who owns the rest of its line.** BASIC 7.0 scopes every statement after + `THEN` to the condition, and the reference has no opinion on the matter because it never + consumed the `COLON` token at all. The parser here takes exactly one statement for each arm, + so the rest of the line arrives at the statement loop as ordinary top-level statements and + something has to say whether to run them. The `BRANCH` case in + `akbasic_runtime_evaluate()` sets `runtime->skiprestofline`, and the two statement loops + stop on it. + + **The rule is not "skip when false"**, which is the reason this is written down: + + | Line | Condition | What runs | + |---|---|---| + | `IF C THEN A : B` | true | `A`, `B` | + | `IF C THEN A : B` | false | nothing | + | `IF C THEN A ELSE B : D` | true | `A` | + | `IF C THEN A ELSE B : D` | false | `B`, `D` | + + The remainder always belongs to whichever arm was written *last*, so it is skipped exactly + when that arm is the one not taken — with an `ELSE` the last arm is `ELSE`, without one it + is `THEN`. That is one line of code and it reads as an oddity without the table above it. + --- ## 6. Reference defects — ported faithfully, fix them deliberately @@ -977,10 +1019,10 @@ entire test corpus and passes clean under ASan and UBSan. | Gate | Result | |---|---| -| `ctest` | 72/72 — 41 upstream golden cases, 5 local ones, 22 unit tests, 2 embedding examples, 1 known-failing, 1 version check | -| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 72/72 — the same, minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends` and `akgl_frontend`; the `akgl_build` CI job | +| `ctest` | 74/74 — 41 upstream golden cases, 8 local ones, 22 unit tests, 2 embedding examples, 1 known-failing | +| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 73/73 — the same, minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends` and `akgl_frontend`; the `akgl_build` CI job | | Golden corpus | 41/41 byte-exact, driven in place from the submodule — **and 41/41 again through the SDL binary**, which is most of what proves the frontend changes no output | -| ASan + UBSan | 72/72 | +| ASan + UBSan | 74/74 | | Line coverage | 93.6% (3618/3867) — above the 90% gate | | Function coverage | 97.8% (267/273) | | Warnings | none under `-Wall -Wextra` | @@ -1066,10 +1108,18 @@ What remains, in priority order: 2. ~~**A line editor for the akgl sink.**~~ **Done** — `readline` in `src/sink_akgl.c`, over the keystroke ring, borrowing frames from the host through an `akbasic_AkglPump`. It cannot type a shifted character, which is `libakgl` item 10 rather than work outstanding here. -3. **§4 — the language completion work queue.** Groups G and I and the `GET`/`GETKEY`/`SCNCLR` - part of E are done. Of what is left, **multiple statements per line** should come first: - the `COLON` token exists and nothing consumes it, and `DO`/`LOOP` in group A reads badly - without it. Then groups A, B, D, F and J, none of which need anything from `libakgl`. +3. **§4 — the language completion work queue.** Groups G and I, the `GET`/`GETKEY`/`SCNCLR` + part of E, and **multiple statements per line** are done. What is left is groups A, B, D, F + and J, none of which need anything from `libakgl`. + + **Do group A next, and expect it to cost more than the table suggests.** Multiple statements + per line turned up the wall it will hit: the `waitingForCommand` model skips forward *by + source line*, so a whole loop written on one line + (`FOR I# = 1 TO 3 : PRINT I# : NEXT I#`) never reaches its `NEXT`. §4 records it as a known + limitation rather than a defect, because it is inherited structure and not a slip — but + `DO`/`LOOP`/`WHILE`/`UNTIL` are exactly the verbs people write on one line, so group A + probably has to move block skipping from lines to statements before it starts. That is a + deliberate restructure and wants its own commit and its own test, ahead of any verb. 4. ~~**CI does not cover `-DAKBASIC_WITH_AKGL=ON`.**~~ **Done** — the `akgl_build` job in `.gitea/workflows/ci.yaml`. It turned out much cheaper than the deferral assumed, and the two reasons are worth keeping because both were guesses that measurement corrected: diff --git a/include/akbasic/parser.h b/include/akbasic/parser.h index 8aa80b8..907ac85 100644 --- a/include/akbasic/parser.h +++ b/include/akbasic/parser.h @@ -52,16 +52,33 @@ akerr_ErrorContext AKERR_NOIGNORE *akbasic_parser_zero(akbasic_Parser *obj); /** * @brief Parse one statement from the token stream. * - * A line can hold more than one statement, so callers loop on - * akbasic_parser_is_at_end(). + * A line can hold more than one statement, separated by colons, so callers loop + * on akbasic_parser_is_at_end(). Leading separators are consumed here. + * + * **`*dest` may be NULL on success.** That is a line that ended in a separator, + * or one that was nothing but separators -- an empty statement, which a C128 + * accepts and so does this. A caller that loops must skip a NULL leaf rather + * than handing it to the interpreter. * * @param obj Object to initialize, inspect, or modify. - * @param dest Output destination populated by the function. + * @param dest Output destination populated by the function; NULL when the + * remaining tokens held no statement. * @return `NULL` on success, otherwise an error context owned by the caller. * @throws AKBASIC_ERR_SYNTAX When the tokens do not parse. */ akerr_ErrorContext AKERR_NOIGNORE *akbasic_parser_parse(akbasic_Parser *obj, akbasic_ASTLeaf **dest); +/** + * @brief Consume a run of statement separators. + * + * Exposed because a verb with its own parse path may need to know where its + * statement ends. Returns false when nothing but separators remained. + * + * @param obj Parser to advance; a NULL or unbound parser reports false. + * @return `true` when a statement follows the separators. + */ +bool akbasic_parser_skip_separators(akbasic_Parser *obj); + /** * @brief True when the token stream for this line is spent. * @param obj Parser to inspect; a NULL or unbound parser is at the end. diff --git a/include/akbasic/runtime.h b/include/akbasic/runtime.h index bbb2652..a51b24f 100644 --- a/include/akbasic/runtime.h +++ b/include/akbasic/runtime.h @@ -126,6 +126,19 @@ typedef struct akbasic_Runtime */ int64_t timems; + /* + * Set by a branch that has decided the remaining statements on its line + * belong to the arm it did not take, and cleared at the top of every line. + * + * This exists because a line can hold several statements and BASIC 7.0 + * scopes everything after THEN to the condition -- `IF C THEN A : B` runs + * neither A nor B when C is false. The statement loop is what knows where a + * line ends, so the branch raises a flag and the loop acts on it. See the + * BRANCH case in akbasic_runtime_evaluate() for the exact rule, which is not + * quite "skip when false". + */ + bool skiprestofline; + /* REPL line assembly */ char userline[AKBASIC_MAX_LINE_LENGTH]; diff --git a/src/parser.c b/src/parser.c index 05de302..2ae5312 100644 --- a/src/parser.c +++ b/src/parser.c @@ -150,12 +150,36 @@ static akerr_ErrorContext *unary(akbasic_Parser *obj, akbasic_ASTLeaf **dest); static akerr_ErrorContext *exponent(akbasic_Parser *obj, akbasic_ASTLeaf **dest); static akerr_ErrorContext *function_call(akbasic_Parser *obj, akbasic_ASTLeaf **dest); +bool akbasic_parser_skip_separators(akbasic_Parser *obj) +{ + while ( akbasic_parser_match1(obj, AKBASIC_TOK_COLON) ) { + /* + * A run of them, so `10 PRINT 1 :: PRINT 2` and a trailing colon both + * work. An empty statement is not an error on a C128 and is not one + * here. + */ + } + return !akbasic_parser_is_at_end(obj); +} + akerr_ErrorContext *akbasic_parser_parse(akbasic_Parser *obj, akbasic_ASTLeaf **dest) { PREPARE_ERROR(errctx); FAIL_ZERO_RETURN(errctx, (obj != NULL && dest != NULL), AKERR_NULLPOINTER, "NULL argument in parse"); + *dest = NULL; + /* + * Statements are separated by colons. The scanner has emitted the token + * since the port began and nothing consumed it, so a line with two + * statements on it was a parse error -- TODO.md section 4. + * + * Nothing after the separators is a line that ended in one. That is not an + * error, so the caller gets a NULL leaf and skips it. + */ + if ( !akbasic_parser_skip_separators(obj) ) { + SUCCEED_RETURN(errctx); + } PASS(errctx, akbasic_parser_command(obj, dest)); SUCCEED_RETURN(errctx); } @@ -196,7 +220,13 @@ akerr_ErrorContext *akbasic_parser_command(akbasic_Parser *obj, akbasic_ASTLeaf * when there is one and it will not parse. */ righttoken = akbasic_parser_peek(obj); - if ( righttoken != NULL && righttoken->tokentype != AKBASIC_TOK_UNDEFINED ) { + if ( righttoken != NULL && righttoken->tokentype != AKBASIC_TOK_UNDEFINED && + righttoken->tokentype != AKBASIC_TOK_COLON ) { + /* + * A colon ends the statement, so `PRINT : PRINT "X"` is a bare PRINT + * followed by another statement rather than a PRINT of whatever a colon + * evaluates to. + */ PASS(errctx, akbasic_parser_expression(obj, &right)); } diff --git a/src/runtime.c b/src/runtime.c index 7ad77d1..4c56dba 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -414,6 +414,24 @@ akerr_ErrorContext *akbasic_runtime_evaluate(akbasic_Runtime *obj, akbasic_ASTLe } HANDLE_DEFAULT(errctx) { PASS(errctx, report_and_reraise(obj, errctx)); } FINISH(errctx, true); + /* + * Who owns the rest of the line? + * + * BASIC 7.0 scopes every statement after THEN to the condition, and the + * parser only ever takes *one* statement for each arm -- the rest arrive + * at the statement loop as ordinary top-level statements. So the branch + * has to say whether that loop should run them. + * + * IF C THEN A : B C false -> B belongs to THEN, skip it + * IF C THEN A ELSE B : D C true -> D belongs to ELSE, skip it + * + * which is not "skip when false": the remainder always belongs to + * whichever arm was written last, so it is skipped exactly when that arm + * is the one *not* taken. With an ELSE present the last arm is ELSE; + * without one it is THEN. + */ + obj->skiprestofline = ((expr->right != NULL) == (rval->boolvalue == AKBASIC_TRUE)); + if ( rval->boolvalue == AKBASIC_TRUE ) { PASS(errctx, akbasic_runtime_evaluate(obj, expr->left, dest)); SUCCEED_RETURN(errctx); @@ -683,8 +701,9 @@ akerr_ErrorContext *akbasic_runtime_process_line_repl(akbasic_Runtime *obj) obj->environment->lineno += obj->autoLineNumber; PASS(errctx, akbasic_scanner_scan(obj, obj->userline, scanned, sizeof(scanned))); PASS(errctx, akbasic_parser_init(&parser, obj)); + obj->skiprestofline = false; - while ( !akbasic_parser_is_at_end(&parser) ) { + while ( !akbasic_parser_is_at_end(&parser) && !obj->skiprestofline ) { ATTEMPT { CATCH(errctx, akbasic_parser_parse(&parser, &leaf)); } CLEANUP { @@ -698,6 +717,11 @@ akerr_ErrorContext *akbasic_runtime_process_line_repl(akbasic_Runtime *obj) SUCCEED_RETURN(errctx); } + if ( leaf == NULL ) { + /* Nothing but statement separators left; an empty statement is not one. */ + continue; + } + PASS(errctx, akbasic_runtime_interpret_immediate(obj, leaf, &value)); if ( value == NULL ) { /* Not an immediate command, so it is program text: file it. */ @@ -732,8 +756,9 @@ akerr_ErrorContext *akbasic_runtime_process_line_run(akbasic_Runtime *obj) PASS(errctx, akbasic_scanner_scan(obj, line, NULL, 0)); PASS(errctx, akbasic_parser_init(&parser, obj)); + obj->skiprestofline = false; - while ( !akbasic_parser_is_at_end(&parser) ) { + while ( !akbasic_parser_is_at_end(&parser) && !obj->skiprestofline ) { ATTEMPT { CATCH(errctx, akbasic_parser_parse(&parser, &leaf)); } CLEANUP { @@ -747,6 +772,10 @@ akerr_ErrorContext *akbasic_runtime_process_line_run(akbasic_Runtime *obj) if ( obj->errclass != AKBASIC_ERRCLASS_NONE ) { SUCCEED_RETURN(errctx); } + if ( leaf == NULL ) { + /* Nothing but statement separators left; an empty statement is not one. */ + continue; + } /* * The reference discards both results here. An error has already been diff --git a/tests/language/statements/multiple_per_line.bas b/tests/language/statements/multiple_per_line.bas new file mode 100644 index 0000000..0b631ba --- /dev/null +++ b/tests/language/statements/multiple_per_line.bas @@ -0,0 +1,17 @@ +10 REM Statements separated by colons. The COLON token existed from the start of +20 REM the port and nothing consumed it, so a line could hold only one statement. +30 PRINT "A" : PRINT "B" +40 A# = 1 : B# = 2 : PRINT A# + B# +50 REM An empty statement is not an error: a trailing separator, or a run of them. +60 PRINT "C" : +70 PRINT "D" :: PRINT "E" +80 REM Everything after THEN belongs to the condition, which is BASIC 7.0 and is +90 REM not something the reference had an opinion about -- it never got here. +100 IF 1 == 1 THEN PRINT "TRUE-1" : PRINT "TRUE-2" +110 IF 1 == 0 THEN PRINT "NEVER-1" : PRINT "NEVER-2" +120 REM With an ELSE, the tail belongs to the ELSE instead. +130 IF 1 == 1 THEN PRINT "T" ELSE PRINT "E" : PRINT "E-TAIL" +140 IF 1 == 0 THEN PRINT "T" ELSE PRINT "E" : PRINT "E-TAIL" +150 REM A colon does not confuse a bare verb that takes no argument. +160 PRINT : PRINT "AFTER A BARE PRINT" +170 PRINT "DONE" diff --git a/tests/language/statements/multiple_per_line.txt b/tests/language/statements/multiple_per_line.txt new file mode 100644 index 0000000..3f85ed6 --- /dev/null +++ b/tests/language/statements/multiple_per_line.txt @@ -0,0 +1,14 @@ +A +B +3 +C +D +E +TRUE-1 +TRUE-2 +T +E +E-TAIL + +AFTER A BARE PRINT +DONE diff --git a/tests/parser_commands.c b/tests/parser_commands.c index fcf8c72..f52db1f 100644 --- a/tests/parser_commands.c +++ b/tests/parser_commands.c @@ -39,6 +39,42 @@ static void expect_parse_error(const char *line) } } +/** + * @brief Parse a whole line and report how many statements came out of it. + * + * The statement loop the runtime uses, in miniature: parse until the token + * stream is spent, skipping the NULL leaf that an empty statement produces. + */ +static int count_statements(const char *line) +{ + akbasic_Parser parser; + akbasic_ASTLeaf *leaf = NULL; + akerr_ErrorContext *e = NULL; + int count = 0; + + e = akbasic_scanner_zero(&HARNESS_RUNTIME); + if ( e == NULL ) { + e = akbasic_scanner_scan(&HARNESS_RUNTIME, line, NULL, 0); + } + if ( e == NULL ) { + e = akbasic_parser_init(&parser, &HARNESS_RUNTIME); + } + while ( e == NULL && !akbasic_parser_is_at_end(&parser) ) { + leaf = NULL; + e = akbasic_parser_parse(&parser, &leaf); + if ( e == NULL && leaf != NULL ) { + count += 1; + } + } + if ( e != NULL ) { + fprintf(stderr, "FAIL: \"%s\" did not parse: %s\n", line, e->message); + akbasic_test_failures += 1; + test_discard_error(e); + return -1; + } + return count; +} + int main(void) { akbasic_ASTLeaf *leaf = NULL; @@ -106,6 +142,38 @@ int main(void) expect_parse_error("READ 1"); expect_parse_error("DATA A#"); + /* + * Colons separate statements. The token has existed since the scanner was + * written and nothing consumed it, so every one of these was a parse error + * until the statement loop learned to skip past it -- TODO.md section 4. + */ + TEST_REQUIRE_INT(count_statements("PRINT 1"), 1); + TEST_REQUIRE_INT(count_statements("PRINT 1 : PRINT 2"), 2); + TEST_REQUIRE_INT(count_statements("A# = 1 : B# = 2 : PRINT A# + B#"), 3); + + /* An empty statement is not an error, and does not count as one. */ + TEST_REQUIRE_INT(count_statements("PRINT 1 :"), 1); + TEST_REQUIRE_INT(count_statements(": PRINT 1"), 1); + TEST_REQUIRE_INT(count_statements("PRINT 1 :: PRINT 2"), 2); + TEST_REQUIRE_INT(count_statements(":"), 0); + TEST_REQUIRE_INT(count_statements(":::"), 0); + + /* + * A verb that takes no rval stops at the separator rather than trying to + * parse one out of it. + */ + TEST_REQUIRE_INT(count_statements("PRINT : PRINT 2"), 2); + + /* + * IF takes exactly one statement for each arm; the rest of the line arrives + * at the statement loop as ordinary statements, and it is the *runtime* that + * decides whether to run them. That split is asserted end to end in + * tests/language/statements/multiple_per_line.bas, because it is a question + * about execution rather than about the shape of the tree. + */ + TEST_REQUIRE_INT(count_statements("IF 1 == 1 THEN PRINT 1 : PRINT 2"), 2); + TEST_REQUIRE_INT(count_statements("IF 1 == 1 THEN PRINT 1 ELSE PRINT 2 : PRINT 3"), 2); + harness_stop(); return akbasic_test_failures; }