603 lines
34 KiB
Markdown
603 lines
34 KiB
Markdown
|
|
# TODO
|
|||
|
|
|
|||
|
|
Implementation plan for the Go → C port of `deps/basicinterpret`.
|
|||
|
|
|
|||
|
|
This file is written to be executed by AI agents, not read by humans for inspiration. Every
|
|||
|
|
item names the files it touches, the exact deliverable, and the command that proves it done.
|
|||
|
|
Work the phases in order. Inside a phase, items with no `Depends:` line may be done in
|
|||
|
|
parallel.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 0. Agent protocol
|
|||
|
|
|
|||
|
|
**Read these before touching anything**, in this order:
|
|||
|
|
|
|||
|
|
1. `CLAUDE.md` in this repository — project goals, the `libakerror` convention, the error-code
|
|||
|
|
range map, and the dependency versions.
|
|||
|
|
2. `deps/libakerror/AGENTS.md` — the `ATTEMPT`/`CLEANUP`/`PROCESS`/`HANDLE`/`FINISH` protocol.
|
|||
|
|
3. `deps/libakerror/UPGRADING.md` — 1.0.0's status registry. Required before writing an error
|
|||
|
|
code; the mechanism it replaced is gone.
|
|||
|
|
4. `deps/libakstdlib/TODO.md` §2.1 — six **confirmed** defects in the library this port calls
|
|||
|
|
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.
|
|||
|
|
|
|||
|
|
**Rules for working this file:**
|
|||
|
|
|
|||
|
|
- **Do not** mark an item done until its acceptance command passes on a clean out-of-tree
|
|||
|
|
build. "It compiles" is not acceptance.
|
|||
|
|
- **Do** update this file in the same commit as the work: strike the item, or replace it with
|
|||
|
|
the defect it uncovered. This file holds outstanding items only.
|
|||
|
|
- **Do** add the agent program name, model name and version as a commit co-author. That rule
|
|||
|
|
comes from `libakgl` and applies here.
|
|||
|
|
- **Never** hand-edit generated output. `build/` trees and the generated `akerror.h` are
|
|||
|
|
off-limits; change the generator.
|
|||
|
|
- **Never** reformat a file you are not otherwise changing.
|
|||
|
|
- When a step is blocked because `libakgl` cannot supply a capability, **do not work around
|
|||
|
|
it here.** File it in `deps/libakgl/TODO.md` in the numbered prose style of that file's
|
|||
|
|
"Carried over" section and note the block in §7 below.
|
|||
|
|
|
|||
|
|
**Style**, restated so nobody has to go look: C99, 4-space indent, tabs at width 8
|
|||
|
|
(`stroustrup`), function-body braces in column 0 on their own line, control braces on the same
|
|||
|
|
line, **always brace**, spaces inside control-flow parens — `if ( x == y ) {`. Pointer star
|
|||
|
|
binds to the identifier: `char *name`. Prefix is `akbasic_` for functions,
|
|||
|
|
`akbasic_TypeName` for types, `AKBASIC_UPPER_SNAKE` for macros. `static` helpers drop the
|
|||
|
|
prefix. Parameter names must match between header and source.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 1. Design decisions already made
|
|||
|
|
|
|||
|
|
These are settled. Do not relitigate them mid-port; if evidence says one is wrong, say so in
|
|||
|
|
a commit that changes it deliberately, and update this section.
|
|||
|
|
|
|||
|
|
### 1.1 Reflection becomes one aligned dispatch table
|
|||
|
|
|
|||
|
|
The Go runtime resolves verbs with `reflect.MethodByName("Command" + NAME)`
|
|||
|
|
(`basicruntime.go:401`), functions with `"Function" + NAME`, and special parse paths with
|
|||
|
|
`"ParseCommand" + NAME` (`basicparser.go:103`). C has no reflection and we are not adding
|
|||
|
|
any. Replace all three with **one** static table in `src/verbs.c`, sorted by name, searched
|
|||
|
|
with `bsearch(3)`:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
/* name token type parse handler exec handler */
|
|||
|
|
{ "AUTO", AKBASIC_TOK_CMDIMM, NULL, cmd_auto },
|
|||
|
|
{ "DATA", AKBASIC_TOK_COMMAND, parse_data, cmd_data },
|
|||
|
|
{ "DEF", AKBASIC_TOK_COMMAND, parse_def, cmd_def },
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
A `NULL` parse handler means "parse the rval as a plain expression", which is exactly what
|
|||
|
|
`commandByReflection` returning `(nil, nil)` means today. Adding a verb is adding one row plus
|
|||
|
|
two functions. **Keep the table column-aligned and one row per verb** — it is a table, so it
|
|||
|
|
gets laid out as one.
|
|||
|
|
|
|||
|
|
This also kills the Go scanner's three separate maps (`reservedwords`, `commands`,
|
|||
|
|
`functions`, `basicscanner.go:64-67`): the token type lives in the same row.
|
|||
|
|
|
|||
|
|
### 1.2 Strings are fixed-size and live inline
|
|||
|
|
|
|||
|
|
`libakstdlib` has no string type. `libakgl` has `akgl_String` but it is `PATH_MAX` bytes,
|
|||
|
|
refcounted, and pool-allocated — wrong shape for a value that gets copied on every
|
|||
|
|
assignment, and it would drag a `libakgl` dependency into the core interpreter.
|
|||
|
|
|
|||
|
|
Define in `include/akbasic/types.h`:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
#define AKBASIC_MAX_STRING_LENGTH 256 /* matches AKBASIC_MAX_LINE_LENGTH */
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
and give `akbasic_Value` a `char stringval[AKBASIC_MAX_STRING_LENGTH]` **inline**. `clone()`
|
|||
|
|
becomes a struct assignment. No allocator, no refcount, no lifetime question.
|
|||
|
|
|
|||
|
|
**Tradeoff, stated:** every `akbasic_Value` is ~300 bytes, so one environment's
|
|||
|
|
`values[AKBASIC_MAX_VALUES]` pool is ~19KB, and 32 environments is ~610KB of BSS. That is
|
|||
|
|
fine on a PC and is the price of never calling `malloc`. If it ever isn't fine, the knob is
|
|||
|
|
`AKBASIC_MAX_STRING_LENGTH`, not the allocator.
|
|||
|
|
|
|||
|
|
Truncation is an **error**, not a silent clamp: `FAIL_RETURN(e, AKBASIC_ERR_VALUE, ...)`.
|
|||
|
|
|
|||
|
|
### 1.3 Maps become fixed-capacity open-addressed tables
|
|||
|
|
|
|||
|
|
Five Go maps need replacing:
|
|||
|
|
|
|||
|
|
| Go site | Purpose | C replacement |
|
|||
|
|
|---|---|---|
|
|||
|
|
| `BasicScanner.reservedwords/commands/functions` | keyword → token type | the §1.1 static table + `bsearch` |
|
|||
|
|
| `BasicEnvironment.variables` | name → `*BasicVariable` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_VARIABLES` |
|
|||
|
|
| `BasicEnvironment.functions` | name → `*BasicFunctionDef` | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_FUNCTIONS` |
|
|||
|
|
| `BasicEnvironment.labels` | name → line number | `akbasic_SymbolTable`, capacity `AKBASIC_MAX_LABELS` |
|
|||
|
|
|
|||
|
|
One implementation, `src/symtab.c`, keyed by `aksl_strhash_djb2()` (already in
|
|||
|
|
`libakstdlib`) with linear probing and a fixed slot array. **Use the existing hash; do not
|
|||
|
|
write another one.** Table full is an error, not a resize.
|
|||
|
|
|
|||
|
|
Caveat, from `deps/libakstdlib/TODO.md` §1.6/§2.2.6: the wrapper sign-extends `char`, so a
|
|||
|
|
high-bit byte hashes wrong — `"\xff\xfe"` returns 5859874 where the `unsigned char` answer is
|
|||
|
|
5868578. BASIC identifiers are 7-bit ASCII (the scanner only accepts `IsLetter`/`IsDigit` plus
|
|||
|
|
a type suffix), so this cannot bite the symbol tables. It **would** bite if anyone later keys
|
|||
|
|
a table on a string literal or a filename. Do not work around it here; it is already filed
|
|||
|
|
upstream.
|
|||
|
|
|
|||
|
|
### 1.4 Environments come from a pool and are released
|
|||
|
|
|
|||
|
|
Go calls `new(BasicEnvironment)` at `basicruntime.go:121` and `basicparser_commands.go:124`
|
|||
|
|
and never frees one. A long-running `GOSUB` or `FOR` in Go leaks; the GC eventually catches
|
|||
|
|
some of it, and nothing in the tests notices.
|
|||
|
|
|
|||
|
|
C gets `HEAP_ENVIRONMENT[AKBASIC_MAX_ENVIRONMENTS]` with
|
|||
|
|
`akbasic_env_acquire()` / `akbasic_env_release()`, in the shape of `akgl_heap_next_*`.
|
|||
|
|
`akbasic_runtime_prev_environment()` **must** release the environment it pops. Pool
|
|||
|
|
exhaustion is `AKBASIC_ERR_ENVIRONMENT`, reported with the current line number.
|
|||
|
|
|
|||
|
|
Watch the one place this is not a clean stack: `userFunction` (`basicruntime.go:348`) stores
|
|||
|
|
a `BasicEnvironment` **by value** inside `BasicFunctionDef` and re-`init()`s it on every call.
|
|||
|
|
In C the funcdef holds an `akbasic_Environment *` acquired at `DEF` time and reset per call —
|
|||
|
|
it is owned by the funcdef, not the pool's free list, until the funcdef dies.
|
|||
|
|
|
|||
|
|
### 1.5 Output goes through a text sink backend
|
|||
|
|
|
|||
|
|
`Write()` and `Println()` (`basicruntime_graphics.go:140,148`) mirror every line to stdout
|
|||
|
|
*and* to an SDL surface. That mirror is the only reason the golden-file suite works. Do not
|
|||
|
|
reproduce it as a hardcoded pair of calls.
|
|||
|
|
|
|||
|
|
Define a record of function pointers, populated by an initializer — the house pattern:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
typedef struct akbasic_TextSink
|
|||
|
|
{
|
|||
|
|
void *self;
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *(*write)(struct akbasic_TextSink *self, char *text);
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *(*writeln)(struct akbasic_TextSink *self, char *text);
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *(*readline)(struct akbasic_TextSink *self, char *dest, size_t len);
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *(*clear)(struct akbasic_TextSink *self);
|
|||
|
|
} akbasic_TextSink;
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
`akbasic_sink_init_stdio()` in the library, `akbasic_sink_init_akgl()` in the akgl-backed
|
|||
|
|
module (phase 9). The driver picks. Cursor arithmetic, wrapping and scrolling belong to the
|
|||
|
|
akgl sink, not to the interpreter.
|
|||
|
|
|
|||
|
|
### 1.6 The interpreter steps; it does not run
|
|||
|
|
|
|||
|
|
Go's `run()` (`basicruntime.go:682`) is a `for {}` that owns the process until `MODE_QUIT`.
|
|||
|
|
Goal 3 forbids that: a host game must be able to bound execution.
|
|||
|
|
|
|||
|
|
The library exposes:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_step(akbasic_Runtime *obj);
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *akbasic_runtime_run(akbasic_Runtime *obj, int maxsteps);
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
`_step()` does exactly what one iteration of Go's `for {}` body does and returns.
|
|||
|
|
`_run(obj, maxsteps)` loops `_step()` until the mode is `AKBASIC_MODE_QUIT` or `maxsteps`
|
|||
|
|
steps have elapsed; `maxsteps <= 0` means unbounded, which is what the standalone driver
|
|||
|
|
passes. **This is a deliberate restructure, and it must not change a single byte of golden
|
|||
|
|
output.**
|
|||
|
|
|
|||
|
|
`QUIT` sets `AKBASIC_MODE_QUIT` and returns. **Nothing in the library calls `exit()`,
|
|||
|
|
`abort()`, or `FINISH_NORETURN`.** `FINISH_NORETURN` appears exactly once in the tree, in
|
|||
|
|
`src/main.c`.
|
|||
|
|
|
|||
|
|
### 1.7 Error codes are absolute, reserved from the 1.0.0 registry
|
|||
|
|
|
|||
|
|
`deps/libakerror` is at **1.0.0**. Read `deps/libakerror/UPGRADING.md` before writing an error
|
|||
|
|
code; `CLAUDE.md`'s error-code section is the condensed version. Three things this changes
|
|||
|
|
from what you may have seen in an older draft or in `libakgl`:
|
|||
|
|
|
|||
|
|
- `AKERR_MAX_ERR_VALUE` **no longer exists**. The name registry is sparse and takes any `int`.
|
|||
|
|
There is nothing for a consumer to size, and no compile definition to set. Anywhere you find
|
|||
|
|
one, delete it.
|
|||
|
|
- `__AKERR_ERROR_NAMES` **no longer exists**. The table is private.
|
|||
|
|
- Codes are **absolute integer constants at 256 or above**, never `AKERR_LAST_ERRNO_VALUE + N`.
|
|||
|
|
The offset scheme is what produced the live `libakgl` collision documented in `CLAUDE.md`.
|
|||
|
|
|
|||
|
|
`akbasic` owns **512–767** per the range map in `CLAUDE.md`. Declare it as an `enum`, so the
|
|||
|
|
values stay compile-time integer constants (`HANDLE` expands to `case` labels, which require
|
|||
|
|
that) and adding a code does not mean renumbering an offset:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
#define AKBASIC_OWNER "akbasic"
|
|||
|
|
|
|||
|
|
enum {
|
|||
|
|
AKBASIC_ERR_BASE = 512, /** Start of akbasic's reserved status range */
|
|||
|
|
AKBASIC_ERR_SYNTAX = AKBASIC_ERR_BASE,
|
|||
|
|
/** Parse-time grammar violation */
|
|||
|
|
AKBASIC_ERR_TYPE, /** Incompatible types in an operation */
|
|||
|
|
AKBASIC_ERR_UNDEFINED, /** Reference to an undefined verb, function or label */
|
|||
|
|
AKBASIC_ERR_BOUNDS, /** Array subscript or pool index out of range */
|
|||
|
|
AKBASIC_ERR_ENVIRONMENT, /** Environment pool exhausted or orphaned environment */
|
|||
|
|
AKBASIC_ERR_VALUE, /** A value was malformed, truncated or unconvertible */
|
|||
|
|
AKBASIC_ERR_STATE, /** A verb ran outside the block structure it requires */
|
|||
|
|
AKBASIC_ERR_LIMIT = AKBASIC_ERR_BASE + 256
|
|||
|
|
};
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
`akbasic_init()` reserves the **whole** 256 in one call and then names each code. Both
|
|||
|
|
registry calls return `akerr_ErrorContext *` and are `AKERR_NOIGNORE`, so a collision is an
|
|||
|
|
ordinary error — `PASS` it and let it propagate out of init:
|
|||
|
|
|
|||
|
|
```c
|
|||
|
|
akerr_ErrorContext AKERR_NOIGNORE *akbasic_init(void)
|
|||
|
|
{
|
|||
|
|
PREPARE_ERROR(errctx);
|
|||
|
|
|
|||
|
|
PASS(errctx, akerr_reserve_status_range(AKBASIC_ERR_BASE,
|
|||
|
|
AKBASIC_ERR_LIMIT - AKBASIC_ERR_BASE,
|
|||
|
|
AKBASIC_OWNER));
|
|||
|
|
PASS(errctx, akerr_register_status_name(AKBASIC_OWNER, AKBASIC_ERR_SYNTAX,
|
|||
|
|
"Syntax Error"));
|
|||
|
|
/* ... one per code ... */
|
|||
|
|
SUCCEED_RETURN(errctx);
|
|||
|
|
}
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Reserve the whole range in **one** call — a subset or superset of your own range raises
|
|||
|
|
`AKERR_STATUS_RANGE_OVERLAP`, not a no-op. Use `akerr_register_status_name()`, never the
|
|||
|
|
two-argument `akerr_name_for_status(status, name)` set path: the owned form is the one that
|
|||
|
|
catches a component writing into a range that is not its own, and the two-argument form is
|
|||
|
|
what let `libakgl` silently clobber `libakerror`'s names.
|
|||
|
|
|
|||
|
|
There is no startup ceiling to assert any more — the BSS-overflow hazard the old draft
|
|||
|
|
defended against was deleted along with `AKERR_MAX_ERR_VALUE`. What replaces it is the
|
|||
|
|
reservation itself: if `akbasic_init()` returns an error, something else owns part of 512–767
|
|||
|
|
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
|
|||
|
|
|
|||
|
|
`tests/language/array_outofbounds.txt` is, verbatim:
|
|||
|
|
|
|||
|
|
```
|
|||
|
|
? 20 : RUNTIME ERROR Variable index access out of bounds at dimension 0: 4 (max 2)\n\n
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
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.
|
|||
|
|
|
|||
|
|
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).
|
|||
|
|
|
|||
|
|
### 1.9 Which `libakstdlib` calls are cleared for use, and which are not
|
|||
|
|
|
|||
|
|
`deps/libakstdlib` is at **0.1.0** and its `TODO.md` §2.1 lists six confirmed defects. Four
|
|||
|
|
carry a known-failing test, so a green `ctest` over there does not mean what you would
|
|||
|
|
assume. Three of the six sit directly in the path of this port. Check this table before
|
|||
|
|
reaching for anything in `akstdlib.h`:
|
|||
|
|
|
|||
|
|
| Call | Verdict | Why |
|
|||
|
|
|---|---|---|
|
|||
|
|
| `aksl_fopen` / `_fread` / `_fwrite` / `_fclose` | **use** | `DLOAD`/`DSAVE` go through these, never bare libc. `aksl_fopen` does not NULL-check `pathname`/`mode` (§2.2.2) — validate before calling. |
|
|||
|
|
| `aksl_malloc` / `_free` | **do not use** | We allocate nothing. Pools only. |
|
|||
|
|
| `aksl_memset` / `_memcpy` | **use** | No open defects. |
|
|||
|
|
| `aksl_printf` / `_fprintf` / `_sprintf` | **use, under protest** | The stdio text sink needs them. All three call `va_start` with no matching `va_end` on any path (§2.1.4) — undefined behaviour, not ours to fix here, but do not add a fourth wrapper in the same shape. |
|
|||
|
|
| `aksl_atoi` / `_atol` / `_atoll` / `_atof` | **BANNED — see below** | Cannot report a conversion failure at all (§2.1.5). |
|
|||
|
|
| `aksl_realpath` | **do not use** | Cannot express a buffer size, never NULL-checks `resolved_path`, and its own error path reads uninitialised memory (§2.1.6). Nothing in the port needs it. |
|
|||
|
|
| `aksl_strhash_djb2` | **use** | See §1.3 for the high-bit caveat, which does not apply to identifiers. |
|
|||
|
|
| `aksl_list_*` | **do not use** | `aksl_list_append` silently truncates to a two-node chain and `aksl_list_iterate` skips the first half of the list — both confirmed (§2.1.1, §2.1.2). The symbol tables in §1.3 are open-addressed and need none of this. |
|
|||
|
|
| `aksl_tree_iterate` | **do not use** | `AKERR_ITERATOR_BREAK` does not stop the traversal (§2.1.3). Nothing in the port needs a tree. |
|
|||
|
|
|
|||
|
|
**The `aksl_ato*` ban is load-bearing, so here is the whole argument.** `atoi("not a number")`
|
|||
|
|
returns *success* with `0`, and `atoi("99999999999999999999")` returns *success* with a
|
|||
|
|
wrapped value. The Go reference checks the conversion error at every one of these sites and
|
|||
|
|
turns it into a BASIC error:
|
|||
|
|
|
|||
|
|
| Go site | What it does on bad input |
|
|||
|
|
|---|---|
|
|||
|
|
| `basicgrammar.go:224` `newLiteralInt` → `strconv.ParseInt` | returns `err`, which the parser reports |
|
|||
|
|
| `basicgrammar.go` `newLiteralFloat` → `strconv.ParseFloat` | returns `err` |
|
|||
|
|
| `basicscanner.go` `matchNumber` → `strconv.Atoi` | `PARSE` error, `"INTEGER CONVERSION ON '%s'"` |
|
|||
|
|
| `basicruntime_functions.go:742` `FunctionVAL` | converts a string at runtime and must be able to fail |
|
|||
|
|
|
|||
|
|
Port those onto `aksl_ato*` and every one of them silently succeeds with `0`. That is not a
|
|||
|
|
cosmetic loss: it converts four diagnosable errors into wrong answers, and `VAL("garbage")`
|
|||
|
|
starts returning `0.000000` instead of raising. The existing golden files would not catch it —
|
|||
|
|
`tests/language/functions/val.txt` only covers `"32"`, `"123.456"` and `"-256"`, all valid.
|
|||
|
|
|
|||
|
|
**Do this instead:** write one local pair of converters in `src/convert.c`
|
|||
|
|
(`akbasic_str_to_int64`, `akbasic_str_to_double`) over `strtoll`/`strtod`, with `errno = 0`
|
|||
|
|
before the call, an `endptr` check for both "no digits consumed" and "trailing junk", and a
|
|||
|
|
range check — raising `AKBASIC_ERR_VALUE` and `ERANGE` respectively. That is the contract
|
|||
|
|
`libakstdlib` §2.1.5 says the `aksl_ato*` family *should* have. When it grows it, delete
|
|||
|
|
`src/convert.c` and switch the call sites over; until then, do not paper over the gap by
|
|||
|
|
calling the broken function and hoping.
|
|||
|
|
|
|||
|
|
*Acceptance for `src/convert.c`:* `tests/convert.c` — valid decimal, valid hex, empty string,
|
|||
|
|
pure garbage, trailing junk, and an overflowing literal, each asserting the raised status.
|
|||
|
|
Add a `.bas`/`.txt` pair for `VAL` on a non-numeric string in the same commit; there is no
|
|||
|
|
golden coverage for it today.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 2. What exists — **the 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.
|
|||
|
|
|
|||
|
|
```sh
|
|||
|
|
cmake -S . -B build && cmake --build build --parallel
|
|||
|
|
ctest --test-dir build --output-on-failure # 59/59
|
|||
|
|
cmake -S . -B build-asan -DAKBASIC_SANITIZE=ON # 59/59
|
|||
|
|
cmake -S . -B build-cov -DAKBASIC_COVERAGE=ON # 92.3% line, 96.9% function
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
| Module | Source | Reference |
|
|||
|
|
|---|---|---|
|
|||
|
|
| Strict numeric conversion | `src/convert.c` | *(new; see §1.9)* |
|
|||
|
|
| Symbol table | `src/symtab.c` | the five Go maps |
|
|||
|
|
| Value | `src/value.c` | `basicvalue.go` |
|
|||
|
|
| Variable | `src/variable.c` | `basicvariable.go` |
|
|||
|
|
| Tokens and AST leaves | `src/grammar.c` | `basicgrammar.go` + `basicparser.go` |
|
|||
|
|
| Dispatch table | `src/verbs.c` | the three reflection lookups |
|
|||
|
|
| Scanner | `src/scanner.c` | `basicscanner.go` |
|
|||
|
|
| Parser | `src/parser.c`, `src/parser_commands.c` | `basicparser*.go` |
|
|||
|
|
| Environment | `src/environment.c` | `basicenvironment.go` |
|
|||
|
|
| Runtime | `src/runtime.c` | `basicruntime.go` minus SDL |
|
|||
|
|
| Verbs and functions | `src/runtime_commands.c`, `src/runtime_functions.c` | `basicruntime_{commands,functions}.go` |
|
|||
|
|
| Text sink | `src/sink_stdio.c` | `basicruntime_graphics.go`'s stdout mirror |
|
|||
|
|
| Driver | `src/main.c` | `main.go` |
|
|||
|
|
|
|||
|
|
**The acceptance suite is the reference's own corpus, driven in place.** All 41 `.bas` files
|
|||
|
|
under `deps/basicinterpret/tests/` are registered as individual CTest cases and byte-compared
|
|||
|
|
against their `.txt` — including the trailing double newline on an error line (§1.8). Nothing
|
|||
|
|
is copied, so the corpus cannot drift from its upstream.
|
|||
|
|
|
|||
|
|
Eighteen unit tests cover the modules the corpus cannot reach on its own.
|
|||
|
|
`tests/known_reference_defects.c` is registered in `AKBASIC_KNOWN_FAILING_TESTS` and asserts
|
|||
|
|
the **correct** contract for six of the defects in §6; when one is fixed CTest reports
|
|||
|
|
"unexpectedly passed", which is the cue to split that assertion out and strike the item.
|
|||
|
|
|
|||
|
|
Two budget numbers moved during implementation and are worth knowing:
|
|||
|
|
`AKBASIC_MAX_ARRAY_ELEMENTS` (1024) caps one array and `AKBASIC_MAX_ARRAY_VALUES` (4096) caps
|
|||
|
|
all of them together, drawn from an `akbasic_ValuePool` the runtime owns. The reference had no
|
|||
|
|
such ceiling because it called `make()`.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
|
|||
|
|
## 3. Remaining work: the akgl text sink
|
|||
|
|
|
|||
|
|
**`src/sink_akgl.c`.** Implement the §1.5 vtable against `akgl_text_loadfont` and
|
|||
|
|
`akgl_text_rendertextat`. Owns the cursor, the wrap, and the scroll — everything in
|
|||
|
|
`basicruntime_graphics.go` except `Write`/`Println`, which are now the sink interface itself.
|
|||
|
|
|
|||
|
|
**The interpreter does not own the window, the renderer, or the game loop.** The sink draws
|
|||
|
|
through whatever renderer the host already initialized. `akbasic_sink_init_akgl()` takes the
|
|||
|
|
renderer; it does not create one.
|
|||
|
|
|
|||
|
|
**Call `akgl_error_init()` before anything else in `libakgl`.** New in `libakgl` 0.1.0
|
|||
|
|
(`deps/libakgl/src/error.c`): it reserves the 256–260 status band and registers a name for
|
|||
|
|
each `AKGL_ERR_*` code. `akgl_game_init()` calls it first, but we drive subsystems directly and
|
|||
|
|
never call `akgl_game_init()`, so it is ours to call. Skip it and every `AKGL_ERR_*` that
|
|||
|
|
reaches a stack trace prints "Unknown Error" — which is exactly the defect the upstream commit
|
|||
|
|
found in `game.c`, where an SDL failure five lines ahead of the old registration site was
|
|||
|
|
guaranteed to be unnamed. It is idempotent, so calling it from both `akbasic_sink_init_akgl()`
|
|||
|
|
and a host that already did is harmless.
|
|||
|
|
`PASS` its result; a range collision is an initialization failure, not a warning.
|
|||
|
|
|
|||
|
|
**Known gap:** `libakgl` has no text-measurement call — the Go code needs
|
|||
|
|
`font.SizeUTF8("A")` (`basicruntime.go:96`) to compute `maxCharsW`/`maxCharsH`, and there is no
|
|||
|
|
`akgl_text_*` equivalent. **File it in `deps/libakgl/TODO.md`** before writing a workaround.
|
|||
|
|
See §7.
|
|||
|
|
|
|||
|
|
*Acceptance:* `tests/sink_akgl.c` against the offscreen renderer harness, or — if that harness
|
|||
|
|
does not exist yet — a known-failing test plus a `libakgl` TODO entry saying so.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 4. Remaining work: language completion (goal 2)
|
|||
|
|
|
|||
|
|
The work queue is the "What Isn't Implemented" list in `deps/basicinterpret/README.md`.
|
|||
|
|
Each verb is: table row (§1.1) → parse handler if it needs one → exec handler → unit test →
|
|||
|
|
a new `.bas`/`.txt` pair. **Both** kinds of test; they answer different questions.
|
|||
|
|
|
|||
|
|
Order by what unblocks the most, and by what does not need `libakgl` to grow first:
|
|||
|
|
|
|||
|
|
| Group | Verbs | Blocked on |
|
|||
|
|
|---|---|---|
|
|||
|
|
| A. Structure | `DO`, `LOOP`, `WHILE`, `UNTIL`, `ON`, `BEGIN`, `BEND`, `END` | nothing — reuses `waitingForCommand` |
|
|||
|
|
| B. Housekeeping | `NEW`, `CLR`, `CONT`, `RESTORE`, `RENUMBER`, `SWAP`, `TRON`, `TROFF`, `HELP` | nothing |
|
|||
|
|
| C. Errors | `TRAP`, `RESUME`, `ER`, `ERR` | needs a BASIC-visible error object |
|
|||
|
|
| D. Strings/format | `USING`, `PUDEF`, `WIDTH`, `CHAR` | nothing |
|
|||
|
|
| E. Console | `GET`, `GETKEY`, `SCNCLR`, `LOCATE`, `WINDOW`, `KEY`, `SLEEP`, `WAIT`, `TI` | sink + `libakgl` input |
|
|||
|
|
| F. Disk | `DOPEN`, `DCLOSE`, `APPEND`, `RECORD`, `HEADER`, `COLLECT`, `BACKUP`, `COPY`, `CONCAT`, `RENAME`, `SCRATCH`, `DIRECTORY`, `CATALOG`, `DCLEAR`, `DVERIFY`, `SAVE`, `LOAD`, `VERIFY`, `BLOAD`, `BSAVE`, `BOOT` | `aksl_f*` only; no new akgl |
|
|||
|
|
| G. Graphics | `GRAPHIC`, `DRAW`, `BOX`, `CIRCLE`, `PAINT`, `COLOR`, `SCALE`, `SSHAPE`, `GSHAPE`, `LOCATE` | **`libakgl` immediate-mode draw API — file it** |
|
|||
|
|
| H. Sprites | `SPRITE`, `MOVSPR`, `SPRCOLOR`, `SPRDEF`, `SPRSAV`, `COLLISION` | `libakgl` sprite/actor API — check before filing |
|
|||
|
|
| I. Audio | `PLAY`, `SOUND`, `ENVELOPE`, `FILTER`, `VOL`, `TEMPO` | **`libakgl` has no mixer-backed audio API — file it** |
|
|||
|
|
| J. Machine | `SYS`, `FETCH`, `STASH`, `POKE`/`PEEK` variants | nothing |
|
|||
|
|
|
|||
|
|
**Out of scope, and staying that way** — the reference marks these as incompatible with a
|
|||
|
|
modern PC and that reasoning stands: `BANK` (no bank switching), `FAST` (irrelevant CPU speed
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 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.
|
|||
|
|
|
|||
|
|
1. Reflection → static dispatch table (§1.1).
|
|||
|
|
2. Go maps → fixed open-addressed tables (§1.3).
|
|||
|
|
3. Unbounded `new(BasicEnvironment)` → pool with release (§1.4).
|
|||
|
|
4. Hardcoded stdout+SDL mirror → text sink backend (§1.5).
|
|||
|
|
5. `run()` owns the process → `step()` / bounded `run()` (§1.6).
|
|||
|
|
6. `panic()` → `FAIL_RETURN`; no library call terminates the process (§1.6).
|
|||
|
|
7. `debug.PrintStack()` on parse error → the `akerr` stack trace only (§3.1 of the original plan).
|
|||
|
|
8. `BasicEnvironment.eval_clone_identifiers` deleted as dead state (§4.2 of the original plan).
|
|||
|
|
9. `BasicValue.name` and `BasicEnvironment.update()` deleted as dead: nothing ever writes the
|
|||
|
|
field a non-empty string, and `update()` — its only reader — has no callers.
|
|||
|
|
10. The `DEF`-statement bootstrap is gone. The reference declares every builtin by *running a
|
|||
|
|
BASIC program of `DEF` lines through the interpreter* at startup and then nulling out the
|
|||
|
|
expressions (`basicruntime_functions.go:14`); here a builtin's name, arity and handler are
|
|||
|
|
a row in the dispatch table, and `MOD`, `SPC` and `STR` are ordinary native handlers. This
|
|||
|
|
removes the need to run the interpreter before the interpreter is ready.
|
|||
|
|
11. Undefined behaviour the reference reaches by a defined route is refused rather than
|
|||
|
|
inherited: a shift count outside 0..63, a negative string multiplier, and integer division
|
|||
|
|
by zero all raise. Go defines all three (or panics); C does not. No golden case exercises
|
|||
|
|
any of them, so observable behaviour is unchanged.
|
|||
|
|
12. `CommandEXIT` clears the pending `NEXT` wait before popping. The reference does not, which
|
|||
|
|
leaves the parent waiting for a `NEXT` that never arrives (§6 item 8) — in C that is a hang
|
|||
|
|
rather than a misbehaviour, and no golden case depends on it.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 6. Reference defects — ported faithfully, fix them deliberately
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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
|
|||
|
|
it returns the "not implemented for arrays" string. *Consequence:* `PRINT` of a scalar
|
|||
|
|
through this path is wrong. Fix: `if ( count == 1 )`.
|
|||
|
|
2. **`basicvariable.go:108`** — `setBoolean` builds a value with `valuetype: TYPE_STRING`.
|
|||
|
|
*Consequence:* a boolean stored in a variable reads back as a string with an empty
|
|||
|
|
`stringval`. Fix: `TYPE_BOOLEAN`.
|
|||
|
|
3. **`basicenvironment.go:157`** — `stopWaiting(command)` ignores `command` and clears the
|
|||
|
|
wait unconditionally. *Consequence:* an inner block can clear an outer block's wait. Fix:
|
|||
|
|
compare before clearing, and error on a mismatch.
|
|||
|
|
4. **`basicvalue.go:181`** — `mathPlus` mutates `self` in place when `self.mutable` is true,
|
|||
|
|
while every other operator always clones. *Consequence:* `A# + 1` can modify `A#` depending
|
|||
|
|
on where the value came from. This one is high blast radius; it interacts with
|
|||
|
|
`eval_clone_identifiers` and with `CommandNEXT`'s increment
|
|||
|
|
(`basicruntime_commands.go:663`), which **relies** on the mutation. Do not fix it before
|
|||
|
|
`FOR`/`NEXT` has full test coverage.
|
|||
|
|
5. **`basicvalue.go:191` and siblings** — every binary operator adds both of the right-hand
|
|||
|
|
operand's numeric fields: `rval.intval + int64(rval.floatval)`. Works only because the
|
|||
|
|
unused field is always zero. *Consequence:* none today; it is a landmine for any future
|
|||
|
|
value that carries both.
|
|||
|
|
6. **`basicvalue.go`, all comparisons** — `dest` is cloned from `self`, so the resulting
|
|||
|
|
boolean inherits `self.name`. *Consequence:* a comparison result can be written back to the
|
|||
|
|
wrong variable through `environment.update()`.
|
|||
|
|
7. **`basicruntime_commands.go:484`** — `CommandIF` walks `expr.right` to the end of the chain
|
|||
|
|
looking for a `THEN` command, then dereferences `expr.right` after the loop guaranteed it
|
|||
|
|
is `nil`. *Consequence:* the "Malformed IF statement" check is unreachable and nested
|
|||
|
|
`IF ... THEN ... ELSE` is unverified.
|
|||
|
|
8. **`basicruntime_commands.go:669`** — `CommandEXIT` pops the environment without
|
|||
|
|
`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`.
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
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`.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 7. Filing gaps against `libakgl`
|
|||
|
|
|
|||
|
|
When phase 7 or phase 8 needs something `libakgl` does not have, **stop and file it** in
|
|||
|
|
`deps/libakgl/TODO.md` in that file's numbered prose style: what the BASIC verb requires, what
|
|||
|
|
the `akgl_*` entry point should look like, and what tests would cover it. Growing `libakgl` to
|
|||
|
|
serve the interpreter is a wanted outcome, not a detour.
|
|||
|
|
|
|||
|
|
Known gaps, to be filed as they are reached:
|
|||
|
|
|
|||
|
|
1. **Text measurement.** No `akgl_text_*` call returns the pixel size of a string in a font.
|
|||
|
|
`PRINT` wrapping, the cursor, and `WIDTH` all need it. Blocks 7.1.
|
|||
|
|
2. **Immediate-mode drawing.** `src/draw.c` is at 0% coverage and there is no public API for
|
|||
|
|
lines, boxes, circles or flood fill. Blocks group G.
|
|||
|
|
3. **Audio.** There is no mixer-backed API at all. Blocks group I entirely.
|
|||
|
|
4. **Console input.** `GET`/`GETKEY` need a non-blocking keystroke read that does not require
|
|||
|
|
the interpreter to own the SDL event loop. Blocks part of group E.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## 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.
|
|||
|
|
|
|||
|
|
| Gate | Result |
|
|||
|
|
|---|---|
|
|||
|
|
| `ctest` | 59/59 — 41 golden cases, 17 unit tests, 1 known-failing |
|
|||
|
|
| Golden corpus | 41/41 byte-exact, driven in place from the submodule |
|
|||
|
|
| ASan + UBSan | 59/59 |
|
|||
|
|
| Line coverage | 92.3% (2823/3058) — above the 90% gate |
|
|||
|
|
| Function coverage | 96.9% (221/228) |
|
|||
|
|
| Warnings | none under `-Wall -Wextra` |
|
|||
|
|
|
|||
|
|
Branch coverage reads 17.3% and is not a target, for the reason `libakgl/TODO.md` and
|
|||
|
|
`libakstdlib/TODO.md` both give: the akerror control-flow macros expand into large branch trees
|
|||
|
|
at every call site, most of them unreachable in normal operation. Track line and function
|
|||
|
|
coverage.
|
|||
|
|
|
|||
|
|
Dependency baseline:
|
|||
|
|
|
|||
|
|
| Submodule | Version | Notes |
|
|||
|
|
|---|---|---|
|
|||
|
|
| `deps/libakerror` | 1.0.0 | Private ownership-enforced status registry. akbasic reserves 512–767 in `akbasic_error_register()`. Does not namespace its `coverage` target when embedded — worked around in our `CMakeLists.txt`. |
|
|||
|
|
| `deps/libakstdlib` | 0.1.0 | soname `libakstdlib.so.0.1`. `AKSL_VERSION_CHECK()` asserted in `tests/version_check.c`. Its `aksl_ato*` family is banned here — see §1.9. |
|
|||
|
|
| `deps/libakgl` | 0.1.0 | Migrated to the 1.0.0 registry and owns 256–260. Not yet linked: `AKBASIC_WITH_AKGL` defaults OFF and `src/sink_akgl.c` does not exist. |
|
|||
|
|
|
|||
|
|
What remains, in priority order:
|
|||
|
|
|
|||
|
|
1. **§3 — the akgl text sink.** Blocked on `libakgl` having no text-measurement call; that gap
|
|||
|
|
needs filing in `deps/libakgl/TODO.md` before any workaround is written.
|
|||
|
|
2. **§4 — the language completion work queue.** Groups A, B, D, F and J need nothing from
|
|||
|
|
`libakgl` and can start immediately. Multiple statements per line (the `COLON` token exists
|
|||
|
|
and nothing consumes it) should come first, because `DO`/`LOOP` reads badly without it.
|
|||
|
|
3. **§6 items 12–16** — 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.
|