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

Retire the byte-for-byte fidelity constraint, and fix what it was blocking

Browse Source 
The Go reference is deprecated and will not be updated, so the two projects
are no longer required to match. Recorded as TODO.md section 0.1, first thing
in the file, because it silently reverses the premise several later sections
were written on -- an agent reading section 6 without it will park work that
is no longer blocked.

Section 6 item 16 was the item waiting on exactly this and is now fixed: the
keyword tables are 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. It cost the one golden case predicted, and
the cost was nothing -- renaming a variable in strreverse.bas left its expected
output byte-for-byte unchanged.

AKBASIC_KNOWN_FAILING_TESTS is empty as a result and known_reference_defects.c
is gone. Section 6 items 1-9 and 11 are reopened as an ordinary defect list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Andrew Kesterson
2026-07-31 12:52:00 -04:00
parent eb5374d212
commit 583c0abbd2
9 changed files with 205 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files

157
TODO.md
View File

@@ -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