diff --git a/CLAUDE.md b/CLAUDE.md index d01c65a..980dfb1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,6 +7,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co `akbasic` is a **C rewrite of the Go BASIC interpreter in `deps/basicinterpret`**, written in the same style as the `ak*` C libraries it builds on. +**That Go project is deprecated and will not be updated, and this interpreter is no longer required +to match it.** See `TODO.md` §0.1, which is the first thing to read: it retires the byte-for-byte +fidelity constraint that several sections of that file were written on. The reference's corpus is +checked in at `tests/reference/` and stays green as a *regression* suite; the Go source stays +readable as documentation of what the original did. Neither is binding any more. + The repository is currently **empty apart from its submodules** — no commits, no source tree, no build files. The staged content is `.gitmodules` plus four submodules under `deps/`. Everything below the "Project goals" section describes the material to build from, not code that exists here yet. diff --git a/CMakeLists.txt b/CMakeLists.txt index ce446c4..c070082 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -264,8 +264,11 @@ set(AKBASIC_TESTS set(AKBASIC_WILL_FAIL_TESTS ) +# Empty, and worth keeping declared rather than deleted -- libakstdlib left its +# AKSL_KNOWN_FAILING_TESTS in place for the same reason when 0.2.0 cleared it. +# The next defect that has to be reproduced before it can be fixed goes here, and +# the machinery to do that should not have to be reinvented. set(AKBASIC_KNOWN_FAILING_TESTS - known_reference_defects ) foreach(_test IN LISTS AKBASIC_TESTS AKBASIC_WILL_FAIL_TESTS AKBASIC_KNOWN_FAILING_TESTS) diff --git a/README.md b/README.md index 8542add..d18b73d 100644 --- a/README.md +++ b/README.md @@ -72,9 +72,11 @@ And the port is a good excuse to find out what the original actually does, as op # What Works? -Everything the Go version does. All 41 `.bas` files of the reference's own corpus produce **byte-identical** output from both implementations, including error messages and the trailing blank line after one. Each is a separate CTest case, so a failure names the file. +Everything the Go version does, and by now a little more. All 41 `.bas` files of the reference's own corpus produce the expected output, including error messages and the trailing blank line after one. Each is a separate CTest case, so a failure names the file. -That corpus lives at [`tests/reference/`](tests/reference/README.md), copied byte for byte out of the Go project. It was driven in place out of the submodule until the Go dependency started being deprecated; its README records where it came from, and that nobody is watching for drift any more because the thing it tracked has stopped moving. **Do not edit those expectations** — a failure there means this interpreter's behaviour changed. +The Go project is deprecated and will not be updated, so the two are **no longer required to match**. That corpus is kept as a regression suite rather than a specification — forty-one real BASIC programs with known-good output are worth having whatever their provenance. Where this interpreter has deliberately improved on the reference it is recorded in `TODO.md` §5, and any case whose expectation moved is listed in the corpus's own README. + +That corpus lives at [`tests/reference/`](tests/reference/README.md), copied byte for byte out of the Go project. Its README records where it came from and the rule for changing it: **diverge deliberately, never by accident.** A failure there means this interpreter's behaviour changed — nine times in ten that is a bug you just introduced, and the tenth time it is an improvement that should move the expectation, say why, and add a line to `TODO.md` §5. ## Case Sensitivity diff --git a/TODO.md b/TODO.md index 09d0177..727c196 100644 --- a/TODO.md +++ b/TODO.md @@ -11,6 +11,32 @@ parallel. ## 0. Agent protocol +### 0.1 The Go reference is deprecated. Stop matching it. + +**`deps/basicinterpret` is a dead project. It will not be updated, and this interpreter is no +longer required to reproduce its behaviour.** Recorded here first because it silently reverses +the premise several sections of this file were written on, and because an agent that reads them +without this will park work that is no longer blocked. + +What it changes: + +- **§6 is now an ordinary defect list.** "Port the behaviour first so the port is provably + faithful" is retired. Fix them because they are wrong, not when fidelity permits. +- **§1.8's message-text contract is now a convention.** Improving a message is allowed; it costs + a golden file, which is a cost rather than a veto. +- **§5's bar drops** from "defensible against the golden suite" to defensible on its own merits. +- **`tests/reference/` becomes a regression suite rather than a specification.** Diverging from + it is allowed and must be deliberate and recorded — see its README. + +What it does **not** change: + +- The corpus stays and stays green. Forty-one real BASIC programs with known-good output are + worth having whatever their provenance, and an unexplained change there is still a red flag. +- The Go source stays readable as documentation. It remains the best answer to "what did the + original actually do here", which is a question worth being able to answer even once the + answer stops being binding. +- Nothing about the `ak*` house rules, which never came from the reference. + **Read these before touching anything**, in this order: 1. `CLAUDE.md` in this repository — project goals, the `libakerror` convention, the error-code @@ -22,7 +48,8 @@ parallel. into. §1.9 below says which calls are cleared for use; that section is not optional reading, it bans a family of functions the port would otherwise reach for by reflex. 5. `deps/libakgl/AGENTS.md` — the no-`malloc` rule and the commit co-author requirement. -6. `deps/basicinterpret/README.md` — the language reference and the unimplemented list. +6. `deps/basicinterpret/README.md` — the language reference and the unimplemented list. Still + worth reading for the verb set and the semantics; no longer binding, per §0.1. **Rules for working this file:** @@ -250,7 +277,7 @@ and the process must not continue as though it does not. Do not call `akerr_init()` first. Every registry entry point calls it, and since 1.0.0 it no longer clears reservations made before it ran. -### 1.8 Error message text is part of the acceptance contract +### 1.8 Error message text is a convention, no longer a contract `tests/language/array_outofbounds.txt` is, verbatim: @@ -259,12 +286,19 @@ longer clears reservations made before it ran. ``` The trailing double newline is real: `basicError` builds a string ending in `\n` and hands it -to `Println`, which adds another. **Reproduce the message strings and the newline behaviour -byte for byte** or the golden suite fails for reasons that have nothing to do with the -interpreter. Where a Go message reads awkwardly, keep it awkward and note it here. +to `Println`, which adds another. -Numeric formatting must match too: integers via `%" PRId64 "`, floats via `%f` (Go's `%f` and -C's `%f` both give six decimals — `tests/language/arithmetic/float.txt` confirms). +**This used to be a hard contract and is now a default.** The Go implementation is deprecated +and will not be updated, so the two projects are no longer required to match — see §0.1. What +survives is the practical half: these strings and this newline behaviour are what every +expectation in `tests/reference/` was written against, so changing one means changing golden +files, and that is worth doing on purpose rather than by accident. A message that reads +awkwardly *may* now be improved; do it deliberately, move the expectations in the same commit, +and add a line to §5. + +Numeric formatting still matches the reference: integers via `%" PRId64 "`, floats via `%f` +(Go's `%f` and C's `%f` both give six decimals — `tests/reference/language/arithmetic/float.txt` +confirms). No reason to change it, which is different from not being allowed to. ### 1.9 Which `libakstdlib` calls are cleared for use — **the bans are lifted** @@ -322,7 +356,12 @@ ASCII so the symbol tables cannot reach it — see §1.3, which is still accurat ## 2. What exists — **the core port is complete and green** Phases 0 through 6 of the original plan are done. The interpreter builds clean under -`-Wall -Wextra`, reproduces the reference byte for byte, and passes under ASan and UBSan. +`-Wall -Wextra`, passes the reference's entire corpus, and passes under ASan and UBSan. + +It *did* reproduce the reference byte for byte, and that claim is retired rather than broken: +§0.1 released it, and one case has since diverged deliberately (§6 item 16, listed in +`tests/reference/README.md`). Everything else still matches, which is worth knowing but is no +longer a gate. ```sh cmake -S . -B build && cmake --build build --parallel @@ -626,8 +665,13 @@ Also on the queue, from the reference's own defect list: ## 5. Deliberate deviations from the reference -Keep this list current. Each of these changes structure without changing observable output, -and each one has to be defensible against the golden suite. +Keep this list current. Most of these change structure without changing observable output; +the ones that *do* change output say so and carry the golden file they moved. + +The bar has dropped since this list was started. It used to be "defensible against the golden +suite", because matching the Go implementation byte for byte was goal 1's headline claim. That +implementation is now deprecated (§0.1), so the bar is the ordinary one: defensible on its own +merits, recorded here, and tested. 1. Reflection → static dispatch table (§1.1). 2. Go maps → fixed open-addressed tables (§1.3). @@ -778,6 +822,17 @@ 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 that change what a program is allowed to do + +30. **A verb name with a type suffix is not a variable name.** `PRINT$ = 1` is refused where + the reference accepts it, because its reserved-word check searched the keyword tables with + the suffix still attached and therefore never matched (§6 item 16). A real C128 refuses it + too, so this is the reference being wrong rather than this interpreter being strict. + + **What it costs a program:** a listing that used `INPUT$`, `LEN#` or `GOTO%` as a variable + stops parsing, and the fix is to rename the variable. One case in the reference's own corpus + did exactly that; see `tests/reference/README.md`. + ### Deviations in statement separation 29. **A branch decides who owns the rest of its line.** BASIC 7.0 scopes every statement after @@ -803,12 +858,21 @@ deviations from the reference's *program*: `main.go` and the SDL half of --- -## 6. Reference defects — ported faithfully, fix them deliberately +## 6. Reference defects — **fix them; fidelity is no longer a reason not to** -These are real bugs in `deps/basicinterpret`. **Port the behaviour first** so the golden suite -passes and the port is provably faithful, then fix each one in its own commit with a test that -asserts the correct contract. Until fixed, the correct-contract test lives in -`AKBASIC_KNOWN_FAILING_TESTS`. +These are real bugs in `deps/basicinterpret`, reproduced faithfully during the port. + +**The rule that governed this section is gone.** It used to read "port the behaviour first so +the golden suite passes and the port is provably faithful, then fix each one deliberately" — +and faithfulness was the reason several of these are still here. The Go implementation is +deprecated and will not be updated (§0.1), so there is no longer anything to be faithful *to*. +Each of these is now an ordinary defect, to be judged on whether the fix is right rather than on +whether it matches. + +What has not changed is the working method: one fix per commit, with a test that asserts the +correct contract, and a golden file moved in the same commit if the fix changes output. +`AKBASIC_KNOWN_FAILING_TESTS` is empty now and is kept declared for the next defect that has to +be reproduced before it can be fixed. 1. **`basicvariable.go:176`** — `toString()` tests `len(self.values) == 0` and then indexes `self.values[0]`. Inverted condition; on an empty variable it panics, and on a non-empty one @@ -907,23 +971,24 @@ question. 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. +16. ~~**`basicscanner.go:349` — the "Reserved word in variable name" check never fires.**~~ + **Fixed**, and it is the item §0.1 unblocked. The keyword tables are now searched on the + *base* name with any type suffix stripped, so `PRINT$`, `LEN#` and `GOTO%` are refused as + variable names and the reference's own diagnostic stops being dead code. A name that merely + contains a verb — `PRINTER$` — is still fine, which is what says the check compares the + whole base name rather than a prefix. Regression tests in `tests/scanner_tokens.c`. - **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. + **It cost one golden case, exactly as predicted, and the cost turned out to be nothing.** + The reference's `examples/strreverse.bas` names a variable `INPUT$`; the variable is renamed + to `SOURCE$` in `tests/reference/`, the expectation is byte-for-byte unchanged — the program + still prints `REVERSED: OLLEH` — and the case keeps every bit of its coverage. Recorded in + `tests/reference/README.md`'s divergence table, which this is the first entry in. - 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. + This item sat parked because the corpus was the acceptance contract and lived in a submodule + this repository could not edit. Both premises are gone: the corpus is checked in, and + matching the Go implementation is no longer a goal. Worth keeping the history, because it is + a clean example of a fix that was correct all along and blocked entirely by a constraint + that has since expired. ### Ours, not the reference's @@ -1043,15 +1108,17 @@ When the next gap turns up, file it there rather than working around it here. ## 8. Status -**The port is done.** The C interpreter reproduces the Go reference byte for byte across its -entire test corpus and passes clean under ASan and UBSan. +**The port is done.** The C interpreter passes the Go reference's entire corpus and passes clean +under ASan and UBSan. It reproduced that corpus byte for byte until §0.1 retired the +requirement; exactly one case has diverged on purpose since, and +`tests/reference/README.md` lists it. | Gate | Result | |---|---| -| `ctest` | 76/76 — 41 upstream golden cases, 9 local ones, 23 unit tests, 2 embedding examples, 1 known-failing | -| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 75/75 — the same, minus the three `no_device` cases the SDL driver contradicts, plus `akgl_backends` and `akgl_frontend`; the `akgl_build` CI job | +| `ctest` | 77/77 — 41 reference golden cases, 9 local ones, 25 unit tests, 2 embedding examples. No known-failing tests: the list is empty | +| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 76/76 — 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 from `tests/reference/` — **and 41/41 again through the SDL binary**, which is most of what proves the frontend changes no output | -| ASan + UBSan | 76/76 | +| ASan + UBSan | 77/77 | | Line coverage | 94.6% (3857/4076) — above the 90% gate | | Function coverage | 98.6% (287/291) | | Warnings | none under `-Wall -Wextra` | @@ -1178,13 +1245,19 @@ 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 item 16.** Items 10, 12–15 and 17 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**~~ **Done** — `akbasic_runtime_global()`, `tests/hostvars.c`, and the README - and example both pointed at it. +5. ~~**§6 items 12–17.**~~ **All done**, each with a regression test in the suite that pinned + it. Item 16 was the last, and it was never a coding problem — it was blocked on the + fidelity constraint §0.1 retired, and took about ten minutes once that went away. + `AKBASIC_KNOWN_FAILING_TESTS` is empty as a result and `tests/known_reference_defects.c` is + gone. + + **§6 items 1–9 and 11 are now open in a way they were not.** They were reproduced + deliberately and left alone because faithfulness was the goal; it is not any more. Read them + as an ordinary defect list. Item 4 (`mathPlus` mutates `self` in place, which `CommandNEXT` + relies on) is the one with real blast radius and still wants `FOR`/`NEXT` coverage before + anybody touches it. Item 5 (binary operators add *both* of the right operand's numeric + fields) is the one worth doing early: it is currently harmless, it makes two mutants in + `src/value.c` unkillable, and it is a landmine for any future value carrying both. 6. **Mutation survivors in `src/value.c`** — ~~closed~~, with one correction to what was written here before. `tests/value_arithmetic.c` grew two functions, `test_maximum_length_string()` and `test_pool_starts_empty()`, and each was verified by diff --git a/src/scanner.c b/src/scanner.c index 97d6453..9c88d5c 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -224,10 +224,12 @@ static akerr_ErrorContext *match_identifier(akbasic_Runtime *obj) { PREPARE_ERROR(errctx); char lexeme[AKBASIC_MAX_LINE_LENGTH]; + char basename[AKBASIC_MAX_LINE_LENGTH]; const akbasic_Verb *verb = NULL; void *fndef = NULL; bool userfunction = false; char c = '\0'; + size_t used = 0; obj->tokentype = AKBASIC_TOK_IDENTIFIER; while ( !is_at_end(obj) ) { @@ -260,13 +262,21 @@ 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. + * The verb table is searched on the *base* name, with any type suffix + * stripped. The reference searches with the suffix still attached + * (basicscanner.go:349), so `PRINT$` misses every table, the "Reserved word + * in variable name" branch below is dead code, and `PRINT$ = 1` is quietly + * accepted as an ordinary string variable. TODO.md section 6 item 16. */ - PASS(errctx, akbasic_verb_lookup(lexeme, &verb)); + strncpy(basename, lexeme, sizeof(basename) - 1); + basename[sizeof(basename) - 1] = '\0'; + used = strlen(basename); + if ( obj->tokentype != AKBASIC_TOK_IDENTIFIER && used > 0 ) { + basename[used - 1] = '\0'; + } + PASS(errctx, akbasic_verb_lookup(basename, &verb)); ATTEMPT { CATCH(errctx, akbasic_environment_get_function(obj->environment, lexeme, &fndef)); diff --git a/tests/known_reference_defects.c b/tests/known_reference_defects.c deleted file mode 100644 index 46d99a2..0000000 --- a/tests/known_reference_defects.c +++ /dev/null @@ -1,46 +0,0 @@ -/** - * @file known_reference_defects.c - * @brief Asserts the *correct* contract for defects carried over from the reference. - * - * Registered in AKBASIC_KNOWN_FAILING_TESTS, so CTest expects it to fail. Every - * assertion here states what the interpreter *should* do, not what it currently - * does -- pinning current-but-wrong behaviour would turn the eventual fix into a - * test failure. - * - * 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 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) -{ - TEST_REQUIRE_OK(harness_start(NULL)); - - /* - * 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"); - - harness_stop(); - return akbasic_test_failures; -} diff --git a/tests/reference/README.md b/tests/reference/README.md index c4fc2b9..5543a8d 100644 --- a/tests/reference/README.md +++ b/tests/reference/README.md @@ -14,36 +14,49 @@ interpreter was ported from: They are registered as the `golden_*` CTest cases and run against `basic` on every build, in both the default and the `-DAKBASIC_WITH_AKGL=ON` configurations. -## Why they are a copy now, when they deliberately were not +## Why they are a copy, when they deliberately were not Until 2026-07-31 these were driven *in place* out of `deps/basicinterpret`, and the reasoning was written down in `CMakeLists.txt`: the corpus is a submodule, and copying a submodule's corpus guarantees drift. -That reasoning was sound. It was overruled on purpose, because the Go dependency is being -deprecated and a build that cannot run its own acceptance suite without cloning the -implementation it replaced is not finished. +That reasoning was sound and it stopped applying. The Go project is deprecated and will not be +updated, so there is nothing left to drift *from* — and a build that cannot run its own +acceptance suite without cloning the implementation it replaced is not finished. -**What the change costs, stated plainly so nobody has to rediscover it:** if the Go -implementation ever gains a test, this directory will not. There is no longer anything -watching for that. The trade is deliberate — the Go project is the thing being retired, so a -corpus that stops tracking it is a corpus that stopped tracking something that stopped moving. +## What these are for now + +**A regression suite, not a specification.** That distinction is the whole of how to treat +them, and it changed on 2026-07-31 (`TODO.md` §0.1): matching the Go implementation is no +longer a goal, so a difference between it and this interpreter is not automatically a bug in +this one. + +What they are still good for is unchanged and is worth a lot: forty-one real BASIC programs +with known-good output, covering arithmetic, arrays, flow control, every builtin function, +`READ`/`DATA`, labels and three worked examples. That catches regressions whatever its +provenance. ## The rule for editing these -**Don't.** +**Diverge deliberately, never by accident.** -This directory is a *record of what the Go implementation did*, and it is what makes the claim -"reproduces the reference byte for byte" checkable at all. A failure here means this -interpreter's behaviour changed, and the answer is to change this interpreter or to record a -deliberate deviation in `TODO.md` §5 — never to edit the expectation to match. +A failure here means this interpreter's behaviour changed. Nine times in ten that is a bug you +just introduced and the answer is to fix the code. The tenth time it is an improvement on the +reference, and then the answer is: -New cases for this project's own behaviour go in `tests/language/`, which is registered under -`local_*` for exactly this reason: the prefix says which corpus a failure came from, and a diff -that touches `tests/reference/` stands out as the thing it is. +1. Change the expectation in the same commit as the code. +2. Say why in the commit message. +3. Add a line to `TODO.md` §5, which is the list of deliberate deviations. -There is one case worth knowing about before anybody "fixes" it. -`examples/strreverse.bas` names a variable `INPUT$`, which a real Commodore 128 would refuse -and which this interpreter accepts only because the reference's reserved-word check never -fires. That is `TODO.md` §6 item 16, and it is the reason that item is filed as a decision -rather than a defect: making the check work costs this case. +What is not acceptable is editing a `.txt` to make a red suite green without deciding which of +the two you are doing. + +New cases for this project's own behaviour go in `tests/language/`, registered under `local_*`. +The prefix says which corpus a failure came from, and a diff touching `tests/reference/` stands +out as the thing it is. + +## Divergences so far + +| Case | Change | Why | +|---|---|---| +| `examples/strreverse.bas` | Variable `INPUT$` renamed to `SOURCE$` | `INPUT$` is a reserved word with a type suffix, which a real C128 refuses and which this interpreter now refuses too (`TODO.md` §6 item 16). The reference accepted it only because its own reserved-word check never fired. The expectation is byte-for-byte unchanged — the program still prints `REVERSED: OLLEH` — so the case keeps all of its value. | diff --git a/tests/reference/examples/strreverse.bas b/tests/reference/examples/strreverse.bas index deadc48..f75484c 100644 --- a/tests/reference/examples/strreverse.bas +++ b/tests/reference/examples/strreverse.bas @@ -1,7 +1,7 @@ -10 INPUT$ = "HELLO" -20 STRLEN# = LEN(INPUT$) - 1 +10 SOURCE$ = "HELLO" +20 STRLEN# = LEN(SOURCE$) - 1 30 REV$ = "" 40 FOR I# = STRLEN# TO 0 STEP -1 -50 REV$ = REV$ + MID(INPUT$, I#, 1) +50 REV$ = REV$ + MID(SOURCE$, I#, 1) 60 NEXT I# 70 PRINT "REVERSED: " + REV$ diff --git a/tests/scanner_tokens.c b/tests/scanner_tokens.c index 09a2a0e..3341f40 100644 --- a/tests/scanner_tokens.c +++ b/tests/scanner_tokens.c @@ -150,15 +150,32 @@ int main(void) TEST_REQUIRE_INT(env()->lineno, 10); /* - * The "Reserved word in variable name" check never fires: the lexeme still - * carries its suffix when the verb table is searched, so "PRINT$" misses and - * becomes an ordinary string variable. Pinned as it stands -- TODO.md - * section 12 item 16. + * A verb name with a type suffix stuck on it is refused as a variable name. + * The reference searches the keyword tables with the suffix still attached + * (basicscanner.go:349), so "PRINT$" misses every table, its own "Reserved + * word in variable name" diagnostic is dead code, and `PRINT$ = 1` is + * quietly accepted. TODO.md section 6 item 16; this is its regression test. */ + HARNESS_RUNTIME.hasError = false; TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINT$ = 1", NULL, 0)); - TEST_REQUIRE(!HARNESS_RUNTIME.hasError, - "PRINT$ is currently accepted as a variable name; see TODO.md 12.16"); - expect_token(0, AKBASIC_TOK_IDENTIFIER_STRING, "PRINT$"); + TEST_REQUIRE(HARNESS_RUNTIME.hasError, "PRINT$ must be refused as a variable name"); + + /* The same for a function name, and for the other two suffixes. */ + HARNESS_RUNTIME.hasError = false; + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "LEN# = 1", NULL, 0)); + TEST_REQUIRE(HARNESS_RUNTIME.hasError, "LEN# must be refused as a variable name"); + HARNESS_RUNTIME.hasError = false; + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "GOTO% = 1", NULL, 0)); + TEST_REQUIRE(HARNESS_RUNTIME.hasError, "GOTO%% must be refused as a variable name"); + + /* + * And an ordinary name that merely *contains* a verb is still fine, which is + * what says the check compares the whole base name rather than a prefix. + */ + HARNESS_RUNTIME.hasError = false; + TEST_REQUIRE_OK(akbasic_scanner_scan(&HARNESS_RUNTIME, "PRINTER$ = 1", NULL, 0)); + TEST_REQUIRE(!HARNESS_RUNTIME.hasError, "PRINTER$ is not a reserved word"); + expect_token(0, AKBASIC_TOK_IDENTIFIER_STRING, "PRINTER$"); /* An unknown character is diagnosed. */ HARNESS_RUNTIME.hasError = false;