diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index a510a73..3af49c8 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -238,16 +238,27 @@ jobs: # sites -- gcov attributes ATTEMPT/CATCH/PASS to the caller, so coverage # cannot see them and mutation testing is the only thing that checks them. # - # Bounded to two small, fast, deterministic files. src/value.c is the file - # most worth mutating and is deliberately *not* here: 368 mutants at ~11s - # each is about 70 minutes, because almost everything links against it. - # Run the default target locally for the whole tree: + # Bounded to one small, fast, deterministic file. It used to be two -- + # src/convert.c was the other -- but that file has been deleted: it existed + # only because libakstdlib's aksl_ato* family could not report a conversion + # failure, and 0.2.0 fixed that. src/value.c is the file most worth + # mutating and is deliberately *not* here: 368 mutants at ~11s each is + # about 70 minutes, because almost everything links against it. Run the + # default target locally for the whole tree: # cmake --build build --target mutation # # The threshold is a ratchet, not a quality bar. The measured score for - # these two files is 77.8% (84 killed, 24 survived, 108 total); 65 leaves - # headroom for runner variance while still failing on a real regression -- - # a test deleted, or new untested code added. + # src/symtab.c is 74.1% (46 killed by test, 14 by compile, 21 survived, 81 + # total); 65 leaves headroom for runner variance while still failing on a + # real regression -- a test deleted, or new untested code added. + # + # src/audio_tables.c was measured as a replacement second file and scored + # 64.7%, below the gate. That is a real test gap rather than a reason to + # avoid 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 -- the same gap this job's own history records + # closing for src/symtab.c. Recorded in TODO.md; add the file back when it + # is closed. # # It was 73.1% before writing this job. The run's own findings closed the # gap: nothing exercised a maximum-length symbol-table key, so every @@ -255,17 +266,16 @@ jobs: # freshly initialised table was actually zeroed. Adding those two # assertions to tests/symtab.c killed five mutants. The same run found that # errno was never asserted clear before a strtoll, which is what stops a - # stale ERANGE from failing a valid conversion -- that is now in - # tests/convert.c. + # stale ERANGE from failing a valid conversion -- that assertion outlived + # the wrapper it was written for and is now in tests/numeric_contract.c, + # asserted against libakstdlib. # - # The 24 remaining survivors are listed in the published report. Most are + # The 21 remaining survivors are listed in the published report. Most are # ICR mutants on loop and accumulator initialisers that a stronger - # placement assertion would catch; three in convert.c are genuinely - # equivalent and cannot be killed. Recorded in TODO.md. + # placement assertion would catch. Recorded in TODO.md. - name: mutation testing run: | python3 scripts/mutation_test.py \ - --target src/convert.c \ --target src/symtab.c \ --junit mutation-junit.xml \ --threshold 65 diff --git a/.gitea/workflows/release.yaml b/.gitea/workflows/release.yaml index e1d77b6..d15fa4f 100644 --- a/.gitea/workflows/release.yaml +++ b/.gitea/workflows/release.yaml @@ -83,10 +83,10 @@ jobs: # measure them. # # The default threshold matches ci.yaml's rather than being stricter. - # Whole-tree coverage is uneven -- src/convert.c and src/symtab.c are the - # two measured files, at 77.8% between them, and the rest is unmeasured, so - # a tighter number here would be a guess. Raise it with the workflow input - # once a full run has established a real baseline. + # Whole-tree coverage is uneven -- src/symtab.c is the only file measured + # on the push path, at 74.1%, and the rest is unmeasured, so a tighter + # number here would be a guess. Raise it with the workflow input once a + # full run has established a real baseline. # # The two inputs arrive through the environment rather than being # interpolated straight into the script. ${{ }} substitution happens before diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c848a9..7c58dce 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,7 +118,6 @@ endfunction() # --------------------------------------------------------------------------- set(AKBASIC_SOURCES src/audio_tables.c - src/convert.c src/environment.c src/error.c src/grammar.c @@ -217,13 +216,13 @@ endif() # --------------------------------------------------------------------------- set(AKBASIC_TESTS audio_verbs - convert devices environment_scope error_codes grammar_leaves graphics_verbs input_verbs + numeric_contract parser_commands parser_expressions runtime_evaluate diff --git a/README.md b/README.md index ea0855e..1e7f6ec 100644 --- a/README.md +++ b/README.md @@ -439,4 +439,4 @@ The exception is `FILTER`, which sets the SID's filter cutoff, band switches and Everything is a submodule; `git submodule update --init --recursive` gets all of it. There is nothing to install first. -`src/convert.c` wraps `strtoll`/`strtod` with a strict contract — no digits consumed, trailing junk and overflow are all errors — because `libakstdlib`'s `aksl_atoi` family could not report a conversion failure at all. As of `libakstdlib` 0.2.0 it can, so that file has outlived its reason and is due for removal; `TODO.md` section 1.9 lists what deleting it touches, including the CI mutation job that currently targets it. +String-to-number conversion goes straight to `libakstdlib`, which raises on no digits, on trailing junk and on overflow — so `VAL("garbage")` is an error rather than a silent `0`. This repository carried its own `src/convert.c` wrapper until `libakstdlib` 0.2.0 grew that contract; `tests/numeric_contract.c` now pins it from this side, because a regression in it would make four things quietly return zero. diff --git a/TODO.md b/TODO.md index 99d358d..a70c093 100644 --- a/TODO.md +++ b/TODO.md @@ -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 12–17** — 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`. diff --git a/include/akbasic/convert.h b/include/akbasic/convert.h deleted file mode 100644 index 3cd66e3..0000000 --- a/include/akbasic/convert.h +++ /dev/null @@ -1,46 +0,0 @@ -/** - * @file convert.h - * @brief Declares string-to-number conversion that can actually report failure. - * - * This exists because libakstdlib's aksl_atoi/atol/atoll/atof family cannot: - * atoi("not a number") returns success with 0, and an overflowing literal - * returns success with a wrapped value (deps/libakstdlib/TODO.md 2.1.5). The Go - * reference checks the conversion error at every numeric site and turns it into - * a BASIC error, so porting onto that family would convert four diagnosable - * errors into wrong answers. - * - * Delete this file and switch the call sites over when libakstdlib grows the - * stricter contract its TODO.md already specifies. - */ - -#ifndef _AKBASIC_CONVERT_H_ -#define _AKBASIC_CONVERT_H_ - -#include - -#include - -/** - * @brief Convert a complete string to a 64-bit integer. - * @param str Source string; must be entirely consumed. - * @param base Numeric base, or 0 to let strtoll infer it from a 0x/0 prefix. - * @param dest Output destination populated by the function. - * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER When `str` or `dest` is NULL. - * @throws AKBASIC_ERR_VALUE When no digits were consumed or trailing junk remains. - * @throws ERANGE When the value does not fit in an int64_t. - */ -akerr_ErrorContext AKERR_NOIGNORE *akbasic_str_to_int64(const char *str, int base, int64_t *dest); - -/** - * @brief Convert a complete string to a double. - * @param str Source string; must be entirely consumed. - * @param dest Output destination populated by the function. - * @return `NULL` on success, otherwise an error context owned by the caller. - * @throws AKERR_NULLPOINTER When `str` or `dest` is NULL. - * @throws AKBASIC_ERR_VALUE When no digits were consumed or trailing junk remains. - * @throws ERANGE When the value is out of range for a double. - */ -akerr_ErrorContext AKERR_NOIGNORE *akbasic_str_to_double(const char *str, double *dest); - -#endif // _AKBASIC_CONVERT_H_ diff --git a/src/convert.c b/src/convert.c deleted file mode 100644 index b7354ac..0000000 --- a/src/convert.c +++ /dev/null @@ -1,65 +0,0 @@ -/** - * @file convert.c - * @brief Implements strict string-to-number conversion over strtoll/strtod. - */ - -#include -#include -#include - -#include - -#include -#include - -akerr_ErrorContext *akbasic_str_to_int64(const char *str, int base, int64_t *dest) -{ - PREPARE_ERROR(errctx); - char *endptr = NULL; - long long result = 0; - - FAIL_ZERO_RETURN(errctx, (str != NULL), AKERR_NULLPOINTER, - "NULL string in integer conversion"); - FAIL_ZERO_RETURN(errctx, (dest != NULL), AKERR_NULLPOINTER, - "NULL destination in integer conversion"); - - errno = 0; - result = strtoll(str, &endptr, base); - - /* No digits consumed at all. */ - FAIL_NONZERO_RETURN(errctx, (endptr == str), AKBASIC_ERR_VALUE, - "INTEGER CONVERSION ON '%s'", str); - /* Trailing junk. The whole lexeme must be the number. */ - FAIL_NONZERO_RETURN(errctx, (*endptr != '\0'), AKBASIC_ERR_VALUE, - "INTEGER CONVERSION ON '%s'", str); - FAIL_NONZERO_RETURN(errctx, (errno == ERANGE), ERANGE, - "Integer literal '%s' is out of range", str); - - *dest = (int64_t)result; - SUCCEED_RETURN(errctx); -} - -akerr_ErrorContext *akbasic_str_to_double(const char *str, double *dest) -{ - PREPARE_ERROR(errctx); - char *endptr = NULL; - double result = 0.0; - - FAIL_ZERO_RETURN(errctx, (str != NULL), AKERR_NULLPOINTER, - "NULL string in float conversion"); - FAIL_ZERO_RETURN(errctx, (dest != NULL), AKERR_NULLPOINTER, - "NULL destination in float conversion"); - - errno = 0; - result = strtod(str, &endptr); - - FAIL_NONZERO_RETURN(errctx, (endptr == str), AKBASIC_ERR_VALUE, - "FLOAT CONVERSION ON '%s'", str); - FAIL_NONZERO_RETURN(errctx, (*endptr != '\0'), AKBASIC_ERR_VALUE, - "FLOAT CONVERSION ON '%s'", str); - FAIL_NONZERO_RETURN(errctx, (errno == ERANGE), ERANGE, - "Float literal '%s' is out of range", str); - - *dest = result; - SUCCEED_RETURN(errctx); -} diff --git a/src/grammar.c b/src/grammar.c index af8faad..fc425f0 100644 --- a/src/grammar.c +++ b/src/grammar.c @@ -8,8 +8,8 @@ #include #include +#include -#include #include #include @@ -260,6 +260,7 @@ akerr_ErrorContext *akbasic_leaf_new_grouping(akbasic_ASTLeaf *obj, akbasic_ASTL akerr_ErrorContext *akbasic_leaf_new_literal_int(akbasic_ASTLeaf *obj, const char *lexeme) { PREPARE_ERROR(errctx); + long long value = 0; int base = 10; FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL leaf in new_literal_int"); @@ -279,7 +280,8 @@ akerr_ErrorContext *akbasic_leaf_new_literal_int(akbasic_ASTLeaf *obj, const cha base = 8; } PASS(errctx, akbasic_leaf_init(obj, AKBASIC_LEAF_LITERAL_INT)); - PASS(errctx, akbasic_str_to_int64(lexeme, base, &obj->literal_int)); + PASS(errctx, aksl_strtoll(lexeme, NULL, base, &value)); + obj->literal_int = (int64_t)value; SUCCEED_RETURN(errctx); } @@ -289,7 +291,7 @@ akerr_ErrorContext *akbasic_leaf_new_literal_float(akbasic_ASTLeaf *obj, const c FAIL_ZERO_RETURN(errctx, (obj != NULL), AKERR_NULLPOINTER, "NULL leaf in new_literal_float"); PASS(errctx, akbasic_leaf_init(obj, AKBASIC_LEAF_LITERAL_FLOAT)); - PASS(errctx, akbasic_str_to_double(lexeme, &obj->literal_float)); + PASS(errctx, aksl_atof(lexeme, &obj->literal_float)); SUCCEED_RETURN(errctx); } diff --git a/src/runtime_commands.c b/src/runtime_commands.c index 939ed18..d561bfb 100644 --- a/src/runtime_commands.c +++ b/src/runtime_commands.c @@ -16,7 +16,6 @@ #include #include -#include #include #include "verbs.h" @@ -305,6 +304,7 @@ akerr_ErrorContext *akbasic_cmd_input(akbasic_Runtime *obj, akbasic_ASTLeaf *exp akbasic_Value *unused = NULL; char rendered[AKBASIC_MAX_STRING_LENGTH]; char buffer[AKBASIC_MAX_LINE_LENGTH]; + long long converted = 0; bool eof = false; (void)lval; (void)rval; @@ -330,11 +330,12 @@ akerr_ErrorContext *akbasic_cmd_input(akbasic_Runtime *obj, akbasic_ASTLeaf *exp switch ( identifier->leaftype ) { case AKBASIC_LEAF_IDENTIFIER_INT: entered->valuetype = AKBASIC_TYPE_INTEGER; - PASS(errctx, akbasic_str_to_int64(buffer, 10, &entered->intval)); + PASS(errctx, aksl_atoll(buffer, &converted)); + entered->intval = (int64_t)converted; break; case AKBASIC_LEAF_IDENTIFIER_FLOAT: entered->valuetype = AKBASIC_TYPE_FLOAT; - PASS(errctx, akbasic_str_to_double(buffer, &entered->floatval)); + PASS(errctx, aksl_atof(buffer, &entered->floatval)); break; default: entered->valuetype = AKBASIC_TYPE_STRING; diff --git a/src/runtime_functions.c b/src/runtime_functions.c index 4ab6335..71259e1 100644 --- a/src/runtime_functions.c +++ b/src/runtime_functions.c @@ -17,8 +17,8 @@ #include #include +#include -#include #include #include @@ -258,7 +258,7 @@ akerr_ErrorContext *akbasic_fn_val(akbasic_Runtime *obj, akbasic_ASTLeaf *expr, * failure, so VAL("garbage") would quietly answer 0.0 instead of raising. * See TODO.md section 1.9. */ - PASS(errctx, akbasic_str_to_double(arg->stringval, &out->floatval)); + PASS(errctx, aksl_atof(arg->stringval, &out->floatval)); *dest = out; SUCCEED_RETURN(errctx); } diff --git a/src/runtime_graphics.c b/src/runtime_graphics.c index f2870fe..8ef81ea 100644 --- a/src/runtime_graphics.c +++ b/src/runtime_graphics.c @@ -18,8 +18,8 @@ #include #include +#include -#include #include #include @@ -620,6 +620,7 @@ akerr_ErrorContext *akbasic_cmd_gshape(akbasic_Runtime *obj, akbasic_ASTLeaf *ex double coords[2]; int count = 0; int64_t handle = 0; + long long converted = 0; double x = 0.0; double y = 0.0; int64_t zerosubscript[1] = { 0 }; @@ -645,7 +646,8 @@ akerr_ErrorContext *akbasic_cmd_gshape(akbasic_Runtime *obj, akbasic_ASTLeaf *ex (strncmp(stored->stringval, SHAPE_PREFIX, strlen(SHAPE_PREFIX)) == 0), AKBASIC_ERR_VALUE, "GSHAPE was given a string that did not come from SSHAPE"); - PASS(errctx, akbasic_str_to_int64(stored->stringval + strlen(SHAPE_PREFIX), 10, &handle)); + PASS(errctx, aksl_atoll(stored->stringval + strlen(SHAPE_PREFIX), &converted)); + handle = (int64_t)converted; scale_point(&obj->gfx, &x, &y); PASS(errctx, obj->graphics->paste_shape(obj->graphics, (int)handle, x, y)); diff --git a/src/scanner.c b/src/scanner.c index 32e432e..885d948 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -9,8 +9,8 @@ #include #include +#include -#include #include #include #include @@ -143,6 +143,7 @@ static akerr_ErrorContext *match_number(akbasic_Runtime *obj) char c = '\0'; char nc = '\0'; int64_t lineno = 0; + long long converted = 0; obj->tokentype = AKBASIC_TOK_LITERAL_INT; while ( !is_at_end(obj) ) { @@ -165,7 +166,8 @@ static akerr_ErrorContext *match_number(akbasic_Runtime *obj) if ( obj->tokentype == AKBASIC_TOK_LITERAL_INT && linenumber ) { PASS(errctx, get_lexeme(obj, lexeme, sizeof(lexeme))); ATTEMPT { - CATCH(errctx, akbasic_str_to_int64(lexeme, 10, &lineno)); + CATCH(errctx, aksl_atoll(lexeme, &converted)); + lineno = (int64_t)converted; } CLEANUP { } PROCESS(errctx) { } HANDLE_DEFAULT(errctx) { diff --git a/tests/audio_verbs.c b/tests/audio_verbs.c index 1d20875..8ff09d1 100644 --- a/tests/audio_verbs.c +++ b/tests/audio_verbs.c @@ -58,16 +58,41 @@ static void test_tables(void) TEST_REQUIRE_STATUS(akbasic_audio_register_to_hz(65536, &hz), AKBASIC_ERR_BOUNDS); TEST_REQUIRE_STATUS(akbasic_audio_register_to_hz(-1, &hz), AKBASIC_ERR_BOUNDS); - /* Attack and decay are different tables, and decay is three times attack. */ - TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(0, true, &ms)); - TEST_REQUIRE_INT(ms, 2); - TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(0, false, &ms)); - TEST_REQUIRE_INT(ms, 6); - TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(15, true, &ms)); - TEST_REQUIRE_INT(ms, 8000); - TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(15, false, &ms)); - TEST_REQUIRE_INT(ms, 24000); + /* + * Every entry of both ADSR tables, not just the ends. + * + * Mutation testing is why: checking only rate 0 and rate 15 left the twelve + * entries between them free to be any number at all, and the failure mode of + * this file is a constant that is plausibly wrong rather than obviously + * wrong. A corrupted middle entry makes one envelope decay at the wrong + * speed, which nothing else in the suite would notice and no listener could + * attribute. + * + * The values are the 6581/8580 datasheet's, transcribed here independently + * of src/audio_tables.c rather than read back from it. + */ + { + static const int attack[16] = { + 2, 8, 16, 24, 38, 56, 68, 80, + 100, 250, 500, 800, 1000, 3000, 5000, 8000 + }; + int rate = 0; + + for ( rate = 0; rate < 16; rate++ ) { + TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(rate, true, &ms)); + TEST_REQUIRE_INT(ms, attack[rate]); + /* + * Decay and release are exactly three times attack. That is a + * property of the chip's envelope generator rather than a + * coincidence, so it is asserted as the relationship it is. + */ + TEST_REQUIRE_OK(akbasic_audio_rate_to_ms(rate, false, &ms)); + TEST_REQUIRE_INT(ms, attack[rate] * 3); + } + } TEST_REQUIRE_STATUS(akbasic_audio_rate_to_ms(16, true, &ms), AKBASIC_ERR_BOUNDS); + TEST_REQUIRE_STATUS(akbasic_audio_rate_to_ms(-1, false, &ms), AKBASIC_ERR_BOUNDS); + TEST_REQUIRE_STATUS(akbasic_audio_rate_to_ms(0, true, NULL), AKERR_NULLPOINTER); /* * Equal temperament, checked at the three points anybody would know: A4 is diff --git a/tests/convert.c b/tests/convert.c deleted file mode 100644 index bfcedcf..0000000 --- a/tests/convert.c +++ /dev/null @@ -1,72 +0,0 @@ -/** - * @file convert.c - * @brief Tests the strict string-to-number converters. - * - * These exist because libakstdlib's aksl_ato* family cannot report a conversion - * failure. Every assertion here is one that family would get wrong. - */ - -#include - -#include -#include - -#include "testutil.h" - -int main(void) -{ - int64_t i = 0; - double d = 0.0; - - TEST_REQUIRE_OK(akbasic_error_register()); - - /* Valid decimal, hex and negative. */ - TEST_REQUIRE_OK(akbasic_str_to_int64("1234", 10, &i)); - TEST_REQUIRE_INT(i, 1234); - TEST_REQUIRE_OK(akbasic_str_to_int64("0xff", 16, &i)); - TEST_REQUIRE_INT(i, 255); - TEST_REQUIRE_OK(akbasic_str_to_int64("-256", 10, &i)); - TEST_REQUIRE_INT(i, -256); - - /* - * The whole point: aksl_atoi returns success with 0 for each of these. - */ - TEST_REQUIRE_STATUS(akbasic_str_to_int64("", 10, &i), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_int64("not a number", 10, &i), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_int64("12abc", 10, &i), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_int64("12 ", 10, &i), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_int64("99999999999999999999999", 10, &i), ERANGE); - - TEST_REQUIRE_STATUS(akbasic_str_to_int64(NULL, 10, &i), AKERR_NULLPOINTER); - TEST_REQUIRE_STATUS(akbasic_str_to_int64("1", 10, NULL), AKERR_NULLPOINTER); - - /* Floats. */ - TEST_REQUIRE_OK(akbasic_str_to_double("123.456", &d)); - TEST_REQUIRE_FEQ(d, 123.456); - TEST_REQUIRE_OK(akbasic_str_to_double("-2.5", &d)); - TEST_REQUIRE_FEQ(d, -2.5); - TEST_REQUIRE_OK(akbasic_str_to_double("32", &d)); - TEST_REQUIRE_FEQ(d, 32.0); - - TEST_REQUIRE_STATUS(akbasic_str_to_double("", &d), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_double("garbage", &d), AKBASIC_ERR_VALUE); - TEST_REQUIRE_STATUS(akbasic_str_to_double("1.5x", &d), AKBASIC_ERR_VALUE); - - /* - * errno hygiene. strtoll and strtod do not clear errno on success, so a - * stale ERANGE left by some earlier call would make a perfectly valid - * conversion raise. That is what the `errno = 0` before each call is for. - * - * Found by mutation testing: deleting that line, or setting errno to - * something non-zero, survived the suite until these four assertions - * existed. - */ - errno = ERANGE; - TEST_REQUIRE_OK(akbasic_str_to_int64("42", 10, &i)); - TEST_REQUIRE_INT(i, 42); - errno = ERANGE; - TEST_REQUIRE_OK(akbasic_str_to_double("2.5", &d)); - TEST_REQUIRE_FEQ(d, 2.5); - - return akbasic_test_failures; -} diff --git a/tests/language/numeric/octal_literal.bas b/tests/language/numeric/octal_literal.bas new file mode 100644 index 0000000..3f5687e --- /dev/null +++ b/tests/language/numeric/octal_literal.bas @@ -0,0 +1,6 @@ +10 REM The reference selects base 8 for any lexeme starting with 0, so 010 is 8 +20 REM and 08 is a parse error. Commodore BASIC has no octal literals -- this is +30 REM TODO.md section 6 item 10, reproduced deliberately until it is fixed on +40 REM purpose. Pinned here so the eventual fix has to change a golden file. +50 PRINT 010 +60 PRINT 08 diff --git a/tests/language/numeric/octal_literal.txt b/tests/language/numeric/octal_literal.txt new file mode 100644 index 0000000..6402eb4 --- /dev/null +++ b/tests/language/numeric/octal_literal.txt @@ -0,0 +1,3 @@ +8 +? 60 : PARSE ERROR trailing junk "8" after the number in "08" + diff --git a/tests/language/numeric/val_refuses_garbage.bas b/tests/language/numeric/val_refuses_garbage.bas new file mode 100644 index 0000000..43e3c3a --- /dev/null +++ b/tests/language/numeric/val_refuses_garbage.bas @@ -0,0 +1,6 @@ +10 REM VAL on a non-numeric string must raise rather than quietly return 0. +20 REM There was no golden coverage of this until the converters moved onto +30 REM libakstdlib 0.2.0, which is when the message became that library's. +40 PRINT VAL("32") +50 PRINT VAL("garbage") +60 PRINT "UNREACHABLE" diff --git a/tests/language/numeric/val_refuses_garbage.txt b/tests/language/numeric/val_refuses_garbage.txt new file mode 100644 index 0000000..89a69d2 --- /dev/null +++ b/tests/language/numeric/val_refuses_garbage.txt @@ -0,0 +1,3 @@ +32.000000 +? 50 : RUNTIME ERROR no digits in "garbage" + diff --git a/tests/numeric_contract.c b/tests/numeric_contract.c new file mode 100644 index 0000000..952b4ae --- /dev/null +++ b/tests/numeric_contract.c @@ -0,0 +1,102 @@ +/** + * @file numeric_contract.c + * @brief Pins the string-to-number contract this port depends on libakstdlib for. + * + * `src/convert.c` used to live here in spirit: it wrapped `strtoll`/`strtod` + * because `libakstdlib`'s `aksl_ato*` family could not report a conversion + * failure at all, and every assertion below is one that family used to get + * wrong. As of `libakstdlib` 0.2.0 it gets them right, so the wrapper is gone + * and the call sites go straight to the library. + * + * The assertions did not go with it. Four things in this interpreter turn a + * string into a number -- an integer literal, a float literal, a line number in + * the scanner, and `VAL()` at runtime -- and every one of them relies on a bad + * string being *refused* rather than quietly becoming zero. That is a contract + * belonging to another library now, which is exactly why it is worth a test on + * this side: if it ever regresses, `VAL("garbage")` starts returning 0 and + * nothing else in the suite would say so. + * + * Same reasoning `libakstdlib`'s own `tests/test_status_registry.c` gives for + * pinning that it reserves no status range: a contract you depend on and do not + * own is worth asserting where you depend on it. + */ + +#include + +#include + +#include + +#include "testutil.h" + +int main(void) +{ + long long i = 0; + double d = 0.0; + + TEST_REQUIRE_OK(akbasic_error_register()); + + /* The shapes the scanner and the parser actually produce. */ + TEST_REQUIRE_OK(aksl_atoll("1234", &i)); + TEST_REQUIRE_INT(i, 1234); + TEST_REQUIRE_OK(aksl_atoll("-256", &i)); + TEST_REQUIRE_INT(i, -256); + + /* + * Hex goes through aksl_strtoll with an explicit base, because the ato* + * forms are base 10 and would stop at the 'x'. src/grammar.c picks the base + * from the lexeme's prefix and passes it here, so both halves matter: base + * 16 must accept the 0x, and base 8 must read a leading zero as octal -- + * which is the reference's behaviour that TODO.md section 6 item 10 records + * as a defect and section 5 says to reproduce until it is fixed on purpose. + */ + TEST_REQUIRE_OK(aksl_strtoll("0xff", NULL, 16, &i)); + TEST_REQUIRE_INT(i, 255); + TEST_REQUIRE_OK(aksl_strtoll("010", NULL, 8, &i)); + TEST_REQUIRE_INT(i, 8); + + /* + * The refusals. Every one of these returned success with 0 before + * libakstdlib 0.2.0, which is the whole reason src/convert.c existed: a + * silent 0 turns four diagnosable errors into wrong answers. + */ + TEST_REQUIRE_STATUS(aksl_atoll("", &i), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atoll("not a number", &i), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atoll("12abc", &i), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atoll("12 ", &i), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atoll("99999999999999999999999", &i), ERANGE); + + /* A NULL endptr is what makes trailing junk an error rather than a stop. */ + TEST_REQUIRE_STATUS(aksl_strtoll("12abc", NULL, 10, &i), AKERR_VALUE); + + TEST_REQUIRE_STATUS(aksl_atoll(NULL, &i), AKERR_NULLPOINTER); + TEST_REQUIRE_STATUS(aksl_atoll("1", NULL), AKERR_NULLPOINTER); + + /* The float half, which is what VAL() and a float literal both reach. */ + TEST_REQUIRE_OK(aksl_atof("123.456", &d)); + TEST_REQUIRE_FEQ(d, 123.456); + TEST_REQUIRE_OK(aksl_atof("-2.5", &d)); + TEST_REQUIRE_FEQ(d, -2.5); + TEST_REQUIRE_OK(aksl_atof("32", &d)); + TEST_REQUIRE_FEQ(d, 32.0); + + TEST_REQUIRE_STATUS(aksl_atof("", &d), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atof("garbage", &d), AKERR_VALUE); + TEST_REQUIRE_STATUS(aksl_atof("1.5x", &d), AKERR_VALUE); + + /* + * errno hygiene, kept from the wrapper's own tests because the hazard did + * not move: strtoll and strtod do not clear errno on success, so a stale + * ERANGE left by an earlier call must not make a valid conversion raise. + * Mutation testing found this when the wrapper owned it; asserting it here + * is what says libakstdlib carries the same guarantee. + */ + errno = ERANGE; + TEST_REQUIRE_OK(aksl_atoll("42", &i)); + TEST_REQUIRE_INT(i, 42); + errno = ERANGE; + TEST_REQUIRE_OK(aksl_atof("2.5", &d)); + TEST_REQUIRE_FEQ(d, 2.5); + + return akbasic_test_failures; +}