Port onto libakstdlib 2b79aca and convert the eight bool predicates (libakstdlib#26) #31

Open

tachikoma wants to merge 3 commits from libakstdlib-26 into main

pull from: libakstdlib-26

merge into: andrew:main

andrew:main

andrew:23

andrew:todo-to-issues

Conversation 1 Commits 3 Files Changed 42 +1284 -593

tachikoma commented 2026-08-03 15:45:50 -04:00

Collaborator

Copy Link

Ports src/ onto libakstdlib 2b79aca and converts the eight sites the earlier port left on raw libc.

This is the akbasic half of libakstdlib#26, and it carries @andrew's instruction on libakstdlib#38 (comment 1105) — "fix the akbasic port branch and reference this discussion". The libakstdlib half is libakstdlib#39.

	libakstdlib	raw libc	bypassed
Before — main at 330d731	45	285	86.4%
After — same tree	313	7	2.2%

Same tree in both rows, so the rate is like-for-like. deps/libakstdlib bumped 669b2b3 → 2b79aca; the bump itself needed no source change — the release is drop-in for what akbasic already used.

The ruling this implements

libakstdlib#38 was filed claiming a bool predicate had no way to call an AKERR_NOIGNORE function. @andrew ruled it invalid (comment 1095):

If you're writing a function that returns bool, and you're performing any kind of operation that could result in an error condition you can't handle internally, then you have no choice but to terminate the program. If you don't like that, then you should change the function to return the error context out to something that can handle it, and provide the bool as an out parameter.

That's the correct fix here. We're not changing libakstdlib or libakerror to support swallowing error codes.

So the signatures change. Seven of the eight now return akerr_ErrorContext * and hand the answer back through an out parameter — the shape symtab.c's probe() and runtime_structure.c's loop_continues() already used.

#	site	now	what came with it
1	structtype.c word_is	(w, k, bool *dest)	7 callers in-file; the two inside FAIL_ZERO_RETURN split into PASS + FAIL_ZERO_RETURN(errctx, ok, ...)
2	environment.c akbasic_environment_is_waiting_for	(env, cmd, bool *dest)	public header change, include/akbasic/environment.h. Its twin _is_waiting_for_any moves with it or the pair is inconsistent. runtime.c's && term hoisted out of its condition; 6 assertions in tests/environment_scope.c
3	scanner.c is_at_end	(rt, bool *dest)	the strlen became aksl_strlen
4	scanner.c peek_next	(rt, char *dest, bool *got)	the existing char *dest stays; the bool moves to a second out-param
5	verbs.c verb_compare	unchanged — see below
6	format.c overflow	akerr_ErrorContext *(dest, width)	2 callers, both already in render_numeric; one PASS each
7	sink_akgl.c scroll	akerr_ErrorContext *
8	sink_akgl.c newline	akerr_ErrorContext *

Four more functions were not in the original eight but sit on the same chains and could not stay behind: scanner.c peek and match_next_char, and sink_akgl.c putchar_at, echo_line and edit_key. Every call site that used one of these as a term in an if or a loop condition is now a hoist into the body.

The eighth stays, deliberately

verbs.c:200 verb_compare remains on strcmp. bsearch(3) fixes the comparator's signature at int (*)(const void *, const void *), so there is no out parameter to report an error through — which is exactly what libakstdlib#38 concluded. Converting it means dropping bsearch for an in-house binary search, on a lookup that runs once per scanned identifier. The site now carries a comment saying that, and saying why the bypass is safe there: both arguments are a NUL-terminated local and a string literal in the static table, so neither can be NULL or unterminated.

libakstdlib#38 cites this as tracked in akbasic #14. That is a mis-citation — #14 is the COLLISION/BUMP pairing threshold. Nothing currently tracks the comparator; worth filing before anybody acts on it.

What is left, and why

7 bypasses, down from a projected 13. Six are snprintf sites that want truncation as an answer rather than an error, which aksl_snprintf cannot express — it raises AKERR_OUTOFBOUNDS and zeroes its count. Tracked as libakstdlib#34. Each site says so inline.

The sharpest is runtime.c:396 akbasic_runtime_error, which formats a 12,384-byte error message into a 512-byte line. A report may be shortened; it may not be replaced by a second, different failure — that function is the one that tells the user what went wrong. runtime_struct.c:216 is the other interesting one: it reads snprintf's return value to detect truncation, which is what makes the field loop and the closing ) both skip.

Two of the three in host.c are a latent defect rather than a decision, and are flagged rather than fixed. A host-registered type name over 31 characters truncates silently, and two names sharing a 31-character prefix then collide in akbasic_structtype_find — where structtype.c's scan_names() refuses the identical case outright with a limit message. The script-declared and host-registered paths disagree. Fixing it is a behaviour change on a public registration call and wants its own issue.

A defect this turned up rather than caused

DLOAD leaked a file descriptor. Its read loop sat inside an ATTEMPT block, and the PASS in it returned past CLEANUP — so a scan error left the file open. The comment there defended PASS over CATCH and missed that both are illegal in a loop inside an ATTEMPT. Converting fgets required hoisting the loop into its own helper, which fixes it. DSAVE and DVERIFY had the same shape and got the same treatment.

Verification

• Default build clean at -Wall -Wextra; 112/112 ctest.
• ASan + UBSan clean, 112/112 (-DAKBASIC_SANITIZE=ON).
• The five *_akgl.c files could not be built here — SDL3 needs X11/Wayland dev packages this box does not have. Unlike the previous attempt they are not unverified: all five are gcc -fsyntax-only -Wall -Wextra clean under -DAKBASIC_WITH_AKGL=1 against stubbed generated headers. That checks syntax and types, not behaviour, so the AKGL configuration still wants a real CI build — sink_akgl.c carries the deepest signature change in the diff and none of it has executed.

Semantic changes worth a reviewer's eye

aksl_strcpy refuses rather than truncates, and aksl_atoi fails on trailing junk where raw atoi returned 0. Both surfaced places where the old silent behaviour was already broken:

• runtime_ui.c and runtime_input.c copied a 256-byte identifier into a 64-byte buffer. The truncated name then failed the later lookup with "could not reach the variable"; it now refuses early with a name-too-long error instead.
• SPRSAV on a hand-built "SHAPE:x" string used to yield handle 0 silently.

Neither is reachable from any in-tree BASIC program, and the suite is green, but both are behaviour changes rather than refactors.

---

Closes nothing on its own — libakstdlib#26 is closed by libakstdlib#39, which carries the recount this branch produced.

Filed by Tachikoma (Claude Code, Opus 5, 1M context)

Ports `src/` onto libakstdlib `2b79aca` and converts the eight sites the earlier port left on raw libc. This is the akbasic half of **libakstdlib#26**, and it carries **@andrew's instruction on libakstdlib#38 (comment 1105)** — "fix the akbasic port branch and reference this discussion". The libakstdlib half is libakstdlib#39. | | libakstdlib | raw libc | bypassed | |---|---|---|---| | Before — `main` at `330d731` | 45 | 285 | **86.4%** | | **After — same tree** | **313** | **7** | **2.2%** | Same tree in both rows, so the rate is like-for-like. `deps/libakstdlib` bumped `669b2b3` → `2b79aca`; **the bump itself needed no source change** — the release is drop-in for what akbasic already used. ## The ruling this implements libakstdlib#38 was filed claiming a `bool` predicate had no way to call an `AKERR_NOIGNORE` function. @andrew ruled it invalid ([comment 1095](https://source.starfort.tech/andrew/libakstdlib/issues/38#issuecomment-1095)): > If you're writing a function that returns `bool`, and you're performing any kind of operation that could result in an error condition you can't handle internally, then you have no choice but to terminate the program. If you don't like that, then you should change the function to return the error context out to something that can handle it, and provide the bool as an out parameter. > > That's the correct fix here. We're not changing libakstdlib or libakerror to support swallowing error codes. So the signatures change. **Seven of the eight** now return `akerr_ErrorContext *` and hand the answer back through an out parameter — the shape `symtab.c`'s `probe()` and `runtime_structure.c`'s `loop_continues()` already used. | # | site | now | what came with it | |---|---|---|---| | 1 | `structtype.c` `word_is` | `(w, k, bool *dest)` | 7 callers in-file; the two inside `FAIL_ZERO_RETURN` split into `PASS` + `FAIL_ZERO_RETURN(errctx, ok, ...)` | | 2 | `environment.c` `akbasic_environment_is_waiting_for` | `(env, cmd, bool *dest)` | **public header change**, `include/akbasic/environment.h`. Its twin `_is_waiting_for_any` moves with it or the pair is inconsistent. `runtime.c`'s `&&` term hoisted out of its condition; 6 assertions in `tests/environment_scope.c` | | 3 | `scanner.c` `is_at_end` | `(rt, bool *dest)` | the `strlen` became `aksl_strlen` | | 4 | `scanner.c` `peek_next` | `(rt, char *dest, bool *got)` | the existing `char *dest` stays; the `bool` moves to a second out-param | | 5 | `verbs.c` `verb_compare` | **unchanged — see below** | | | 6 | `format.c` `overflow` | `akerr_ErrorContext *(dest, width)` | 2 callers, both already in `render_numeric`; one `PASS` each | | 7 | `sink_akgl.c` `scroll` | `akerr_ErrorContext *` | | | 8 | `sink_akgl.c` `newline` | `akerr_ErrorContext *` | | Four more functions were not in the original eight but sit on the same chains and could not stay behind: **`scanner.c` `peek` and `match_next_char`**, and **`sink_akgl.c` `putchar_at`, `echo_line` and `edit_key`**. Every call site that used one of these as a term in an `if` or a loop condition is now a hoist into the body. ## The eighth stays, deliberately `verbs.c:200` `verb_compare` remains on `strcmp`. `bsearch(3)` fixes the comparator's signature at `int (*)(const void *, const void *)`, so there is no out parameter to report an error through — which is exactly what libakstdlib#38 concluded. Converting it means dropping `bsearch` for an in-house binary search, on a lookup that runs once per scanned identifier. The site now carries a comment saying that, and saying why the bypass is safe there: both arguments are a NUL-terminated local and a string literal in the static table, so neither can be NULL or unterminated. **libakstdlib#38 cites this as tracked in akbasic #14. That is a mis-citation** — #14 is the `COLLISION`/`BUMP` pairing threshold. Nothing currently tracks the comparator; worth filing before anybody acts on it. ## What is left, and why 7 bypasses, down from a projected 13. Six are `snprintf` sites that want **truncation as an answer rather than an error**, which `aksl_snprintf` cannot express — it raises `AKERR_OUTOFBOUNDS` and zeroes its `count`. Tracked as libakstdlib#34. Each site says so inline. The sharpest is `runtime.c:396` `akbasic_runtime_error`, which formats a 12,384-byte error message into a 512-byte line. **A report may be shortened; it may not be replaced by a second, different failure** — that function is the one that tells the user what went wrong. `runtime_struct.c:216` is the other interesting one: it reads `snprintf`'s return value to *detect* truncation, which is what makes the field loop and the closing `)` both skip. **Two of the three in `host.c` are a latent defect rather than a decision, and are flagged rather than fixed.** A host-registered type name over 31 characters truncates silently, and two names sharing a 31-character prefix then collide in `akbasic_structtype_find` — where `structtype.c`'s `scan_names()` refuses the identical case outright with a limit message. The script-declared and host-registered paths disagree. Fixing it is a behaviour change on a public registration call and wants its own issue. ## A defect this turned up rather than caused **`DLOAD` leaked a file descriptor.** Its read loop sat inside an `ATTEMPT` block, and the `PASS` in it returned past `CLEANUP` — so a scan error left the file open. The comment there defended `PASS` over `CATCH` and missed that both are illegal in a loop inside an `ATTEMPT`. Converting `fgets` required hoisting the loop into its own helper, which fixes it. `DSAVE` and `DVERIFY` had the same shape and got the same treatment. ## Verification - Default build clean at `-Wall -Wextra`; **112/112** ctest. - **ASan + UBSan clean**, 112/112 (`-DAKBASIC_SANITIZE=ON`). - **The five `*_akgl.c` files could not be built here** — SDL3 needs X11/Wayland dev packages this box does not have. Unlike the previous attempt they are **not unverified**: all five are `gcc -fsyntax-only -Wall -Wextra` clean under `-DAKBASIC_WITH_AKGL=1` against stubbed generated headers. That checks syntax and types, not behaviour, so **the AKGL configuration still wants a real CI build** — `sink_akgl.c` carries the deepest signature change in the diff and none of it has executed. ## Semantic changes worth a reviewer's eye `aksl_strcpy` refuses rather than truncates, and `aksl_atoi` fails on trailing junk where raw `atoi` returned 0. Both surfaced places where the old silent behaviour was already broken: - `runtime_ui.c` and `runtime_input.c` copied a 256-byte identifier into a 64-byte buffer. The truncated name then failed the later lookup with "could not reach the variable"; it now refuses early with a name-too-long error instead. - `SPRSAV` on a hand-built `"SHAPE:x"` string used to yield handle 0 silently. Neither is reachable from any in-tree BASIC program, and the suite is green, but both are behaviour changes rather than refactors. --- Closes nothing on its own — libakstdlib#26 is closed by libakstdlib#39, which carries the recount this branch produced. Filed by Tachikoma (Claude Code, Opus 5, 1M context)

tachikoma added 1 commit 2026-08-03 15:45:51 -04:00

Port onto libakstdlib 2b79aca and convert the eight bool predicates ... d219f80777

akbasic's src/ now calls libakstdlib 313 times and raw libc 7 -- 2.2%
bypassed, against 86.4% on the same tree before this. The submodule bump
669b2b3 -> 2b79aca needed no source change of its own: the release is
drop-in for what akbasic already used.

Seven of the eight sites the earlier port left on raw libc change their own
signature rather than swallowing an error, per andrew's ruling on
libakstdlib#38. word_is, the is_waiting_for pair, the scanner's is_at_end,
peek, peek_next and match_next_char, format.c's overflow, and sink_akgl's
scroll/newline/putchar_at/echo_line/edit_key chain all return an
akerr_ErrorContext * and hand the answer back through an out parameter.
is_waiting_for and is_waiting_for_any are a public header change; every
call site that used one as a term in a condition hoists it into a
statement first.

verb_compare is the eighth and stays on strcmp. bsearch(3) fixes the
comparator's signature, so there is no out parameter to report through --
which is what libakstdlib#38 concluded. It carries a comment saying so and
saying why the bypass is safe there.

Six snprintf sites stay raw because they want truncation as an answer
rather than an error, and aksl_snprintf cannot express that until
libakstdlib#34 hands the required length back. Each of the six says so at
the site. Two of them, in host.c, are a latent defect rather than a
decision: a host type name over 31 characters truncates silently and two
sharing a prefix then collide, where structtype.c refuses the same case.

DLOAD leaked a file descriptor. Its read loop sat inside an ATTEMPT and the
PASS in it returned past CLEANUP, so a scan error left the file open.
Hoisting the loop into its own helper to convert fgets fixes it.

Refs libakstdlib#26, libakstdlib#38

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

tachikoma requested review from andrew 2026-08-03 15:46:00 -04:00

andrew was assigned by tachikoma 2026-08-03 15:46:00 -04:00

tachikoma self-assigned this 2026-08-03 15:46:01 -04:00

tachikoma referenced this pull request from andrew/libakstdlib 2026-08-03 15:46:37 -04:00

Recount the consumer calls against this release (#26) #39

tachikoma referenced this pull request from andrew/libakstdlib 2026-08-03 15:47:05 -04:00

Port akbasic onto this release and count the calls again #26

andrew commented 2026-08-03 15:52:51 -04:00

Owner

Copy Link

@logikoma review this merge request and make comments please

@logikoma review this merge request and make comments please

tachikoma added 1 commit 2026-08-03 16:17:50 -04:00

Install the build dependencies CI has been missing since run #2 ... 88ac2fc3f2

Every job in ci.yaml has failed on every run since #2, on main and on
every branch, for two reasons that have nothing to do with any of the
code under test.

moreutils, for errno(1). deps/libakerror/scripts/generrno.sh shells out
to it to generate its errno name table and to stamp
AKERR_LAST_ERRNO_VALUE into the generated akerror.h. The runner does not
have it, and its absence does not fail the build -- which is what made
this expensive to find. The script prints "errno: command not found",
emits an empty akerr_init_errno() so no errno ever gets a name, and
substitutes an empty AKERR_LAST_ERRNO_VALUE, so every code in
libakerror's reserved band collapses from (134 + n) to ( + n), i.e. to n.
AKERR_OUTOFBOUNDS is (AKERR_LAST_ERRNO_VALUE + 2) and therefore becomes
literally 2, so ERR(2) answers "Out Of Bounds Error" where ENOENT's "No
such file or directory" belongs. That is the docs_examples failure at
docs/15-error-codes.md:105, and it took cmake_build, sanitizers and
coverage down with it; mutation_test then aborted on "baseline is not
green" because the same suite is its baseline.

Reproduced locally by shimming errno(1) out of PATH: identical failure,
same test #47, same two lines of diff. With errno present the suite is
112/112.

The nine X11 dev packages, for akgl_build, which has never once been
green. SDL_X11 and its sub-options default ON on Linux, and CheckX11()
calls SDL_missing_dependency() -- a hard CMake error, not a downgrade --
for any whose header is absent. The runner has libx11-dev but none of
the extension packages, so configuration stopped at the first of them:
"Couldn't find dependency package for XCURSOR". The list is SDL's own,
from deps/SDL/docs/README-linux.md, reduced to the X11 entries; the
audio, Wayland, KMSDRM and Vulkan backends all degrade to "not found"
and skip themselves rather than erroring, so they cost nothing to leave
out. Turning the missing options OFF would also configure and is the
wrong answer -- the point of that job is that an AKGL build is a real
SDL program. The tests still run headless under SDL_VIDEODRIVER=dummy.

release.yaml's full_mutation job builds the same tree and gets moreutils
for the same reason.

No source file changes. The port itself was never the problem.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

tachikoma added 1 commit 2026-08-03 16:32:30 -04:00

Clone libakgl's other two submodules, and pin upload-artifact to v3 ... 9e5496f6b2

Two more gaps that only became visible once the errno fix let the jobs
run far enough to hit them.

akgl_build cloned six of libakgl's submodules and needs eight. libakgl
does not add_subdirectory deps/libccd or deps/clay -- their own
CMakeLists are unusable as subprojects -- it compiles them into itself,
so nothing declares them and configuration dies at
deps/libakgl/CMakeLists.txt:282 with "File
deps/libccd/src/ccd/config.h.cmake.in does not exist". deps/tg stays out:
nothing references it.

The X11 packages from the previous commit did their job -- SDL now
configures the lot: "X11 libraries: xcursor xdbe xfixes xinput2 xrandr
xscrnsaver xshape xsync xtest".

coverage passes its gate now (112/112, lines 94.1% against a 90 floor)
and failed on the upload instead. actions/upload-artifact@v4 bundles
@actions/artifact v2, whose isGhes() treats any GITHUB_SERVER_URL that is
not github.com as GitHub Enterprise Server and refuses outright. @v3 uses
the older artifact API, which Gitea 1.25.3 does implement. The step had
never had a file to upload before -- gcovr was never reached, so it
warned "No files were found" and passed -- which is why this surfaced
only now. Same class of accommodation as annotate_only on the junit
reporter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Some checks failed

Hide all checks

akbasic CI Build / mutation_test (push) Waiting to run

Required

Details

akbasic CI Build / cmake_build (push) Successful in 3m30s

Required

Details

akbasic CI Build / coverage (push) Successful in 4m4s

Required

Details

akbasic CI Build / akgl_build (push) Successful in 7m58s

Required

Details

akbasic CI Build / sanitizers (push) Failing after 17m23s

Required

Details

This pull request doesn't have enough required approvals yet. 0 of 1 approvals granted from users or teams on the allowlist.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin libakstdlib-26:libakstdlib-26

git checkout libakstdlib-26

Sign in to join this conversation.

Reviewers

No Reviewers

andrew

Labels

Clear labels

abi-break

Cannot land without a soname bump

api-gap

A caller cannot express something

blast-radius:high

Ordering: highest blast radius

blast-radius:low

Ordering: lowest blast radius

blast-radius:medium

Ordering: middling blast radius

defect

Wrong behaviour reachable from the public API

design-decision

Needs a decision written down before code

docs

Documentation, comments, AGENTS.md

hygiene

Style, dead code, consistency

packaging

Build, install, pkg-config, CMake config

performance

Has a measurement behind it

status::blocked

Waiting on something else -- a decision, a dependency, or another issue

status::grooming

Not yet worked out: scope, approach or priority still being settled

status::ready

Understood well enough to start: scope and approach are settled

test-coverage

A gap in the suites, not a defect

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

logikoma tachikoma andrew

No Assignees andrew tachikoma

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: andrew/akbasic#31