Delete src/convert.c: libakstdlib 0.2.0 grew the contract it existed for
Some checks failed
akbasic CI Build / cmake_build (push) Successful in 2m52s
akbasic CI Build / sanitizers (push) Successful in 3m9s
akbasic CI Build / coverage (push) Failing after 3m12s
akbasic CI Build / akgl_build (push) Failing after 20s
akbasic CI Build / mutation_test (push) Successful in 7m27s

That file wrapped strtoll and strtod because libakstdlib's aksl_ato* family
could not report a conversion failure at all -- atoi("not a number") returned
success with 0, turning four diagnosable errors into wrong answers. Its own note
said what to do when that changed: "When it grows it, delete src/convert.c and
switch the call sites over." 0.2.0 grew it, so it is gone.

Six call sites go straight to the library now: both literal constructors in
src/grammar.c, the line number in src/scanner.c, FunctionVAL in
src/runtime_functions.c, INPUT in src/runtime_commands.c, and GSHAPE's handle in
src/runtime_graphics.c. aksl_strtoll(str, NULL, base, &dest) rather than
aksl_atoll wherever a base is involved, because the ato* forms are base 10 and
grammar.c picks base 8 or 16 off the lexeme's prefix; the NULL endptr is what
makes trailing junk an error rather than a stopping point.

The raised status changed from AKBASIC_ERR_VALUE to AKERR_VALUE, and the message
with it -- VAL("garbage") now says `no digits in "garbage"`. Section 1.8 makes
message text part of the acceptance contract, so that was checked against the
corpus before touching anything: no golden file contained a conversion message,
which is also why section 1.9 had been asking for one. Two exist now, so the next
change to that text has to move a golden file, and one of them pins the
reference's octal-literal defect (section 6 item 10) while it is still
deliberately reproduced.

tests/convert.c became tests/numeric_contract.c rather than being deleted with
the code. The assertions did not stop being worth making when the wrapper went
away -- they became assertions about a contract this port depends on and no
longer owns, and a regression in it would make four things quietly return zero.

Repoints the CI mutation job, which was bounded to src/convert.c and
src/symtab.c. src/symtab.c alone measures 74.1% against the gate of 65.
src/audio_tables.c was measured as a replacement second file and scored 64.7%:
that is a real gap rather than a reason to skip it, since almost every survivor
is in akbasic_audio_state_init, where nothing asserts a freshly initialised audio
state is actually zeroed -- the same gap this job's history records closing for
symtab. Recorded in TODO.md so the file can earn its place back.

One thing worth knowing before reading any score here, and now written down: the
harness's ICR operator only rewrites the constants 0 and 1, so a lookup table of
other values produces no mutants and a high score over one says nothing about
whether its entries are right. tests/audio_verbs.c now asserts all 32 ADSR
entries against the datasheet anyway, because a hand-edit typo is the real
failure mode there -- but that could not and did not move the mutation number,
and claiming otherwise would be the kind of thing this file exists to stop.

72/72 core, 73/73 with libakgl, 72/72 under ASan+UBSan, coverage 93.6% line and
97.8% function, clean under -Wall -Wextra, doxygen clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-07-31 10:28:04 -04:00
parent c70f8e7727
commit ecdc6b60ef
19 changed files with 245 additions and 248 deletions

67
TODO.md
View File

@@ -286,21 +286,30 @@ success — so a `DSAVE` onto a full disk is now an error a program sees, where
reported nothing. `src/runtime_commands.c` passes the count and discards it, which is correct:
the library does the noticing now.
**`src/convert.c` is the outstanding consequence, and it is deliberately not done yet.** It
exists only because `aksl_ato*` could not report a conversion failure, and its own note says
what to do when that changed: *"When it grows it, delete `src/convert.c` and switch the call
sites over."* That condition is now met. Doing it touches more than the two functions:
**`src/convert.c` is gone.** It existed only because `aksl_ato*` could not report a conversion
failure, and its own note said what to do when that changed: *"When it grows it, delete
`src/convert.c` and switch the call sites over."* That is done. Six call sites now go straight
to the library — `newLiteralInt` and `newLiteralFloat` in `src/grammar.c`, the line number in
`src/scanner.c`, `FunctionVAL` in `src/runtime_functions.c`, `INPUT` in
`src/runtime_commands.c`, and `GSHAPE`'s handle in `src/runtime_graphics.c`.
- Four call sites — `newLiteralInt`/`newLiteralFloat` in `src/grammar.c`, `matchNumber` in
`src/scanner.c`, and `FunctionVAL` in `src/runtime_functions.c`.
- `tests/convert.c`, which would move to asserting `libakstdlib`'s contract rather than ours.
- **`.gitea/workflows/ci.yaml`'s mutation job, which is bounded to `src/convert.c` and
`src/symtab.c`.** Deleting the file without repointing that job silently removes half of the
only gate that checks the error-handling control flow at all.
- The error *status* changes from `AKBASIC_ERR_VALUE` to `AKERR_VALUE`, which is visible in
the message text a program sees — so §1.8 applies and the golden files have to be checked.
Three things that fell out of it, recorded because each was checked rather than assumed:
Worth doing, and worth doing on purpose rather than as a side effect of a version bump.
- **`aksl_strtoll(str, NULL, base, &dest)` is the exact replacement**, not `aksl_atoll`, wherever
a base is involved. The `ato*` forms are base 10; `src/grammar.c` picks base 8 or 16 from the
lexeme's prefix. A `NULL` endptr is what makes trailing junk an error rather than a stopping
point, and that is the whole contract this port needs.
- **The raised status changed** from `AKBASIC_ERR_VALUE` to `AKERR_VALUE`, and the message text
with it — `VAL("garbage")` now says `no digits in "garbage"`. §1.8 makes message text part of
the acceptance contract, so this was checked against the corpus first: **no golden file
contained a conversion message**, which is also why §1.9 used to ask for one. Two now exist,
`tests/language/numeric/val_refuses_garbage` and `octal_literal`, so the next change to that
text has to move a golden file.
- **`tests/convert.c` became `tests/numeric_contract.c`.** The assertions did not become
worthless when the wrapper went away — they became assertions about a contract this port
depends on and no longer owns, which is worth pinning exactly because a regression in it would
make `VAL("garbage")` return 0 silently. Same reasoning `libakstdlib`'s own
`tests/test_status_registry.c` gives for pinning that it reserves no status range.
One caveat survives the upgrade unchanged: `aksl_strhash_djb2` still sign-extends `char`, so a
high-bit byte hashes differently from the `unsigned char` answer. BASIC identifiers are 7-bit
@@ -322,7 +331,6 @@ 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` |
@@ -790,15 +798,15 @@ entire test corpus and passes clean under ASan and UBSan.
| Gate | Result |
|---|---|
| `ctest` | 70/70 — 41 upstream golden cases, 5 local ones, 21 unit tests, 2 embedding examples, 1 known-failing |
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 71/71 — the above plus `akgl_backends`; the `akgl_build` CI job |
| `ctest` | 72/72 — 41 upstream golden cases, 5 local ones, 21 unit tests, 2 embedding examples, 1 known-failing |
| `ctest` with `-DAKBASIC_WITH_AKGL=ON` | 73/73 — the above plus `akgl_backends`; the `akgl_build` CI job |
| Golden corpus | 41/41 byte-exact, driven in place from the submodule |
| ASan + UBSan | 70/70 |
| Line coverage | 93.5% (3631/3884) — above the 90% gate |
| Function coverage | 97.8% (269/275) |
| ASan + UBSan | 72/72 |
| Line coverage | 93.6% (3618/3867) — above the 90% gate |
| Function coverage | 97.8% (267/273) |
| Warnings | none under `-Wall -Wextra` |
| `doxygen Doxyfile` | clean |
| Mutation (`src/convert.c`, `src/symtab.c`) | see `.gitea/workflows/ci.yaml` for the current score and gate |
| Mutation (`src/symtab.c`) | 74.1%, against a gate of 65 — see `.gitea/workflows/ci.yaml` |
Coverage of the verb groups added for goal 2, since they are the new surface: `src/audio_tables.c`
and `src/graphics_tables.c` 100%, `src/runtime_input.c` 100%, `src/runtime_graphics.c` and
@@ -848,8 +856,19 @@ keeping:
`AKBASIC_WITH_AKGL` and defaults OFF — and recursing into it would clone SDL, SDL_image,
SDL_mixer, SDL_ttf and jansson for a target that is never configured. The `docs` job needs
no submodules at all.
- The push-path `mutation_test` job is bounded to `src/convert.c` and `src/symtab.c`, about
four minutes between them, scoring 77.8% against a gate of 65. `src/value.c` is the file most
- The push-path `mutation_test` job is bounded to `src/symtab.c`, scoring 74.1% against a gate
of 65. It was two files until `src/convert.c` was deleted. `src/audio_tables.c` was measured
as a replacement and scored 64.7%, below the gate — a real gap rather than a reason to skip
the file: almost every survivor is in `akbasic_audio_state_init`, where nothing asserts that
a freshly initialised audio state is actually zeroed and defaulted. That is the same gap this
job's history records closing for `src/symtab.c`, and closing it is what earns the file its
place back in CI.
Worth knowing before reading a score: the harness's `ICR` operator only rewrites the
constants `0` and `1`, so a lookup table of other values produces no mutants at all and a
high score over one says nothing about whether its entries are right. `tests/audio_verbs.c`
asserts all 32 ADSR entries anyway — that catches a hand-edit typo, which is the real failure
mode, but it did not and could not move the mutation number. `src/value.c` is the file most
worth mutating and is deliberately not on that path: 368 mutants at roughly 11 seconds each
is about 70 minutes, because almost everything links against it. It is covered by
`release.yaml`, or locally with `cmake --build build --target mutation`.
@@ -887,7 +906,7 @@ What remains, in priority order:
4. **§6 items 1217** — the defects the port uncovered. Item 12 is the one that produces a
*wrong answer* rather than a refused one, and should be fixed first.
5. **Mutation survivors in `src/value.c`.** The harness is in place (`scripts/mutation_test.py`,
the `mutation` CMake target, and a CI job on `src/convert.c`), and a partial run over
the `mutation` CMake target, and a CI job on `src/symtab.c`), and a partial run over
`src/value.c` turned up test gaps worth closing. These are *not* equivalent mutants; each
is a real bug of that shape the suite would not notice:
@@ -906,5 +925,5 @@ What remains, in priority order:
argument for doing it.
`src/value.c` takes about 70 minutes to mutate in full (368 mutants, ~11s each, because
almost everything links against it), which is why CI runs `src/convert.c` instead and the
almost everything links against it), which is why CI runs `src/symtab.c` instead and the
whole-tree run is a local `cmake --build build --target mutation`.