diff --git a/README.md b/README.md index 258d77e..86732ef 100644 --- a/README.md +++ b/README.md @@ -181,8 +181,9 @@ would notice. ## Testing -There are four harnesses. The first three take seconds; the fourth takes about -half an hour. +There are five harnesses. The first three take seconds and the fifth is instant; +the fourth takes about half an hour. The fifth is the only one that measures +something outside this repository. ### 1. The test suite @@ -469,6 +470,41 @@ right-leaning tree would have blown the stack the depth cap exists to protect), and `aksl_tree_remove` on an empty tree, which without its guard dereferences NULL. Both are in the suite now — which is what the harness is for. +### 5. Consumer adoption + +Coverage says the tests reach the code and mutation testing says they would +notice it breaking. Neither says anybody *wanted* the code. That question only has +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 313/7 # against a past count +``` + +It counts, across a consumer's source directory, how often that consumer calls +this library against how often it reaches past it to the libc function this +library wraps. Calls to libc functions **not** wrapped here — `isdigit`, `exit`, +`qsort` — score on neither side; the question is how often an *available* wrapper +gets bypassed. Comments and string literals are stripped before counting, and the +wrapped-libc set is read out of `include/akstdlib.h` rather than hardcoded, so a +recount after a release measures the surface that release actually shipped. + +A wrapper nobody calls either does not fit or is not discoverable, and both are +this library's problem rather than the consumer's. `TODO.md` carries the standing +figures and what they did and did not justify. + +Two warnings, both learned the hard way and both printed by `--baseline`: + +- **A rate is only comparable between two counts of the same tree.** If the + consumer grew between them, compare the percentage and say which commit each + number came from. Comparing the totals across a tree that tripled in size is how + a real improvement gets reported as a regression, or the reverse. +- **One consumer's ratio is evidence, not a plan.** A consumer that draws + everything from fixed pools will never call the allocator however good the + allocator is. Weight the result by what the consumer is, and get a second + consumer before treating any ranking as settled. + ## The pre-push hook `.githooks/pre-push` runs the fast harnesses — the default build and the diff --git a/TODO.md b/TODO.md index e55eb9b..af257f8 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +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 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, @@ -102,15 +103,16 @@ are fixed: the right child's `depth + 1` in the depth-first walk, and ## Evidence from the first full consumer -`akbasic` (`source.starfort.tech/andrew/akbasic`) is a ~6,300-line C interpreter -built on this library and `libakerror`. It was the first consumer to exercise the -whole surface rather than a corner of it, **and what it could not use is what -prioritised everything that has been built since.** +`akbasic` (`source.starfort.tech/andrew/akbasic`) is a C interpreter built on this +library and `libakerror` — ~6,300 lines of `src/` when it was first measured, +20,169 now. It was the first consumer to exercise the whole surface rather than a +corner of it, **and what it could not use is what prioritised everything that has +been built since.** **The number that started it.** Across `src/`, akbasic made **10 calls into this -library and 116 to raw libc** — a library whose value proposition is "turn silent -libc failures into error contexts", bypassed 92% of the time by the consumer most -committed to it. +library and 119 to raw libc** — a library whose value proposition is "turn silent +libc failures into error contexts", bypassed **92%** of the time by the consumer +most committed to it. | Raw libc it had to use | Count | Now available as | |---|---|---| @@ -121,8 +123,17 @@ committed to it. | `strncpy` | 15 | `aksl_strncpy` | | `strtoll` / `strtod` | 2 | `aksl_strtoll` / `aksl_strtod` | | `fgets` | 2 | `aksl_fgets` | +| `strncmp` | 1 | `aksl_strncmp` | +| `memmove` | 1 | `aksl_memmove` | | `strstr` | 1 | `aksl_strstr` | +Two corrections to that figure, both found by rebuilding it. It used to read 116; +the table it sat above summed to 117 and had no row for `strncmp` or `memmove`. +119 is what `scripts/consumer_calls.py` returns against akbasic `4e188b2`, and it +is the number everything below compares to. **The method is now a script rather +than a paragraph, because recovering it afterwards cost more than writing it down +would have.** + **All four things the port had to write for itself now exist here.** 1. **A strict `strtoll`/`strtod` wrapper** (`akbasic/src/convert.c`, ~60 lines). The @@ -144,8 +155,138 @@ committed to it. the sign-extended djb2 reads bytes unsigned; and the missing `va_end` — which akbasic's stdio text sink ran on every line of program output — is fixed. -**Still true, and still shaping the wishlist.** akbasic uses no allocator, no lists -and no trees, drawing everything from fixed pools by design. **A consumer that does -allocate would weight the `open`/`read`/`write` work far higher than this one -does**, so one consumer's count is evidence, not a plan. Re-counting against this -release is #26. +### The recount, against this release + +akbasic's `src/` was ported onto 0.2.0 and counted again (#26). The port builds +clean at `-Wall -Wextra`, passes **112/112** of akbasic's ctest suite, and is +ASan+UBSan-clean. + +| | libakstdlib | raw libc | bypassed | +|---|---|---|---| +| 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** | **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% → 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. 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 | +|---|---|---| +| 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 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 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-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 was 8 of the 13 sites the first count could not convert. **Ruled invalid** — the consumer changes its own signature, and now has | #38 | + +**#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 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 313 calls it now +makes: + +| Area | Calls | | +|---|---|---| +| 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%** | + +**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 +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 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. + +`akbasic/src/symtab.c` is the sharpest instance. It is the hand-rolled fixed-capacity +string-keyed hash table `aksl_hashmap_*` was generalised from, it survived the port +untouched, and the reason turned out to be one field rather than a design +disagreement — everything else about the two already lines up. #35 has it, and it +is the first collections work with a consumer actually waiting for it. diff --git a/scripts/consumer_calls.py b/scripts/consumer_calls.py new file mode 100755 index 0000000..b8f01f1 --- /dev/null +++ b/scripts/consumer_calls.py @@ -0,0 +1,245 @@ +#!/usr/bin/env python3 +""" +Consumer adoption harness for libakstdlib. + +Answers one question about a consumer's source tree: how often does it call +this library, and how often does it reach past this library to the libc +function this library wraps? + +The ratio is the only external evidence there is about whether the surface that +got built is the surface anyone wanted. A wrapper nobody calls is a wrapper that +either does not fit or is not discoverable, and both are the library's problem. + +The number this reports is only worth something if it is reproducible, which is +why this is a script and not a paragraph. TODO.md's first consumer figure was +recorded without one, and recovering the method afterwards cost more than +writing it down would have. + +WHAT COUNTS + + * The corpus is every *.c and *.h directly under the given directory. It does + not recurse: a consumer's src/ is the thing being measured, not its vendored + dependencies, and those are usually a subdirectory. + + * Comments and string/character literals are stripped before anything is + counted, so a function named in prose or inside a format string does not + score. This matters more than it sounds -- "strlen" appears in doc comments + throughout a codebase that has been thinking about strlen. + + * A call site is IDENT immediately followed by '(', where IDENT is not + preceded by an identifier character. Declarations are not distinguished from + calls; a consumer that declares a function named for a libc entry point will + over-count by one per declaration, which is visible in --detail. + + * A library call is any IDENT matching ^aksl_. + + * A bypass is any IDENT naming a libc function this library wraps. That set is + read out of include/akstdlib.h rather than hardcoded, so it grows when the + library grows and a recount after a release measures the surface that + release actually shipped. + + * libc functions this library does NOT wrap -- isdigit, exit, qsort -- score + on neither side. The question is how often a consumer bypasses an available + wrapper, not how much libc it uses. Adding a wrapper for something and + having it ignored is a finding; a consumer calling exit() is not. + +WHAT IT CANNOT TELL YOU + + One consumer's ratio is evidence, not a plan. A consumer that draws + everything from fixed pools will never call the allocator no matter how good + the allocator is, and will weight the string wrappers accordingly. Weight the + result by what the consumer is, and get a second consumer before treating any + ranking as settled. + +Usage: + scripts/consumer_calls.py DIR [options] + + DIR consumer source directory to measure, e.g. + ../akbasic/src + --header PATH akstdlib.h to read the wrapped-libc set from + (default: include/akstdlib.h beside this script's repo) + --detail list the per-function breakdown on both sides + --per-file list per-file counts, worst bypass ratio first + --baseline A/B compare against a previous count, e.g. --baseline 10/119 + --json emit the whole result as JSON instead of text +""" + +import argparse +import json +import os +import re +import sys +from collections import Counter + +# Wrapper families that are this library's own constructs rather than a libc +# function under a new name. aksl_list_append has no libc counterpart, so +# "append" must not become a name a consumer can be scored for bypassing. +LIBRARY_ONLY_PREFIXES = ("hashmap", "list", "tree", "strbuf", "version", + "strhash") + +# Library-only names whose first underscore-separated word is shared with a real +# libc entry point, so a prefix rule cannot separate them. aksl_realpath wraps +# realpath(3) and must score; aksl_realpath_alloc is this library's own. +LIBRARY_ONLY_NAMES = frozenset(("freep", "realpath_alloc")) + +CALL = re.compile(r"(?5}{raw:>6}") + + return 0 + + +if __name__ == "__main__": + sys.exit(main())