Ruled invalid: a bool predicate changes its own signature; the eight sites are akbasic's to convert #38

Closed
opened 2026-08-03 12:52:03 -04:00 by tachikoma · 3 comments
Collaborator

Ruled on by @andrew (comment 1095): invalid as filed. Recommendation: close. The framing — "a bool predicate has no way to call an AKERR_NOIGNORE function" — was wrong. It has a way; it is to stop returning bool.

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.

This follows from libakerror's third design bullet (libakerror/README.md:38): "Uncaught errors should cause program termination with a stacktrace." A predicate that cannot report is not a gap in the wrapper library — it is a signature the consumer chose, and the consumer is the one who has to change it.

The shape is not hypothetical in akbasic; it is already the house style. akerr_ErrorContext * out, answer in an out-param, ~19 functions:

worked example what it answers
akbasic/src/symtab.c:22 probe(obj, key, int *slot, bool *found) is this key in the table? — and it PASSes the fallible aksl_strhash_djb2 straight through
akbasic/src/runtime_structure.c:46 loop_continues(obj, condition, kind, bool *dest) should this loop run again?
akbasic/src/audio_akgl.c:124 snd_voice_active(self, voice, bool *active) is this voice playing?
akbasic/src/runtime_commands.c:733 evaluate_for_condition(obj, counter, bool *met) has FOR reached its limit?

probe() is the direct analogue of word_is and is_at_end. So the eight sites below are not blocked on anything; they are eight signatures akbasic has not converted yet.

The eight sites, and what each becomes

Line numbers are akbasic main (330d731); the numbers in the original filing were from the port branch and are two to six lines further down.

# site today corrected
1 src/structtype.c:92 word_is static bool word_is(const char *w, const char *k) static akerr_ErrorContext *word_is(const char *w, const char *k, bool *dest) — body becomes PASS(errctx, aksl_strcmp(w, k, &cmp)); *dest = (cmp == 0);
2 src/environment.c:156 akbasic_environment_is_waiting_for bool f(env *, const char *) akerr_ErrorContext *f(env *, const char *, bool *dest)
3 src/scanner.c:29 is_at_end static bool is_at_end(rt *) static akerr_ErrorContext *is_at_end(rt *, bool *dest) — the strlen becomes aksl_strlen
4 src/scanner.c:43 peek_next static bool peek_next(rt *, char *dest) static akerr_ErrorContext *peek_next(rt *, char *dest, bool *got) — the existing char *dest stays, the bool moves to a second out-param
5 src/verbs.c:200 verb_compare static int verb_compare(const void *, const void *) no out-param is possible — see below
6 src/format.c:165 overflow static void overflow(char *dest, size_t width) static akerr_ErrorContext *overflow(char *dest, size_t width) — no bool involved, it just returns the context
7 src/sink_akgl.c:36 scroll static void scroll(sink *) static akerr_ErrorContext *scroll(sink *)
8 src/sink_akgl.c:62 newline static void newline(sink *) static akerr_ErrorContext *newline(sink *)

What each one drags with it

  1. word_is — 7 call sites in the same file (:234,236,245,312,357,361,363), all already inside functions returning akerr_ErrorContext *. Five are if ( word_is(...) ) and become PASS(errctx, word_is(w, k, &ok)); if ( ok ) {. Two (:357, :363) sit inside FAIL_ZERO_RETURN(errctx, word_is(word, "AS"), ...) and split into two statements — the PASS, then FAIL_ZERO_RETURN(errctx, ok, ...). Self-contained; nothing leaves the file.

  2. akbasic_environment_is_waiting_for — the only one that is an ABI break: declared at include/akbasic/environment.h:170. Its recursive tail call becomes a PASS. Its sibling akbasic_environment_is_waiting_for_any (src/environment.c:145, header :163) has the identical shape and has to change with it or the pair becomes inconsistent. Callers: src/runtime_commands.c:158, src/runtime.c:827,829,864:864 is a term inside an && chain and has to be hoisted out of the condition. Plus six assertions in tests/environment_scope.c:61-77, which currently call these directly inside TEST_REQUIRE.

3–4. The scanner cascadeis_at_end, peek (:34, not in the original eight but on the same chain and must go with them) and peek_next. Nine call sites: :105,131,132,159,160,162,235,236,336, all inside functions that already return akerr_ErrorContext *. Every one is a loop or if condition, so each becomes a hoist:

for ( ;; ) {
    PASS(errctx, is_at_end(obj, &done));
    if ( done ) { break; }
    ...
}

:160 and :236 are (void)peek(obj, &c) today and become PASS. This is the expensive one: it runs once per character of every line scanned, and adds an aksl_strlen call per character on top.

  1. verb_compare — the one site where the ruling's remedy is unavailable, because bsearch fixes the comparator's signature and libc will not carry an out-param. The two options are to leave it on strcmp (its arguments are the search key and a static table entry, neither of which the caller can make NULL) or to drop bsearch for an in-house binary search that can propagate. Already tracked as akbasic #14; that issue is where it should be decided, not here.

  2. overflow — cheapest of the eight. Both call sites (src/format.c:229,274) are already inside render_numeric, which returns akerr_ErrorContext *, so each is one PASS. No signature above it moves.

7–8. scroll / newline — a four-deep chain of void helpers: scrollnewline (:62) → putchar_at (:92) → echo_line (:154). All four have to return contexts before the chain reaches sink_write (:116), sink_writeln (:133) and sink_readline (:333), which already return them. Call sites: :67, :97,101,140,380, :128,163,170. AKGL-only — it does not build unless AKBASIC_WITH_AKGL=ON.

The ctype.h note

The original filing also asked whether isspace/isdigit/isalnum/toupper should have aksl_* forms. The same ruling answers it: they cannot fail, so there is nothing to return and nothing to terminate over. What is worth writing down is the (unsigned char) cast every correct ctype.h call needs, which is a precondition the caller must remember and which nothing in either library will remind them of. That belongs in akbasic's own notes, not as a request to libakstdlib.

Where this leaves things

  • This issue: close. Nothing in libakstdlib or libakerror changes.
  • The work is akbasic's, and it is now specified above: six signature changes that stay inside akbasic, one public-header change, one site (verb_compare) that has to be settled in akbasic #14 instead.
  • It also lands on the port. The akbasic port branch left these eight on raw libc on the assumption that the library owed them a form. It does not, so the port either carries the signature changes or carries an explicit note that these eight are deliberately unconverted and why. Worth deciding before that branch merges.

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

**Ruled on by @andrew (comment 1095): invalid as filed. Recommendation: close.** The framing — "a `bool` predicate has no way to call an `AKERR_NOIGNORE` function" — was wrong. It has a way; it is to stop returning `bool`. > 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. This follows from libakerror's third design bullet (`libakerror/README.md:38`): *"Uncaught errors should cause program termination with a stacktrace."* A predicate that cannot report is not a gap in the wrapper library — it is a signature the consumer chose, and the consumer is the one who has to change it. **The shape is not hypothetical in akbasic; it is already the house style.** `akerr_ErrorContext *` out, answer in an out-param, ~19 functions: | worked example | what it answers | |---|---| | `akbasic/src/symtab.c:22` `probe(obj, key, int *slot, bool *found)` | is this key in the table? — and it `PASS`es the fallible `aksl_strhash_djb2` straight through | | `akbasic/src/runtime_structure.c:46` `loop_continues(obj, condition, kind, bool *dest)` | should this loop run again? | | `akbasic/src/audio_akgl.c:124` `snd_voice_active(self, voice, bool *active)` | is this voice playing? | | `akbasic/src/runtime_commands.c:733` `evaluate_for_condition(obj, counter, bool *met)` | has `FOR` reached its limit? | `probe()` is the direct analogue of `word_is` and `is_at_end`. So the eight sites below are not blocked on anything; they are eight signatures akbasic has not converted yet. ## The eight sites, and what each becomes Line numbers are akbasic `main` (`330d731`); the numbers in the original filing were from the port branch and are two to six lines further down. | # | site | today | corrected | |---|---|---|---| | 1 | `src/structtype.c:92` `word_is` | `static bool word_is(const char *w, const char *k)` | `static akerr_ErrorContext *word_is(const char *w, const char *k, bool *dest)` — body becomes `PASS(errctx, aksl_strcmp(w, k, &cmp)); *dest = (cmp == 0);` | | 2 | `src/environment.c:156` `akbasic_environment_is_waiting_for` | `bool f(env *, const char *)` | `akerr_ErrorContext *f(env *, const char *, bool *dest)` | | 3 | `src/scanner.c:29` `is_at_end` | `static bool is_at_end(rt *)` | `static akerr_ErrorContext *is_at_end(rt *, bool *dest)` — the `strlen` becomes `aksl_strlen` | | 4 | `src/scanner.c:43` `peek_next` | `static bool peek_next(rt *, char *dest)` | `static akerr_ErrorContext *peek_next(rt *, char *dest, bool *got)` — the existing `char *dest` stays, the `bool` moves to a second out-param | | 5 | `src/verbs.c:200` `verb_compare` | `static int verb_compare(const void *, const void *)` | **no out-param is possible** — see below | | 6 | `src/format.c:165` `overflow` | `static void overflow(char *dest, size_t width)` | `static akerr_ErrorContext *overflow(char *dest, size_t width)` — no bool involved, it just returns the context | | 7 | `src/sink_akgl.c:36` `scroll` | `static void scroll(sink *)` | `static akerr_ErrorContext *scroll(sink *)` | | 8 | `src/sink_akgl.c:62` `newline` | `static void newline(sink *)` | `static akerr_ErrorContext *newline(sink *)` | ### What each one drags with it 1. **`word_is`** — 7 call sites in the same file (`:234,236,245,312,357,361,363`), all already inside functions returning `akerr_ErrorContext *`. Five are `if ( word_is(...) )` and become `PASS(errctx, word_is(w, k, &ok)); if ( ok ) {`. Two (`:357`, `:363`) sit inside `FAIL_ZERO_RETURN(errctx, word_is(word, "AS"), ...)` and split into two statements — the `PASS`, then `FAIL_ZERO_RETURN(errctx, ok, ...)`. Self-contained; nothing leaves the file. 2. **`akbasic_environment_is_waiting_for`** — the only one that is an **ABI break**: declared at `include/akbasic/environment.h:170`. Its recursive tail call becomes a `PASS`. Its sibling `akbasic_environment_is_waiting_for_any` (`src/environment.c:145`, header `:163`) has the identical shape and has to change with it or the pair becomes inconsistent. Callers: `src/runtime_commands.c:158`, `src/runtime.c:827,829,864` — `:864` is a term inside an `&&` chain and has to be hoisted out of the condition. Plus six assertions in `tests/environment_scope.c:61-77`, which currently call these directly inside `TEST_REQUIRE`. 3–4. **The scanner cascade** — `is_at_end`, `peek` (`:34`, not in the original eight but on the same chain and must go with them) and `peek_next`. Nine call sites: `:105,131,132,159,160,162,235,236,336`, all inside functions that already return `akerr_ErrorContext *`. Every one is a loop or `if` condition, so each becomes a hoist: ```c for ( ;; ) { PASS(errctx, is_at_end(obj, &done)); if ( done ) { break; } ... } ``` `:160` and `:236` are `(void)peek(obj, &c)` today and become `PASS`. This is the expensive one: it runs once per character of every line scanned, and adds an `aksl_strlen` call per character on top. 5. **`verb_compare`** — the one site where the ruling's remedy is unavailable, because `bsearch` fixes the comparator's signature and libc will not carry an out-param. The two options are to leave it on `strcmp` (its arguments are the search key and a static table entry, neither of which the caller can make NULL) or to drop `bsearch` for an in-house binary search that can propagate. Already tracked as akbasic **#14**; that issue is where it should be decided, not here. 6. **`overflow`** — cheapest of the eight. Both call sites (`src/format.c:229,274`) are already inside `render_numeric`, which returns `akerr_ErrorContext *`, so each is one `PASS`. No signature above it moves. 7–8. **`scroll` / `newline`** — a four-deep chain of `void` helpers: `scroll` → `newline` (`:62`) → `putchar_at` (`:92`) → `echo_line` (`:154`). All four have to return contexts before the chain reaches `sink_write` (`:116`), `sink_writeln` (`:133`) and `sink_readline` (`:333`), which already return them. Call sites: `:67`, `:97,101,140,380`, `:128,163,170`. AKGL-only — it does not build unless `AKBASIC_WITH_AKGL=ON`. ## The `ctype.h` note The original filing also asked whether `isspace`/`isdigit`/`isalnum`/`toupper` should have `aksl_*` forms. The same ruling answers it: they cannot fail, so there is nothing to return and nothing to terminate over. What is worth writing down is the `(unsigned char)` cast every correct `ctype.h` call needs, which is a precondition the caller must remember and which nothing in either library will remind them of. That belongs in akbasic's own notes, not as a request to libakstdlib. ## Where this leaves things * **This issue: close.** Nothing in libakstdlib or libakerror changes. * **The work is akbasic's**, and it is now specified above: six signature changes that stay inside akbasic, one public-header change, one site (`verb_compare`) that has to be settled in akbasic #14 instead. * **It also lands on the port.** The akbasic port branch left these eight on raw libc on the assumption that the library owed them a form. It does not, so the port either carries the signature changes or carries an explicit note that these eight are deliberately unconverted and why. Worth deciding before that branch merges. Filed by Tachikoma (Claude Code, Opus 5, 1M context)
tachikoma added this to the 0.3.0 milestone 2026-08-03 12:52:03 -04:00
tachikoma added the design-decisionblast-radius:mediumstatus::grooming labels 2026-08-03 12:52:03 -04:00
Owner

@tachikoma

So a function that answers a yes/no question -- and therefore returns bool -- has no way to call one.

They call it just like everything else. This isn't a bug in libakerror or libakstdlib. This library exists to wrap libakerror around libc, and libakerror's #3 design bullet is

Uncaught errors should cause program termination with a stacktrace

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.

@tachikoma > So a function that answers a yes/no question -- and therefore returns bool -- has no way to call one. They call it just like everything else. This isn't a bug in libakerror or libakstdlib. This library exists to wrap libakerror around libc, and libakerror's #3 design bullet is > Uncaught errors should cause program termination with a stacktrace 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.
tachikoma changed title from There is no form a small pure predicate can call, so eight sites stayed on raw libc to Ruled invalid: a bool predicate changes its own signature; the eight sites are akbasic's to convert 2026-08-03 14:49:12 -04:00
Author
Collaborator

@andrew Accepted — the ticket was wrong and I have rewritten it to record the ruling and recommend closing. The premise that a bool predicate has no way to call one of these was mine, not the library's.

It is also already refuted by akbasic's own code: src/symtab.c:22 is probe(obj, key, int *slot, bool *found) — a lookup predicate that returns the context and hands the yes/no out, PASSing the fallible aksl_strhash_djb2 straight through. src/runtime_structure.c:46, src/audio_akgl.c:124 and src/runtime_commands.c:733 are the same shape. So the eight sites are eight signatures akbasic has not converted, not a hole in libakstdlib.

Per site, what the conversion actually costs (line numbers on akbasic 330d731):

site corrected shape consequence
structtype.c:92 word_is akerr_ErrorContext *(w, k, bool *dest) 7 callers in the same file; :357 and :363 are inside FAIL_ZERO_RETURN and split into PASS + FAIL_ZERO_RETURN(errctx, ok, ...)
environment.c:156 ..._is_waiting_for akerr_ErrorContext *(env, cmd, bool *dest) ABI breakinclude/akbasic/environment.h:170; its twin _is_waiting_for_any must move with it; runtime.c:864 is an && term that needs hoisting; 6 assertions in tests/environment_scope.c
scanner.c:29 is_at_end, :43 peek_next (+ :34 peek, same chain) akerr_ErrorContext *(rt, bool *dest) / (rt, char *dest, bool *got) 9 callers, every one a loop or if condition that hoists into the body; adds an aksl_strlen per character scanned
verbs.c:200 verb_compare none available bsearch fixes the comparator signature, so it is either left on strcmp or bsearch is replaced — belongs in akbasic #14
format.c:165 overflow akerr_ErrorContext *(dest, width) cheapest: 2 callers, both already inside render_numeric, one PASS each
sink_akgl.c:36 scroll, :62 newline akerr_ErrorContext * on both drags putchar_at:92 and echo_line:154 with it before reaching sink_write/sink_writeln/sink_readline, which already return contexts; AKGL-only

Six of the eight stay inside akbasic. One is a public-header change. One has no out-param available at all.

This does land on the akbasic port branch, which left these eight on raw libc on exactly the assumption you have just rejected — so that branch either carries the signature changes or carries a note saying the eight are deliberately unconverted. Flagging it rather than deciding it, since the port is not mine to move.

@andrew Accepted — the ticket was wrong and I have rewritten it to record the ruling and recommend closing. The premise that a `bool` predicate has no way to call one of these was mine, not the library's. It is also already refuted by akbasic's own code: `src/symtab.c:22` is `probe(obj, key, int *slot, bool *found)` — a lookup predicate that returns the context and hands the yes/no out, `PASS`ing the fallible `aksl_strhash_djb2` straight through. `src/runtime_structure.c:46`, `src/audio_akgl.c:124` and `src/runtime_commands.c:733` are the same shape. So the eight sites are eight signatures akbasic has not converted, not a hole in libakstdlib. Per site, what the conversion actually costs (line numbers on akbasic `330d731`): | site | corrected shape | consequence | |---|---|---| | `structtype.c:92` `word_is` | `akerr_ErrorContext *(w, k, bool *dest)` | 7 callers in the same file; `:357` and `:363` are inside `FAIL_ZERO_RETURN` and split into `PASS` + `FAIL_ZERO_RETURN(errctx, ok, ...)` | | `environment.c:156` `..._is_waiting_for` | `akerr_ErrorContext *(env, cmd, bool *dest)` | **ABI break** — `include/akbasic/environment.h:170`; its twin `_is_waiting_for_any` must move with it; `runtime.c:864` is an `&&` term that needs hoisting; 6 assertions in `tests/environment_scope.c` | | `scanner.c:29` `is_at_end`, `:43` `peek_next` (+ `:34` `peek`, same chain) | `akerr_ErrorContext *(rt, bool *dest)` / `(rt, char *dest, bool *got)` | 9 callers, every one a loop or `if` condition that hoists into the body; adds an `aksl_strlen` per character scanned | | `verbs.c:200` `verb_compare` | **none available** | `bsearch` fixes the comparator signature, so it is either left on `strcmp` or `bsearch` is replaced — belongs in akbasic #14 | | `format.c:165` `overflow` | `akerr_ErrorContext *(dest, width)` | cheapest: 2 callers, both already inside `render_numeric`, one `PASS` each | | `sink_akgl.c:36` `scroll`, `:62` `newline` | `akerr_ErrorContext *` on both | drags `putchar_at:92` and `echo_line:154` with it before reaching `sink_write`/`sink_writeln`/`sink_readline`, which already return contexts; AKGL-only | Six of the eight stay inside akbasic. One is a public-header change. One has no out-param available at all. This does land on the akbasic port branch, which left these eight on raw libc on exactly the assumption you have just rejected — so that branch either carries the signature changes or carries a note saying the eight are deliberately unconverted. Flagging it rather than deciding it, since the port is not mine to move.
Owner

@tachikoma fix the akbasic port branch and reference this discussion

@tachikoma fix the akbasic port branch and reference this discussion
Sign in to join this conversation.