Recount the consumer calls against this release (#26) #39
@@ -479,7 +479,7 @@ an external answer, so there is a harness for it too:
|
||||
```sh
|
||||
scripts/consumer_calls.py ../akbasic/src # the ratio
|
||||
scripts/consumer_calls.py ../akbasic/src --detail --per-file # where it comes from
|
||||
scripts/consumer_calls.py ../akbasic/src --baseline 301/13 # against a past count
|
||||
scripts/consumer_calls.py ../akbasic/src --baseline 313/7 # against a past count
|
||||
```
|
||||
|
||||
It counts, across a consumer's source directory, how often that consumer calls
|
||||
|
||||
101
TODO.md
101
TODO.md
@@ -22,7 +22,7 @@ it has been through grooming.
|
||||
| Function coverage | 100% (154/154) |
|
||||
| Doxygen | 100% of 154, gated — `cmake --build build --target docs` fails on an undocumented function, parameter or return |
|
||||
| Mutation score | 72.3% (188/260 sampled from 1701), gated at 65 |
|
||||
| Consumer adoption | akbasic ported onto 0.2.0 calls this library 301 times and raw libc 13 — **4.1% bypassed**, from 92.2% at first count. Ungated, and one consumer only |
|
||||
| Consumer adoption | akbasic ported onto 0.2.0 calls this library 313 times and raw libc 7 — **2.2% bypassed**, from 92.2% at first count. Ungated, and one consumer only |
|
||||
|
||||
The six confirmed defects that used to head this file are fixed and
|
||||
`AKSL_KNOWN_FAILING_TESTS` is empty. What they were, and what changed as a result,
|
||||
@@ -165,74 +165,113 @@ ASan+UBSan-clean.
|
||||
|---|---|---|---|
|
||||
| Baseline — akbasic `4e188b2`, 5,679 lines of `src/` | 10 | 119 | **92.2%** |
|
||||
| Before the port — akbasic `330d731`, 20,169 lines | 45 | 285 | **86.4%** |
|
||||
| **After the port — same tree** | **301** | **13** | **4.1%** |
|
||||
| **After the port — same tree** | **313** | **7** | **2.2%** |
|
||||
|
||||
**Read the third row against the second, not the first.** The tree tripled between
|
||||
the baseline and the port, so 10/119 and 45/285 are counts of two different
|
||||
programs; only 86.4% → 4.1% is a like-for-like measurement. The 45 in the middle
|
||||
programs; only 86.4% → 2.2% is a like-for-like measurement. The 45 in the middle
|
||||
row is worth its own note — akbasic had already adopted `aksl_f*` across
|
||||
`runtime_disk.c` on its own, without anybody counting.
|
||||
|
||||
**Nothing was blocked by a missing wrapper.** Every libc call akbasic makes had an
|
||||
`aksl_*` counterpart. 272 of the 285 sites converted; the 13 that did not are
|
||||
`aksl_*` counterpart. 278 of the 285 sites converted; the 7 that did not are
|
||||
blocked by wrapper *shape*, and they are the useful output:
|
||||
|
||||
| Why it could not be used | Sites | Where |
|
||||
|---|---|---|
|
||||
| No error channel to route into — the enclosing function returns `bool` or `void`, or is a `bsearch` comparator whose signature libc fixes | 8 | `structtype.c` `word_is`, `environment.c` `akbasic_environment_is_waiting_for` (public API), `scanner.c` `is_at_end` and `peek_next`, `verbs.c` `verb_compare`, `format.c` `overflow`, `sink_akgl.c` `scroll` (×2) |
|
||||
| Truncation is the answer, not the error | 4 | `format.c`, `structtype.c`, `runtime_struct.c`, `renumber.c` |
|
||||
| Short-circuit is memory-safety-load-bearing and the compare cannot be hoisted past the NULL arm guarding it | 1 | `runtime_trap.c` |
|
||||
| Truncation is the answer, not the error | 6 | `runtime.c` `akbasic_runtime_error`, `host.c` (×3), `runtime_struct.c` (×2) |
|
||||
| A `bsearch(3)` comparator, whose signature libc fixes, so there is no out parameter to report through | 1 | `verbs.c` `verb_compare` |
|
||||
|
||||
The truncation four are worth spelling out, because they are a contract decision
|
||||
rather than an accident. `PRINT USING "###"; 1E300` prints `***` today: the render
|
||||
truncates, the truncated text has no `.`, and the formatter takes its overflow
|
||||
path on exactly that. Through `aksl_snprintf` it raises `AKERR_OUTOFBOUNDS` out of
|
||||
the interpreter instead. Two more are truncation-tolerant renderers that print
|
||||
what fits and stop, and the fourth uses `snprintf`'s return to raise akbasic's
|
||||
own `AKBASIC_ERR_BOUNDS` with its own message.
|
||||
**The eight sites that used to head this table are gone, and how they went is the
|
||||
finding.** They were the `bool` predicates and `void` helpers with no error channel
|
||||
to route into, and the first filing (#38) asked this library for a form they could
|
||||
call. Andrew ruled that invalid: a function that cannot report an error changes its
|
||||
own signature rather than being handed a way to swallow one. Seven of the eight did
|
||||
exactly that — they now return `akerr_ErrorContext *` and hand the answer back
|
||||
through an out parameter, one of them (`akbasic_environment_is_waiting_for` and its
|
||||
sibling) as a public header change. Four more functions on the same call chains
|
||||
(`scanner.c` `peek` and `match_next_char`, `sink_akgl.c` `putchar_at`, `echo_line`
|
||||
and `edit_key`) had to move with them. **The wrapper shape was right and the
|
||||
consumer's signatures were wrong**, which is not what the first count assumed.
|
||||
|
||||
The truncation six are a contract decision rather than an accident, and they are
|
||||
what is genuinely left. The sharpest is `akbasic_runtime_error` — the one function
|
||||
that tells the user *what went wrong*, formatting a 12,384-byte error message into
|
||||
a 512-byte line. Truncating a report there is correct; raising `AKERR_OUTOFBOUNDS`
|
||||
would replace the diagnosis with a second, different failure. Two more are
|
||||
truncation-tolerant renderers that print what fits and stop, one of which reads
|
||||
`snprintf`'s return value to *detect* the truncation and skip the rest of the
|
||||
render. The remaining three read host-supplied strings into fixed fields.
|
||||
|
||||
Two of those three, in `akbasic/src/host.c`, are a latent defect the port surfaced
|
||||
rather than a decision: 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` refuses the identical case
|
||||
outright with a limit message. The two registration paths disagree. That is
|
||||
akbasic's to fix, and it is flagged at the site.
|
||||
|
||||
### What the recount found, and where it went
|
||||
|
||||
Every one of the 13 blocked sites came back to wrapper *shape* rather than a
|
||||
missing wrapper, and the same seven shapes recurred across ten independent
|
||||
conversion passes. They are filed, not listed here:
|
||||
Every blocked site came back to wrapper *shape* rather than a missing wrapper, and
|
||||
the same seven shapes recurred across ten independent conversion passes. They are
|
||||
filed, not listed here:
|
||||
|
||||
| Finding | Filed as |
|
||||
|---|---|
|
||||
| `aksl_snprintf`'s `count` out-param is required, so ~20 sites carry an `int written` that is written and never read. Raised by all ten passes. `-Wall -Wextra` cannot see it — `&written` is a use | #32 |
|
||||
| No equality comparison. All 43 comparison sites flatten the three-way `int` to `== 0`; not one wants an ordering, and five now need a sentinel whose *initial value is load-bearing* | #33 |
|
||||
| No truncating format and no length query, which is the whole of the truncation-four above | #34 |
|
||||
| No truncating format and no length query, which is the whole of the truncation-six above and the only shape still blocking a conversion | #34 |
|
||||
| `aksl_hashmap_*` carries one payload, which is the only reason `akbasic/src/symtab.c` still exists | #35 |
|
||||
| `aksl_fgets` signals end of input by raising, so a read loop cannot be a condition | #36 |
|
||||
| A caller cannot add its own context to a wrapper's error, so it raises and discards instead — eight lines where there were two | #37 |
|
||||
| No form a `bool` predicate or a `void` function can call, which is 8 of the 13 blocked sites. Carries the `ctype.h` question and the infallible-`memset` question with it | #38 |
|
||||
| No form a `bool` predicate or a `void` function can call, which was 8 of the 13 sites the first count could not convert. **Ruled invalid** — the consumer changes its own signature, and now has | #38 |
|
||||
|
||||
`#14` already covered the `bsearch` comparator, and the port confirmed it from the
|
||||
consumer side.
|
||||
**#38 is the one worth reading, because it is the one that was wrong.** It asked
|
||||
this library to grow a form a `bool` predicate could call, and the answer was that
|
||||
a predicate which cannot report an error should stop returning `bool`. Seven of its
|
||||
eight sites converted on that basis, and they are why the count is 2.2% and not 4.1%.
|
||||
The `ctype.h` half of the same filing is settled too: `isspace`, `isdigit`,
|
||||
`isalnum` and `toupper` cannot fail, so there is nothing for a wrapper to return
|
||||
and no reason to add one. What a caller does need is the `(unsigned char)` cast
|
||||
every correct `ctype.h` call takes, and that is akbasic's note to keep, not this
|
||||
library's.
|
||||
|
||||
The `bsearch` comparator has **no issue of its own**. #38 attributed it to akbasic
|
||||
`#14`, which is a mis-citation — that issue is the `COLLISION`/`BUMP` pairing
|
||||
threshold. Converting the comparator means dropping `bsearch(3)` for an in-house
|
||||
binary search that can propagate, on a lookup that runs once per scanned
|
||||
identifier, and that wants filing against akbasic before anybody does it.
|
||||
|
||||
**The one thing the wrappers did better than the libc they replaced** is worth
|
||||
recording next to the complaints: `aksl_fgets`'s `len_out` **deleted** two `strlen`
|
||||
calls rather than converting them, and is more correct than what it replaced for a
|
||||
line containing an embedded NUL. It is the only one of 272 conversions that
|
||||
line containing an embedded NUL. It is the only one of 278 conversions that
|
||||
produced less code than it started with.
|
||||
|
||||
**The port also found a defect in akbasic rather than in this library.** `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. Hoisting
|
||||
the loop into its own helper — which converting `fgets` required anyway, because
|
||||
neither `CATCH` nor `PASS` is legal in a loop inside an `ATTEMPT` — fixes it. That
|
||||
is the protocol's own rule catching a real leak the moment somebody had to obey it.
|
||||
|
||||
### Still true, and still the reason one count is not a plan
|
||||
|
||||
akbasic uses no allocator, no lists and no trees, drawing everything from fixed
|
||||
pools by design, and porting it did not change that. Of the 301 calls it now
|
||||
pools by design, and porting it did not change that. Of the 313 calls it now
|
||||
makes:
|
||||
|
||||
| Area | Calls | |
|
||||
|---|---|---|
|
||||
| Strings | 122 | 40.5% |
|
||||
| Memory | 69 | 22.9% |
|
||||
| Formatted output | 59 | 19.6% |
|
||||
| Streams | 38 | 12.6% |
|
||||
| String → number | 12 | 4.0% |
|
||||
| Strings | 149 | 47.6% |
|
||||
| Memory | 72 | 23.0% |
|
||||
| Formatted output | 43 | 13.7% |
|
||||
| Streams and files | 36 | 11.5% |
|
||||
| String → number | 12 | 3.8% |
|
||||
| Hashing | 1 | 0.3% |
|
||||
| **Collections** | **0** | **0%** |
|
||||
|
||||
**Four fifths of the evidence is strings, memory and formatting.** The collections
|
||||
**Five sixths of the evidence is strings, memory and formatting.** The collections
|
||||
work — list, tree, hash map, string buffer, `src/collections.c` and the largest
|
||||
single body of code in this library — has **not one consumer call site**, and the
|
||||
single hashing call next to it is `aksl_strhash_djb2` feeding a hash table akbasic
|
||||
@@ -240,8 +279,8 @@ wrote for itself. **A consumer that does allocate would weight the
|
||||
`open`/`read`/`write` work far higher than this one does**, so this remains
|
||||
evidence and not a plan.
|
||||
|
||||
**The number to distrust is not the 4.1%; it is the 0%.** A recount that moves
|
||||
92% to 4% on one consumer says the string, memory and format wrappers fit the
|
||||
**The number to distrust is not the 2.2%; it is the 0%.** A recount that moves
|
||||
92% to 2% on one consumer says the string, memory and format wrappers fit the
|
||||
consumer that asked for them. It says nothing at all about the half of the library
|
||||
that consumer never calls, and it cannot, however many times it is run. What would
|
||||
say something is a second consumer with different shape — one that allocates.
|
||||
|
||||
Reference in New Issue
Block a user