From 32e32c3a4add8ce3f41433305b0d2f97b9b7d356 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:14:11 -0700 Subject: [PATCH 01/27] float reads through format_f64 Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- expected.shcl | 0 project/backlog.md | 6 ++++-- project/conformance/026-init-schema/expected-init.shcl | 6 ++++++ project/conformance/026-init-schema/init-schema.shcl | 9 +++++++++ .../conformance/080-float-shortest/expected-write.shcl | 5 +++++ project/conformance/080-float-shortest/expected.shcl | 5 +++++ project/conformance/080-float-shortest/input.shcl | 5 +++++ project/conformance/080-float-shortest/reads.tsv | 5 +++++ source/rust/src/lib.rs | 8 ++++---- source/rust/src/main.rs | 8 ++++---- source/rust/tests/conformance.rs | 4 ++-- 11 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 expected.shcl diff --git a/expected.shcl b/expected.shcl new file mode 100644 index 0000000..e69de29 diff --git a/project/backlog.md b/project/backlog.md index 32ab8af..7339aa1 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -51,11 +51,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - A fresh adversarial pass started from scratch, aimed at ground no earlier round had read: the C writer, resolver, generator and file tier; the generator bodies in all four bindings; the reference's read, coercion, datetime and validation code judged against the spec rather than against the other bindings; the CLI against its own help and man page; the Go and Python writers and file tiers; the merge code at library level; every gate under fault injection; and the windows builds run under wine. Twenty-eight defects here, seventeen enhancements under Features and enhancements. Everything marked confirmed was reproduced on this box; the two marked plausible rest on vendor documentation or a library-level probe in one binding. - Eighteen of the defects are shapes all four bindings share, so the four-way check cannot see them. Four are C or C++ only. Three are gates or fixtures that assert less than they claim. The fix round before this one (20260901b) cost nothing measurable: parse, fmt, bulk read, bulk write, absent defaults and float writes are unchanged against `b49501d`, and the did-you-mean workload went from 24 s to 0.09 s. A thousand structurally generated documents (bad dedents, fences and `*` lines at bad columns, content beneath skipped lines, bracket arrays, mixed indent) agree four-way and hold every fixpoint and write-gate property, so the E012/E013 change from that round held up. - - 🔘 Item 1: the reference CLI's `get --float` and the Rust generator's annotation line never got the round-half-even rule, so the distributed CLI prints a tie differently from the other three. + - ✅ Item 1: the reference CLI's `get --float` and the Rust generator's annotation line never got the round-half-even rule, so the distributed CLI prints a tie differently from the other three. - Reproduced: `f: 1125899906842624.2` through `get --float` prints `...624.3` from the reference and `...624.2` from Go, Python and C; same on `--array`, and same digit in `init`'s `# float, -` and `one of:` text for a tie-valued bound. The `set` path is right in all four, which is what 20260901b item 10 verified. - Cause: `main.rs` formats reads with `to_string()` at both `Kind::Float` arms, and `allowed_join` and `gen_annotation` in `lib.rs` do the same; `format_f64` is public and unused by either. The crosscheck's float dimension replays `set` only and corpus `080` reads one int, so nothing compared a float read. - - Note: fix all four sites through `format_f64`, then add float reads of the tie values to `080/reads.tsv` so the corpus replay pins the read side too. + - Fixed: every float the reference prints goes through `format_f64` - both CLI read arms, the generator's `one of:` list and its numeric bound line, and the corpus runner's own float rows, which formatted reads the same wrong way. + - Pinned by corpus `080` (four scalar float reads and a three-element array over the tie values, replayed through `get --float` by the crosscheck) and `026` (a float bound pair and a float `allowed` set, both tie-valued, in the generated annotation). Both diverged on the old reference and agree now. - Opened: 20260902-170000 + - Closed: 20260902-181500 - 🔘 Item 2: nothing pins the "counted as lost" half of 20260901b item 3, and the corpus has no way to pin any lost count. - Reproduced: with the one `lost += 1` after the index arm's `E002` deleted in a scratch build, the Rust corpus stays green (33 of 33), the crosscheck on `076` agrees, and `fmt --write` on `a: 1` / `a[0]: 2` exits 0 and deletes the second line. `project/conformance/README.md` and the backlog both say `076` pins it. diff --git a/project/conformance/026-init-schema/expected-init.shcl b/project/conformance/026-init-schema/expected-init.shcl index 92200cb..680d544 100644 --- a/project/conformance/026-init-schema/expected-init.shcl +++ b/project/conformance/026-init-schema/expected-init.shcl @@ -21,6 +21,12 @@ server.host: # int, repeat 1-5 replicas: +# float, 811212085039910.2-1125899906842624.2 +#ratio: + +# float, one of: 811212085039910.2, 1125899906842624.2 +#scale: + # # This config file format is SHCL. # "Simple Hierarchical Config Language" diff --git a/project/conformance/026-init-schema/init-schema.shcl b/project/conformance/026-init-schema/init-schema.shcl index 5ddfb69..edd6b52 100644 --- a/project/conformance/026-init-schema/init-schema.shcl +++ b/project/conformance/026-init-schema/init-schema.shcl @@ -30,3 +30,12 @@ field: "server[*].host" field: replicas type: int repeat: 1, 5 + +field: ratio + type: float + min: 811212085039910.2 + max: 1125899906842624.2 + +field: scale + type: float + allowed: 811212085039910.2, 1125899906842624.2 diff --git a/project/conformance/080-float-shortest/expected-write.shcl b/project/conformance/080-float-shortest/expected-write.shcl index cde4113..e59e1d6 100644 --- a/project/conformance/080-float-shortest/expected-write.shcl +++ b/project/conformance/080-float-shortest/expected-write.shcl @@ -1,4 +1,9 @@ x: 1 +t16: 811212085039910.2 +t17: 1125899906842624.2 +tneg: -0.000000029802322387695312 +p2: 0.00000005960464477539063 +ties: 811212085039910.2, 1125899906842624.2, 0.00000005960464477539063 f0: -1929377421321310.2 diff --git a/project/conformance/080-float-shortest/expected.shcl b/project/conformance/080-float-shortest/expected.shcl index d508cf7..8effb3d 100644 --- a/project/conformance/080-float-shortest/expected.shcl +++ b/project/conformance/080-float-shortest/expected.shcl @@ -1 +1,6 @@ x: 1 +t16: 811212085039910.2 +t17: 1125899906842624.2 +tneg: -0.000000029802322387695312 +p2: 0.00000005960464477539063 +ties: 811212085039910.2, 1125899906842624.2, 0.00000005960464477539063 diff --git a/project/conformance/080-float-shortest/input.shcl b/project/conformance/080-float-shortest/input.shcl index d508cf7..8effb3d 100644 --- a/project/conformance/080-float-shortest/input.shcl +++ b/project/conformance/080-float-shortest/input.shcl @@ -1 +1,6 @@ x: 1 +t16: 811212085039910.2 +t17: 1125899906842624.2 +tneg: -0.000000029802322387695312 +p2: 0.00000005960464477539063 +ties: 811212085039910.2, 1125899906842624.2, 0.00000005960464477539063 diff --git a/project/conformance/080-float-shortest/reads.tsv b/project/conformance/080-float-shortest/reads.tsv index 136bbd9..8cb3512 100644 --- a/project/conformance/080-float-shortest/reads.tsv +++ b/project/conformance/080-float-shortest/reads.tsv @@ -1,3 +1,8 @@ query type expected status - load ok - x int 1 Good +t16 float 811212085039910.2 Good +t17 float 1125899906842624.2 Good +tneg float -0.000000029802322387695312 Good +p2 float 0.00000005960464477539063 Good +ties float[] 811212085039910.2|1125899906842624.2|0.00000005960464477539063 Good diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index d751047..2787cf3 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -5637,7 +5637,7 @@ fn allowed_join(a: &AllowedSet) -> String { .join(", "), AllowedSet::Floats(v) => v .iter() - .map(|x| x.to_string()) + .map(|x| format_f64(*x)) .collect::>() .join(", "), AllowedSet::Bools(v) => v @@ -5668,9 +5668,9 @@ fn gen_annotation(c: &Constraint, tyname: &str) -> String { }); } else if c.min_f.is_some() || c.max_f.is_some() { parts.push(match (c.min_f, c.max_f) { - (Some(lo), Some(hi)) => format!("{}-{}", lo, hi), - (Some(lo), None) => format!(">= {}", lo), - (None, Some(hi)) => format!("<= {}", hi), + (Some(lo), Some(hi)) => format!("{}-{}", format_f64(lo), format_f64(hi)), + (Some(lo), None) => format!(">= {}", format_f64(lo)), + (None, Some(hi)) => format!("<= {}", format_f64(hi)), (None, None) => String::new(), // guarded above; keep the map total }); } diff --git a/source/rust/src/main.rs b/source/rust/src/main.rs index c4c1c28..0d1a686 100644 --- a/source/rust/src/main.rs +++ b/source/rust/src/main.rs @@ -5,8 +5,8 @@ //! so the exit codes and flags below are a stable surface, not conveniences. use shcl::{ - Diagnostic, Document, SaveError, Severity, Status, Strictness, generate, parse_datetime, - suppress_declared_reopens, suppress_declared_repeats, + Diagnostic, Document, SaveError, Severity, Status, Strictness, format_f64, generate, + parse_datetime, suppress_declared_reopens, suppress_declared_repeats, }; use std::process::ExitCode; @@ -804,7 +804,7 @@ fn do_get(o: &Opts) -> u8 { Kind::Float => { let r = doc.read_float_array(path); ( - r.value.iter().map(|v| v.to_string()).collect(), + r.value.iter().map(|v| format_f64(*v)).collect(), r.status, r.slots, ) @@ -842,7 +842,7 @@ fn do_get(o: &Opts) -> u8 { } Kind::Float => { let r = doc.read_float(path); - (vec![r.value.to_string()], r.status, Vec::new()) + (vec![format_f64(r.value)], r.status, Vec::new()) } Kind::Bool => { let r = doc.read_bool(path); diff --git a/source/rust/tests/conformance.rs b/source/rust/tests/conformance.rs index 474c069..dda333f 100644 --- a/source/rust/tests/conformance.rs +++ b/source/rust/tests/conformance.rs @@ -422,7 +422,7 @@ fn reads_match_expected() { } "float" => { let r = doc.read_float(query); - (r.value.to_string(), r.status, r.slots) + (shcl::format_f64(r.value), r.status, r.slots) } "bool" => { let r = doc.read_bool(query); @@ -461,7 +461,7 @@ fn reads_match_expected() { ( r.value .iter() - .map(|v| v.to_string()) + .map(|v| shcl::format_f64(*v)) .collect::>() .join("|"), r.status, From e941a40d91524bec20dd84eafcce849b1ac03f40 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:17:04 -0700 Subject: [PATCH 02/27] pin lost counts in the corpus Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/crosscheck.bash | 11 +++++++++++ project/backlog.md | 6 ++++-- project/conformance/010-mixed-star/reads.tsv | 1 + project/conformance/018-selector-value/reads.tsv | 1 + project/conformance/028-depth-cap/reads.tsv | 1 + project/conformance/031-unterm-quote/reads.tsv | 1 + project/conformance/049-retained-lines/reads.tsv | 1 + project/conformance/057-skipped-parent/reads.tsv | 1 + project/conformance/059-raw-errors/reads.tsv | 1 + project/conformance/060-star-errors/reads.tsv | 1 + project/conformance/061-dedent-error/reads.tsv | 1 + project/conformance/065-bracket-array/reads.tsv | 1 + project/conformance/075-e012-holds-level/reads.tsv | 1 + .../conformance/076-index-selector-value/reads.tsv | 1 + project/conformance/README.md | 2 +- source/c/tests/conformance.c | 5 +++++ source/go/shcl_test.go | 10 ++++++++++ source/python/tests/conformance.py | 4 ++++ source/rust/tests/conformance.rs | 7 +++++++ 19 files changed, 54 insertions(+), 3 deletions(-) diff --git a/cicd/utility/crosscheck.bash b/cicd/utility/crosscheck.bash index 90b4c07..45e30fc 100755 --- a/cicd/utility/crosscheck.bash +++ b/cicd/utility/crosscheck.bash @@ -210,6 +210,11 @@ fFixDangling(){ mkdir -p "$1/real"; ln -s real/c.shcl "$1/c.shcl"; echo "$1/c.sh ## Nothing at the path at all - the create case. The tree it leaves behind is ## the whole point, so the fixture deliberately builds nothing. fFixAbsent(){ echo "$1/c.shcl"; } +## The corpus case's own input, copied in so the write has something real to +## refuse or rewrite. The fixture protocol takes only the root, so the source +## arrives in caseSrc. +caseSrc="" +fFixCase(){ cp "$caseSrc" "$1/c.shcl"; chmod 600 "$1/c.shcl"; echo "$1/c.shcl"; } ## A load that dropped a line canonical output cannot re-emit (a BOM-led one), ## so the in-place write is the destructive case the save gate exists for. fFixLost(){ printf 'a: 1\n\xef\xbb\xbfb: 2\n' >"$1/c.shcl"; chmod 600 "$1/c.shcl"; echo "$1/c.shcl"; } @@ -229,6 +234,7 @@ fReadRow(){ instances) fCompare "instances ${query}" instances "${strictArg[@]}" "$input" "$query" ;; children) fCompare "children ${query}" children "${strictArg[@]}" "$input" "$query" ;; paths) fCompare "paths" paths "${strictArg[@]}" "$input" ;; + lost) : ;; ## no CLI surface; the in-place write below is what it reaches *'[]') fCompare "get ${query} ${type}" get "--${type%[]}" --array "${strictArg[@]}" "$input" "$query" fCompare "get ${query} ${type} slots" get "--${type%[]}" --array --slots "${strictArg[@]}" "$input" "$query" ;; *) fCompare "get ${query} ${type}" get "--${type}" "${strictArg[@]}" "$input" "$query" @@ -256,6 +262,11 @@ for caseDir in "$corpus"/*/; do continue fi fCompare "fmt ${caseName}" fmt "$input" + # The save gate over real inputs: a case whose load dropped something must + # refuse the in-place write and leave the file byte-identical, and one that + # dropped nothing must rewrite it. Nothing else replays a corpus input + # through --write, so a lost count that stops being kept goes unseen. + caseSrc="$input"; fCompareWrite "fmt --write ${caseName}" fFixCase fmt --write # Write dimension: apply the case's ops script and compare canonical output. ops="${caseDir}write.ops" [[ -f "$ops" ]] && fCompareStdin "set ${caseName}" "$ops" set "$input" diff --git a/project/backlog.md b/project/backlog.md index 7339aa1..862608d 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -59,11 +59,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170000 - Closed: 20260902-181500 - - 🔘 Item 2: nothing pins the "counted as lost" half of 20260901b item 3, and the corpus has no way to pin any lost count. + - ✅ Item 2: nothing pins the "counted as lost" half of 20260901b item 3, and the corpus has no way to pin any lost count. - Reproduced: with the one `lost += 1` after the index arm's `E002` deleted in a scratch build, the Rust corpus stays green (33 of 33), the crosscheck on `076` agrees, and `fmt --write` on `a: 1` / `a[0]: 2` exits 0 and deletes the second line. `project/conformance/README.md` and the backlog both say `076` pins it. - Cause: `expected-diags.txt` pins codes only, the write dimension exercises the save gate through its own BOM fixture and never through a corpus input, and the four runners assert `lost_count()` only in hand-written fixtures, none for the selector shape. The same gap covers every other "counted as lost" rule (items 1 and 2 of that round included). - - Note: give the corpus a lost column (a `lost N` line in `expected-diags.txt`, or a pseudo-row in `reads.tsv`) that all four runners assert, and run `fmt --write` over every corpus input with error diagnostics in the crosscheck's write dimension, where a refusal leaves the tree byte-identical. + - Fixed: `reads.tsv` takes a `lost` pseudo-call, asserted by all four runners, and every case that drops content carries one; two cases that report errors while losing nothing carry a zero. The crosscheck now copies each corpus input into a fresh tree and runs `fmt --write` over it, comparing the exit code and the resulting bytes across the four, so the save gate is exercised by real inputs rather than by one hand-built fixture. + - Pinned by the ten nonzero `lost` rows plus the per-case write comparison. With the increment deleted, the corpus fails on `076` and the crosscheck reports the file rewritten with two lines gone at exit 0. - Opened: 20260902-170100 + - Closed: 20260902-183000 - 🔘 Item 3: a wildcard followed by another wildcard reports every slot `Multiple`, and `Remove` on it removes nothing. - Reproduced in all four. `server[*].*` on three servers with two, one and no children gives three `Multiple` slots at exit 5, `count` says 3, `instances` prints three empty lines, and `set --remove='server[*].*'` leaves the document unchanged at exit 0. The spec says the two compose and that `Remove` on a wildcard removes every resolved slot. Same for `*.*` and `a[*].b[*]`. diff --git a/project/conformance/010-mixed-star/reads.tsv b/project/conformance/010-mixed-star/reads.tsv index 14c4aa5..8007e1a 100644 --- a/project/conformance/010-mixed-star/reads.tsv +++ b/project/conformance/010-mixed-star/reads.tsv @@ -5,3 +5,4 @@ sizes string small Good sizes.color string red Good shape string - Empty shape.side int 4 Good +- lost 2 - diff --git a/project/conformance/018-selector-value/reads.tsv b/project/conformance/018-selector-value/reads.tsv index 34f8de5..a4eedd5 100644 --- a/project/conformance/018-selector-value/reads.tsv +++ b/project/conformance/018-selector-value/reads.tsv @@ -3,3 +3,4 @@ query type expected status level - load fail - strict city count 2 - city instances Portland|Boston - +- lost 1 - diff --git a/project/conformance/028-depth-cap/reads.tsv b/project/conformance/028-depth-cap/reads.tsv index 7bc7c4b..63eb95c 100644 --- a/project/conformance/028-depth-cap/reads.tsv +++ b/project/conformance/028-depth-cap/reads.tsv @@ -3,3 +3,4 @@ ok int 1 Good a0 string - NotFound - load ok - - load fail - strict +- lost 1 - diff --git a/project/conformance/031-unterm-quote/reads.tsv b/project/conformance/031-unterm-quote/reads.tsv index f1b14a2..1b626c0 100644 --- a/project/conformance/031-unterm-quote/reads.tsv +++ b/project/conformance/031-unterm-quote/reads.tsv @@ -2,3 +2,4 @@ query type expected status level ok int 1 Good - load ok - - load fail - strict +- lost 0 - diff --git a/project/conformance/049-retained-lines/reads.tsv b/project/conformance/049-retained-lines/reads.tsv index 634a295..a9c680e 100644 --- a/project/conformance/049-retained-lines/reads.tsv +++ b/project/conformance/049-retained-lines/reads.tsv @@ -2,3 +2,4 @@ query type expected status level - load ok - server.host string h1 Good count int 2 Good +- lost 0 - diff --git a/project/conformance/057-skipped-parent/reads.tsv b/project/conformance/057-skipped-parent/reads.tsv index 10312ff..3b45ff2 100644 --- a/project/conformance/057-skipped-parent/reads.tsv +++ b/project/conformance/057-skipped-parent/reads.tsv @@ -12,3 +12,4 @@ a.ll count 0 Good a.m count 1 Good a children d|m - - paths a|a.d|a.d.e|a.m - +- lost 5 - diff --git a/project/conformance/059-raw-errors/reads.tsv b/project/conformance/059-raw-errors/reads.tsv index 03b97ee..f136a96 100644 --- a/project/conformance/059-raw-errors/reads.tsv +++ b/project/conformance/059-raw-errors/reads.tsv @@ -2,3 +2,4 @@ query type expected status - load ok - - load fail - strict a raw never closed\n Good +- lost 1 - diff --git a/project/conformance/060-star-errors/reads.tsv b/project/conformance/060-star-errors/reads.tsv index 9d88472..b88ebfb 100644 --- a/project/conformance/060-star-errors/reads.tsv +++ b/project/conformance/060-star-errors/reads.tsv @@ -3,3 +3,4 @@ query type expected status - load fail - strict list string[] a|d Good val int 1 Good +- lost 4 - diff --git a/project/conformance/061-dedent-error/reads.tsv b/project/conformance/061-dedent-error/reads.tsv index 7b1aa73..841b302 100644 --- a/project/conformance/061-dedent-error/reads.tsv +++ b/project/conformance/061-dedent-error/reads.tsv @@ -5,3 +5,4 @@ a.b.c int 1 Good a.e int 3 Good a.d int 0 NotFound a.b.d int 0 NotFound +- lost 1 - diff --git a/project/conformance/065-bracket-array/reads.tsv b/project/conformance/065-bracket-array/reads.tsv index 2c5a01b..2207fdd 100644 --- a/project/conformance/065-bracket-array/reads.tsv +++ b/project/conformance/065-bracket-array/reads.tsv @@ -1,3 +1,4 @@ query type expected status - load ok - ports string 80, 443 Good +- lost 1 - diff --git a/project/conformance/075-e012-holds-level/reads.tsv b/project/conformance/075-e012-holds-level/reads.tsv index 69a0a9f..3626ec0 100644 --- a/project/conformance/075-e012-holds-level/reads.tsv +++ b/project/conformance/075-e012-holds-level/reads.tsv @@ -14,3 +14,4 @@ p.j count 0 Good a children b - p children q - - paths a|a.b|p|p.q|z - +- lost 7 - diff --git a/project/conformance/076-index-selector-value/reads.tsv b/project/conformance/076-index-selector-value/reads.tsv index 347ed48..b44b3ee 100644 --- a/project/conformance/076-index-selector-value/reads.tsv +++ b/project/conformance/076-index-selector-value/reads.tsv @@ -5,3 +5,4 @@ a int 1 Good a count 1 Good c.d int 4 Good - paths a|c|c.d - +- lost 2 - diff --git a/project/conformance/README.md b/project/conformance/README.md index 495122c..fc155b6 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -11,7 +11,7 @@ Each case is a directory `NNN-short-name/` containing: - `expected.shcl` - the canonical formatter output for that input (block form, tabs, insertion order, minimal quoting, redundancy collapsed), at Standard strictness. -- `reads.tsv` - expected typed reads. Columns, tab-separated: `query` `type` `expected` `status` `[level]` `[slots]`. `type` uses `int|float|bool|datetime|string|raw|rawinfo` and `[]` for array forms (except `raw`/`rawinfo`, which have no array form), or the pseudo-calls `count`/`instances`/`load`. `rawinfo` reads a raw block's info-string (the fence tag) rather than its content. `expected` is the value (`-` when not applicable); `status` is one of `Good|Empty|NotFound|BadType|Multiple`. The optional fifth column is the strictness level (`loose|standard|strict`), default `standard`. The optional sixth column (requires the fifth) pins the per-slot statuses of an array read, `|`-joined in slot order; the row's `status` is then the worst slot. The `load` pseudo-call asserts whether the document loads at that level: query `-`, expected `ok` or `fail`, status `-`. In `expected`, a newline inside a raw-block value is written `\n` (a literal newline or tab would break the TSV). +- `reads.tsv` - expected typed reads. Columns, tab-separated: `query` `type` `expected` `status` `[level]` `[slots]`. `type` uses `int|float|bool|datetime|string|raw|rawinfo` and `[]` for array forms (except `raw`/`rawinfo`, which have no array form), or the pseudo-calls `count`/`instances`/`load`/`lost`. `rawinfo` reads a raw block's info-string (the fence tag) rather than its content. `expected` is the value (`-` when not applicable); `status` is one of `Good|Empty|NotFound|BadType|Multiple`. The optional fifth column is the strictness level (`loose|standard|strict`), default `standard`. The optional sixth column (requires the fifth) pins the per-slot statuses of an array read, `|`-joined in slot order; the row's `status` is then the worst slot. The `load` pseudo-call asserts whether the document loads at that level: query `-`, expected `ok` or `fail`, status `-`. The `lost` pseudo-call asserts the document's lost count: query `-`, expected the number, status `-`. In `expected`, a newline inside a raw-block value is written `\n` (a literal newline or tab would break the TSV). - `expected-diags.txt` - the diagnostic golden (required): the exact `check` stdout at Standard strictness - one `line N: Severity: CODE` line per diagnostic in emission order, then the summary line (`ok (N diagnostic(s))`, or `failed: N diagnostic(s), M error(s)` when errors are present). Pins count, line, severity, and stable code per case, including the mandatory repeated-leaf hint (`H001`) and the zero-diagnostic cases. diff --git a/source/c/tests/conformance.c b/source/c/tests/conformance.c index 991cd06..0aabba8 100644 --- a/source/c/tests/conformance.c +++ b/source/c/tests/conformance.c @@ -427,6 +427,11 @@ int main(int argc, char **argv) { } shcl_doc *rd = shcl_parse_with(input, ilen, level); if (shcl_strict_failed(rd)) { fail(at, "load failed but reads.tsv has reads there"); shcl_free(rd); continue; } + if (!strcmp(kind, "lost")) { + char nb[32]; snprintf(nb, sizeof nb, "%zu", shcl_lost_count(rd)); + if (strcmp(nb, exp)) fail(at, "lost mismatch"); + shcl_free(rd); continue; + } if (!strcmp(kind, "count")) { char nb[32]; snprintf(nb, sizeof nb, "%zu", shcl_count(rd, query, qn)); if (strcmp(nb, exp)) fail(at, "count mismatch"); diff --git a/source/go/shcl_test.go b/source/go/shcl_test.go index f7c9829..f014cc1 100644 --- a/source/go/shcl_test.go +++ b/source/go/shcl_test.go @@ -1653,6 +1653,16 @@ func TestReadsMatchExpected(t *testing.T) { } doc := docFor(t, &c, level) + if kind == "lost" { + want, err := strconv.Atoi(expected) + if err != nil { + t.Fatalf("%s: bad lost count", at) + } + if got := doc.LostCount(); got != want { + t.Errorf("%s: lost: got %d want %d", at, got, want) + } + continue + } if kind == "count" { want, err := strconv.Atoi(expected) if err != nil { diff --git a/source/python/tests/conformance.py b/source/python/tests/conformance.py index 6126cec..903dcaf 100644 --- a/source/python/tests/conformance.py +++ b/source/python/tests/conformance.py @@ -493,6 +493,10 @@ def main(): fails.append(f"{at}: load failed but reads.tsv has reads there: {e}") continue + if kind == "lost": + if str(doc.lost_count()) != expected: + fails.append(f"{at}: lost got {doc.lost_count()} want {expected}") + continue if kind == "count": if str(doc.count(query)) != expected: fails.append(f"{at}: count got {doc.count(query)} want {expected}") diff --git a/source/rust/tests/conformance.rs b/source/rust/tests/conformance.rs index dda333f..eda9dcb 100644 --- a/source/rust/tests/conformance.rs +++ b/source/rust/tests/conformance.rs @@ -391,6 +391,13 @@ fn reads_match_expected() { } let doc = doc_for(&case, level); + if kind == "lost" { + let want: usize = expected + .parse() + .unwrap_or_else(|_| panic!("{}: bad lost count", at)); + assert_eq!(doc.lost_count(), want, "{}", at); + continue; + } if kind == "count" { let want: usize = expected .parse() From b14d037bdbdd4b0f956e69913e581267a4f57d6a Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:22:04 -0700 Subject: [PATCH 03/27] nested wildcards flatten Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 + project/backlog.md | 7 +- .../081-wildcard-compose/expected-diags.txt | 2 + .../081-wildcard-compose/expected-write.shcl | 8 ++ .../081-wildcard-compose/expected.shcl | 11 +++ .../081-wildcard-compose/input.shcl | 11 +++ .../081-wildcard-compose/reads.tsv | 10 +++ .../081-wildcard-compose/write.ops | 1 + project/design.md | 2 + project/spec.md | 2 +- source/c/shcl.h | 14 ++++ source/go/shcl.go | 10 +++ source/python/shcl.py | 82 ++++++++++++------- source/rust/src/lib.rs | 8 ++ 14 files changed, 137 insertions(+), 33 deletions(-) create mode 100644 project/conformance/081-wildcard-compose/expected-diags.txt create mode 100644 project/conformance/081-wildcard-compose/expected-write.shcl create mode 100644 project/conformance/081-wildcard-compose/expected.shcl create mode 100644 project/conformance/081-wildcard-compose/input.shcl create mode 100644 project/conformance/081-wildcard-compose/reads.tsv create mode 100644 project/conformance/081-wildcard-compose/write.ops diff --git a/changelog.md b/changelog.md index 4fd2f08..8ea07bf 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- A wildcard after a wildcard flattens instead of answering `Multiple` for every slot. `server[*].*` reported one unreadable slot per instance, `count` counted instances rather than leaves, and `Remove` on such a path removed nothing. The inner slots now join the outer run, so the result is one slot per resolved leaf and the two wildcards compose the way the spec says they do. + - A float literal past the double range (`1e400`) reads as `BadType` instead of an infinity at `Good`. No double holds the value, and the infinity could not be written back, so a read-modify-write left a field the reader then refused. A literal below the range still reads as zero. - `get`, `count` and `instances` print the load's diagnostics to stderr, the way `fmt` and `set` already did. Below strict a damaged file used to read back a correct value at exit 0 with nothing said, so the only way to learn a line had been dropped was a separate `check` run. One report per run; stdout is unchanged. diff --git a/project/backlog.md b/project/backlog.md index 862608d..526c6c2 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -67,11 +67,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170100 - Closed: 20260902-183000 - - 🔘 Item 3: a wildcard followed by another wildcard reports every slot `Multiple`, and `Remove` on it removes nothing. + - ✅ Item 3: a wildcard followed by another wildcard reports every slot `Multiple`, and `Remove` on it removes nothing. - Reproduced in all four. `server[*].*` on three servers with two, one and no children gives three `Multiple` slots at exit 5, `count` says 3, `instances` prints three empty lines, and `set --remove='server[*].*'` leaves the document unchanged at exit 0. The spec says the two compose and that `Remove` on a wildcard removes every resolved slot. Same for `*.*` and `a[*].b[*]`. - Cause: `resolve_from` resolves the rest of the path per instance and maps anything that is not exactly one node to `Multiple`, so a nested slot list is `Multiple` whatever its length. Rust `lib.rs` both wildcard arms, Go `resolveFrom`, Python `_resolve_from`, C `resolve_from`. - - Decided: needs a shape first. Minimal: a nested list of zero is `NotFound` and of one is that slot. Full: flatten nested lists so `count`, `instances`, `lines` and the read stay aligned. Either way `Remove` then removes the resolved leaves. No corpus case has a wildcard after a wildcard. + - Decided: flatten. The inner slots join the outer run, so the result is one slot per resolved leaf and `count`, `instances`, the read and `Remove` all see the same list. A repeated leaf under one instance still reports `Multiple`, since that is the same ambiguity a path with no wildcard has. Reasoning in `design.md` -> open sections; the spec's compose sentence now says so, and there is a changelog line under Changed. + - Fixed: the two wildcard arms in all four splice a nested slot list into the outer one. Python's flat sub-walk grew an explicit stack so it can widen the run instead of ending it there; the other three had a recursive call already. + - Pinned by corpus `081` (three instances with two, one and no children, plus a repeated leaf and a `*.port` read whose slots straddle two parents; the write dimension removes `server[*].*`). All four failed it before on the reads, the instance list and the removal, and pass now. - Opened: 20260902-170200 + - Closed: 20260902-190000 - 🔘 Item 4: a wildcard read over a parent that does not exist reports `Empty` where the path does not resolve. - Reproduced in all four. On a document with no `x`, `get --int --array x[*]` exits 2 and `get --int x` exits 3. The spec defines `Empty` as present but no value, and this is the tri-state the spec advertises. diff --git a/project/conformance/081-wildcard-compose/expected-diags.txt b/project/conformance/081-wildcard-compose/expected-diags.txt new file mode 100644 index 0000000..c28e204 --- /dev/null +++ b/project/conformance/081-wildcard-compose/expected-diags.txt @@ -0,0 +1,2 @@ +line 11: Hint: H001 +ok (1 diagnostic(s)) diff --git a/project/conformance/081-wildcard-compose/expected-write.shcl b/project/conformance/081-wildcard-compose/expected-write.shcl new file mode 100644 index 0000000..107e78f --- /dev/null +++ b/project/conformance/081-wildcard-compose/expected-write.shcl @@ -0,0 +1,8 @@ +# Two wildcards in one path: every child of every instance. +server: alpha +server: beta +server: gamma + +pool: p + a: 1 + a: 2 diff --git a/project/conformance/081-wildcard-compose/expected.shcl b/project/conformance/081-wildcard-compose/expected.shcl new file mode 100644 index 0000000..5bdda6e --- /dev/null +++ b/project/conformance/081-wildcard-compose/expected.shcl @@ -0,0 +1,11 @@ +# Two wildcards in one path: every child of every instance. +server: alpha + port: 8080 + host: a.example +server: beta + port: 9090 +server: gamma + +pool: p + a: 1 + a: 2 diff --git a/project/conformance/081-wildcard-compose/input.shcl b/project/conformance/081-wildcard-compose/input.shcl new file mode 100644 index 0000000..5bdda6e --- /dev/null +++ b/project/conformance/081-wildcard-compose/input.shcl @@ -0,0 +1,11 @@ +# Two wildcards in one path: every child of every instance. +server: alpha + port: 8080 + host: a.example +server: beta + port: 9090 +server: gamma + +pool: p + a: 1 + a: 2 diff --git a/project/conformance/081-wildcard-compose/reads.tsv b/project/conformance/081-wildcard-compose/reads.tsv new file mode 100644 index 0000000..1e27f08 --- /dev/null +++ b/project/conformance/081-wildcard-compose/reads.tsv @@ -0,0 +1,10 @@ +query type expected status +- load ok - +server[*].* string[] 8080|a.example|9090 Good +server[*].* count 3 - +server[*].* instances 8080|a.example|9090 - +pool[*].* int[] 1|2 Good +*.port int[] 8080|9090|0|0 NotFound standard Good|Good|NotFound|NotFound +*.port count 4 - +server[*].*[0] string[] NotFound +- lost 0 - diff --git a/project/conformance/081-wildcard-compose/write.ops b/project/conformance/081-wildcard-compose/write.ops new file mode 100644 index 0000000..1c7aa22 --- /dev/null +++ b/project/conformance/081-wildcard-compose/write.ops @@ -0,0 +1 @@ +remove server[*].* diff --git a/project/design.md b/project/design.md index fb80fa1..c414384 100644 --- a/project/design.md +++ b/project/design.md @@ -263,6 +263,8 @@ Both open points are settled: - Suggestions ("did you mean") do not reach below a `*`, since there is no fixed sibling list to suggest from, and a `repeat` on a `*` leaf disavows no `H001`. +- A wildcard after a wildcard flattens. `server[*].*` used to report every slot `Multiple`, because the per-instance sub-resolution collapsed anything that was not exactly one node, and a nested slot list is never one node - so `Remove` on such a path removed nothing and a read of it could not answer. Among the options (call a nested list of one that slot and a nested list of none `NotFound`, or splice the inner slots into the outer run), splicing was decided: it is what "the two compose" already promised, it keeps `count`, `instances` and the read aligned with each other, and it makes the result one slot per resolved leaf, which is what a caller writing `server[*].*` is asking for. A repeated leaf under one instance stays `Multiple`, since that ambiguity is the same one a path with no wildcard has. + **A quoted by-value selector is scalar-only.** By-value matching is against the display form, and an inline array's display joins elements with `, ` - so the scalar `"a, b"` and the list `a, b` met the same selector and a read could only answer Multiple. - Among the candidates (a scalar-only quoted spelling, an IndexOf companion, leaving it documented) we decided the quoted spelling wins: quoting already forces a value match over an index and already escapes sentinels and formats elsewhere this round, so "quoted selects the scalar spelling only" extends one rule instead of adding a second construct. diff --git a/project/spec.md b/project/spec.md index e900581..e7f82a9 100644 --- a/project/spec.md +++ b/project/spec.md @@ -392,7 +392,7 @@ Materialization is idempotent and order-stable, so two traversals of the same do - `Instances` on a wildcard path keeps unresolved slots in the enumeration as empty strings, preserving index alignment with the read and with `Count` (which counts slots). - The libraries substitute nothing per slot: the convenience tier's fallback is the whole array, returned only when the read is `Good`. Per-slot substitution is the CLI's `--default`, because a shell caller has no slot list to inspect. -- A bare `*` in **name position** (`indicators.*.period`) is the name wildcard: it matches every child field regardless of name, one slot per child in file order, with the same per-slot status, `Instances`, `Count`, and per-slot substitution behavior as `field[*]` above. The two compose (`server[*].*`, `*.port`), and `Remove` on a wildcard path removes every resolved slot. A name wildcard takes no selector of its own (`*[x]` is not a path), and it is a query construct only: a binding line never accepts it, the Writer's setters refuse it (`WriteReason` = `Wildcard`), and a field literally named `*` is written and addressed quoted (`"*"`), which stays a literal name everywhere. +- A bare `*` in **name position** (`indicators.*.period`) is the name wildcard: it matches every child field regardless of name, one slot per child in file order, with the same per-slot status, `Instances`, `Count`, and per-slot substitution behavior as `field[*]` above. The two compose (`server[*].*`, `*.port`): a wildcard after a wildcard flattens, so the inner slots join the outer run and the result is one slot per resolved leaf rather than one per outer instance. `Remove` on a wildcard path removes every resolved slot. A name wildcard takes no selector of its own (`*[x]` is not a path), and it is a query construct only: a binding line never accepts it, the Writer's setters refuse it (`WriteReason` = `Wildcard`), and a field literally named `*` is written and addressed quoted (`"*"`), which stays a literal name everywhere. - `Instances(field)` and `Count(field)` enumerate instances by value or index. diff --git a/source/c/shcl.h b/source/c/shcl.h index daa8634..eee5f16 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -2852,6 +2852,13 @@ static ShclResolved resolve_from(shcl_doc *d, const size_t *start, size_t nstart else { size_t inst = next.data[k]; ShclResolved r = resolve_from(d, &inst, 1, rest, nrest); if (r.kind == R_ONE) { sl.present = 1; sl.idx = r.one; } + else if (r.kind == R_SLOTS) { + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + for (size_t j = 0; j < r.slots.len; j++) ShclVecSlot_push(a, &slots, r.slots.data[j]); + continue; + } else if (r.kind != R_NONE) sl.miss = SHCL_MULTIPLE; } ShclVecSlot_push(a, &slots, sl); @@ -2881,6 +2888,13 @@ static ShclResolved resolve_from(shcl_doc *d, const size_t *start, size_t nstart else { size_t inst = next.data[k]; ShclResolved r = resolve_from(d, &inst, 1, rest, nrest); if (r.kind == R_ONE) { sl.present = 1; sl.idx = r.one; } + else if (r.kind == R_SLOTS) { + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + for (size_t j = 0; j < r.slots.len; j++) ShclVecSlot_push(a, &slots, r.slots.data[j]); + continue; + } else if (r.kind != R_NONE) sl.miss = SHCL_MULTIPLE; } ShclVecSlot_push(a, &slots, sl); diff --git a/source/go/shcl.go b/source/go/shcl.go index 31ea3c6..3905134 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -3414,6 +3414,11 @@ func (d *Document) resolveFrom(start []int, segs []segment) resolved { slots = append(slots, r.one) case resNone: slots = append(slots, -1) + case resSlots: + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + slots = append(slots, r.slots...) default: slots = append(slots, -2) } @@ -3453,6 +3458,11 @@ func (d *Document) resolveFrom(start []int, segs []segment) resolved { slots = append(slots, r.one) case resNone: slots = append(slots, -1) + case resSlots: + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + slots = append(slots, r.slots...) default: slots = append(slots, -2) } diff --git a/source/python/shcl.py b/source/python/shcl.py index e18515e..20b8dac 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -2275,11 +2275,10 @@ def _resolve_from(self, start, segs): # A slots entry is a node idx, or the Status saying why the sub-path did # not land on one node (NotFound missing, Multiple ambiguous). # The per-instance sub-resolution behind a wildcard is the same walk - # over the remaining segments, run flat (_resolve_slot) rather than one + # over the remaining segments, run flat (_resolve_slots) rather than one # frame per wildcard: a path can carry a wildcard per document level, - # and the frame budget is small. The sub-walk's answer collapses to one - # slot, and a wildcard inside it can only ever answer Multiple, so the - # flat walk ends there. + # and the frame budget is small. A wildcard inside the sub-walk widens + # the run rather than ending it, so the two compose. cur = list(start) for i, seg in enumerate(segs): nxt = [] @@ -2296,7 +2295,7 @@ def _resolve_from(self, start, segs): if not rest: slots.append(inst) else: - slots.append(self._resolve_slot(inst, rest)) + slots.extend(self._resolve_slots(inst, rest)) return ("slots", slots) sel = seg.selector if sel is None: @@ -2315,7 +2314,7 @@ def _resolve_from(self, start, segs): if not rest: slots.append(inst) else: - slots.append(self._resolve_slot(inst, rest)) + slots.extend(self._resolve_slots(inst, rest)) return ("slots", slots) if len(cur) == 0: return ("none",) @@ -2323,31 +2322,54 @@ def _resolve_from(self, start, segs): return ("one", cur[0]) return ("many", cur) - def _resolve_slot(self, inst, rest): - # One wildcard slot: _resolve_from's walk from `inst` over `rest`, - # collapsed to a node index, NotFound (nothing) or Multiple (several, or - # a further wildcard - whose per-instance answer is itself a list). - cur = [inst] - for seg in rest: - sel = seg.selector - if seg.star or (sel is not None and sel[0] == "wild"): - return Status.Multiple - nxt = [] - for node in cur: - nxt.extend(self._children_named(node, seg.name)) - if sel is None: - cur = nxt - elif sel[0] == "val": - want = _apply_escapes(sel[1]) - cur = [c for c in nxt if _disp_key(self.arena[c].value) == want and (not sel[2] or _single_scalar(self.arena[c].value))] + def _resolve_slots(self, inst, rest): + # The slots one wildcard instance contributes: normally one - a node + # index, or the Status saying why the sub-path did not land on one node + # (NotFound missing, Multiple ambiguous) - but a further wildcard in + # `rest` contributes its own slots to the same flat run. Walked with an + # explicit stack rather than one frame per wildcard: a path can carry a + # wildcard per document level, and the frame budget is small. The stack + # is depth-first with children pushed in reverse, so slots come out in + # file order. + out = [] + stack = [([inst], rest)] + while stack: + cur, segs = stack.pop() + split = False + for i, seg in enumerate(segs): + sel = seg.selector + nxt = [] + for node in cur: + if seg.star: + nxt.extend(self.arena[node].children) + else: + nxt.extend(self._children_named(node, seg.name)) + if seg.star or (sel is not None and sel[0] == "wild"): + tail = segs[i + 1:] + if not tail: + out.extend(nxt) + else: + for c in reversed(nxt): + stack.append(([c], tail)) + split = True + break + if sel is None: + cur = nxt + elif sel[0] == "val": + want = _apply_escapes(sel[1]) + cur = [c for c in nxt if _disp_key(self.arena[c].value) == want and (not sel[2] or _single_scalar(self.arena[c].value))] + else: + k = sel[1] + cur = [nxt[k]] if k < len(nxt) else [] + if split: + continue + if len(cur) == 0: + out.append(Status.NotFound) + elif len(cur) == 1: + out.append(cur[0]) else: - k = sel[1] - cur = [nxt[k]] if k < len(nxt) else [] - if len(cur) == 0: - return Status.NotFound - if len(cur) == 1: - return cur[0] - return Status.Multiple + out.append(Status.Multiple) + return out def _resolve(self, path): # Returns a _resolve_from result, or ("err", Status). diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 2787cf3..6a4cde8 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -3275,6 +3275,10 @@ impl Document { match self.resolve_from(&[inst], rest) { Resolved::One(x) => slots.push(Ok(x)), Resolved::None => slots.push(Err(Status::NotFound)), + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + Resolved::Slots(inner) => slots.extend(inner), _ => slots.push(Err(Status::Multiple)), } } @@ -3310,6 +3314,10 @@ impl Document { match self.resolve_from(&[inst], rest) { Resolved::One(x) => slots.push(Ok(x)), Resolved::None => slots.push(Err(Status::NotFound)), + // A wildcard after a wildcard: the inner slots join the + // outer list, so the two compose into one flat run of + // leaves rather than one unreadable slot. + Resolved::Slots(inner) => slots.extend(inner), _ => slots.push(Err(Status::Multiple)), } } From 725cd92fd5b49c04cb80f5b6e7a1a84ae9ca95f1 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:24:17 -0700 Subject: [PATCH 04/27] empty wildcard is NotFound Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ project/backlog.md | 6 ++++-- project/conformance/081-wildcard-compose/reads.tsv | 3 +++ source/c/shcl.h | 4 +++- source/go/shcl.go | 4 +++- source/python/shcl.py | 4 +++- source/rust/src/lib.rs | 4 +++- 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/changelog.md b/changelog.md index 8ea07bf..ef35e23 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- A wildcard read whose parent does not exist reports `NotFound` instead of `Empty`. `x[*]` on a document with no `x` said the path was there and empty, which is the answer for a field written with nothing after the colon; `x` on its own said `NotFound`. The two agree now. + - A wildcard after a wildcard flattens instead of answering `Multiple` for every slot. `server[*].*` reported one unreadable slot per instance, `count` counted instances rather than leaves, and `Remove` on such a path removed nothing. The inner slots now join the outer run, so the result is one slot per resolved leaf and the two wildcards compose the way the spec says they do. - A float literal past the double range (`1e400`) reads as `BadType` instead of an infinity at `Good`. No double holds the value, and the infinity could not be written back, so a read-modify-write left a field the reader then refused. A literal below the range still reads as zero. diff --git a/project/backlog.md b/project/backlog.md index 526c6c2..9384ffb 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -76,11 +76,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170200 - Closed: 20260902-190000 - - 🔘 Item 4: a wildcard read over a parent that does not exist reports `Empty` where the path does not resolve. + - ✅ Item 4: a wildcard read over a parent that does not exist reports `Empty` where the path does not resolve. - Reproduced in all four. On a document with no `x`, `get --int --array x[*]` exits 2 and `get --int x` exits 3. The spec defines `Empty` as present but no value, and this is the tri-state the spec advertises. - Cause: the array read maps an empty slot list to `Empty` in all four (Rust `read_array`, Go `readArray`, Python `_read_array`, C `array_elements`). - - Note: an empty slot list is `NotFound`. No corpus row has a wildcard read with zero slots. + - Fixed: an empty slot list is `NotFound` in all four, so a wildcard whose parent is absent answers the same as the bare path. + - Pinned by corpus `081` (`nosuch[*]` and `nosuch[*].deeper`, plus a count of zero). All four reported `Empty` before and `NotFound` now. - Opened: 20260902-170300 + - Closed: 20260902-191500 - 🔘 Item 5: a valued line generated from a filled wildcard is not in the generator's parent-value map, so its dotted child names the empty instance and a satisfiable schema refuses to generate. - Reproduced in all four. `field: a` required, `field: "a[*].b"` required with `default: bee`, `field: "a[*].b[*].c"` required: `init` exits 6 with `V097 required path missing: a[*].b[*].c`. The text behind the refusal is `a:` / `a.b: bee` / `a.b.c:`; the hand-corrected `a.b[bee].c:` validates clean. Same shape through a fragment mount under a valued parent. diff --git a/project/conformance/081-wildcard-compose/reads.tsv b/project/conformance/081-wildcard-compose/reads.tsv index 1e27f08..ecf9b3a 100644 --- a/project/conformance/081-wildcard-compose/reads.tsv +++ b/project/conformance/081-wildcard-compose/reads.tsv @@ -7,4 +7,7 @@ pool[*].* int[] 1|2 Good *.port int[] 8080|9090|0|0 NotFound standard Good|Good|NotFound|NotFound *.port count 4 - server[*].*[0] string[] NotFound +nosuch[*] int[] NotFound +nosuch[*].deeper string[] NotFound +nosuch[*] count 0 - - lost 0 - diff --git a/source/c/shcl.h b/source/c/shcl.h index eee5f16..ebb6cb1 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -2959,7 +2959,9 @@ static shcl_status array_elements(shcl_doc *d, ShclArena *a, ShclStr path, ShclE else if (v->kind == V_CELL && v->nels == 1) { arr[i] = &v->els[0]; st[i] = SHCL_GOOD; } else st[i] = SHCL_BAD_TYPE; // raw block, or an array is not one scalar } - *els = arr; *sts = st; *n = m; return m == 0 ? SHCL_EMPTY : SHCL_GOOD; + // No slots at all means the wildcard's parent is not there, so the + // path did not resolve - Empty is for a node that is. + *els = arr; *sts = st; *n = m; return m == 0 ? SHCL_NOT_FOUND : SHCL_GOOD; } if (r.kind == R_NONE) return SHCL_NOT_FOUND; if (r.kind == R_MANY) return SHCL_MULTIPLE; diff --git a/source/go/shcl.go b/source/go/shcl.go index 3905134..bb37926 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -5233,7 +5233,9 @@ func readArray[T any](d *Document, path string, coerce func(*element) (T, bool)) out = append(out, val) sts = append(sts, cst) } - status := Empty + // No slots at all means the wildcard's parent is not there, so the + // path did not resolve - Empty is for a node that is. + status := NotFound if len(sts) > 0 { status = Good for _, s := range sts { diff --git a/source/python/shcl.py b/source/python/shcl.py index 20b8dac..71fdbdc 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -3219,7 +3219,9 @@ def _read_array(self, path, coerce, default): v, st = _coerced(coerce, se[1], default) out.append(v) sts.append(st) - status = max(sts, key=lambda s: s.value) if sts else Status.Empty + # No slots at all means the wildcard's parent is not there, so the + # path did not resolve - Empty is for a node that is. + status = max(sts, key=lambda s: s.value) if sts else Status.NotFound return Read(out, status, None, sts) if tag == "none": return Read([], Status.NotFound, None) diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 6a4cde8..285caba 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -4894,8 +4894,10 @@ impl Document { }, } } + // No slots at all means the wildcard's parent is not there, so + // the path did not resolve - Empty is for a node that is. let status = if sts.is_empty() { - Status::Empty + Status::NotFound } else { sts.iter().copied().max().unwrap_or(Status::Good) }; From 287504469e24d3aa0752f243b06eed7c4dd0a38d Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:31:10 -0700 Subject: [PATCH 05/27] generator fills wildcards it refused Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- project/backlog.md | 17 +++-- .../082-init-wildcard-fill/expected-diags.txt | 1 + .../082-init-wildcard-fill/expected-init.shcl | 31 +++++++++ .../082-init-wildcard-fill/expected.shcl | 1 + .../082-init-wildcard-fill/init-schema.shcl | 29 +++++++++ .../082-init-wildcard-fill/input.shcl | 1 + .../082-init-wildcard-fill/reads.tsv | 4 ++ source/c/shcl.h | 49 +++++++++++---- source/go/shcl.go | 63 +++++++++++++------ source/python/shcl.py | 41 ++++++++---- source/rust/src/lib.rs | 62 ++++++++++++------ 11 files changed, 231 insertions(+), 68 deletions(-) create mode 100644 project/conformance/082-init-wildcard-fill/expected-diags.txt create mode 100644 project/conformance/082-init-wildcard-fill/expected-init.shcl create mode 100644 project/conformance/082-init-wildcard-fill/expected.shcl create mode 100644 project/conformance/082-init-wildcard-fill/init-schema.shcl create mode 100644 project/conformance/082-init-wildcard-fill/input.shcl create mode 100644 project/conformance/082-init-wildcard-fill/reads.tsv diff --git a/project/backlog.md b/project/backlog.md index 9384ffb..6bf3da7 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -84,22 +84,29 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170300 - Closed: 20260902-191500 - - 🔘 Item 5: a valued line generated from a filled wildcard is not in the generator's parent-value map, so its dotted child names the empty instance and a satisfiable schema refuses to generate. + - ✅ Item 5: a valued line generated from a filled wildcard is not in the generator's parent-value map, so its dotted child names the empty instance and a satisfiable schema refuses to generate. - Reproduced in all four. `field: a` required, `field: "a[*].b"` required with `default: bee`, `field: "a[*].b[*].c"` required: `init` exits 6 with `V097 required path missing: a[*].b[*].c`. The text behind the refusal is `a:` / `a.b: bee` / `a.b.c:`; the hand-corrected `a.b[bee].c:` validates clean. Same shape through a fragment mount under a valued parent. - Cause: the parent-value map is built only from constraints with no wildcard, so a filled wildcard line with a default never enters it, and `under_valued_parent` consults only that map. 20260901 item 5 covered concrete parents only. + - Fixed: the map is built after the fill decision and takes filled wildcards too, so a line generated from `a[*].b` with a default is a valued parent like any other and `a[*].b[*].c` renders `a.b[bee].c:`. + - Pinned by corpus `082`, whose schema generates and then validates clean against itself. All four refused it at exit 6 before. - Opened: 20260902-170400 + - Closed: 20260902-201500 - - 🔘 Item 6: a valued live parent whose default is a bare integer is selected as `[8]`, which the scanner reads as an index, so `init` exits 6. + - ✅ Item 6: a valued live parent whose default is a bare integer is selected as `[8]`, which the scanner reads as an index, so `init` exits 6. - Reproduced in all four. `field: num` required `default: 8` plus `field: "num[*].v"` required generates `num: 8` / `num[8].v:`, which loads with `E003 no instance 8 of 'num'`; `init` reports V097. Same with `default: "8"` (emitted bare by the data-format rule) and with `type: int`. `[007]` and `[+3]` fail the same way. - Cause: `gen_selector_text` quotes only a default holding `[`, `]`, `\` or a newline; the spec's index rule (`[0]` bare numeric is an index, `["2020"]` quoted is a value) is not applied. - - Note: also quote a default that parses as a bare index. Adjacent to 20260901b item 30 (`E003` reachable). + - Fixed: the selector text quotes a default whose trimmed spelling the scanner would read as a selector of its own - all digits with an optional sign, the same with a leading `#`, or a bare `*` - beside the bracket and backslash cases it already quoted. Each binding asks its own scanner's number parser, so the four cannot drift. + - Pinned by corpus `082` (`default: 8` under a wildcard child, and a `*` default beside it). All four refused the schema before. - Opened: 20260902-170500 + - Closed: 20260902-201500 - - 🔘 Item 7: a must-exist path whose only wildcard is its last segment (`w[*]`) is never filled, so `init` exits 6; beside a `field: w` it is emitted twice. + - ✅ Item 7: a must-exist path whose only wildcard is its last segment (`w[*]`) is never filled, so `init` exits 6; beside a `field: w` it is emitted twice. - Reproduced in all four. `field: "w[*]"` required `default: v` alone generates only the trailing block and fails V097; `w: v` on its own passes `check --schema`. With `field: w` added the output carries `# any, required` / `w: v` twice. - Cause: the fill loop wants a live line whose name list has the wildcard segment's own chain as a prefix; for a trailing wildcard that is the field's own name, so only a separate `field: w` satisfies it, and then both lines emit. - - Note: treat a trailing wildcard as fillable with the root as parent, and skip a fill whose rendering equals a concrete line already emitted. + - Fixed: a wildcard in the last segment is always fillable, since the line generated from it is the instance it needs, and no rendered path is emitted twice - the first spelling in schema order wins, so `field: w` beside `field: "w[*]"` produces one line. A duplicate whose default differs loses that default; the two spellings describe one field, and the surviving line still satisfies both. + - Pinned by corpus `082` (a `w[*]` alone). All four refused the schema before. - Opened: 20260902-170600 + - Closed: 20260902-201500 - 🔘 Item 8: a stdout that cannot be written is reported as success by three CLIs, and as 120 by the fourth. - Reproduced: `fmt f.shcl > /dev/full` exits 0 with an empty stderr in Rust, Go and C; Python exits 120 with an interpreter message. Same for `check`, `get` and `set`. The help and man page say 8 for a stream that could not be written. diff --git a/project/conformance/082-init-wildcard-fill/expected-diags.txt b/project/conformance/082-init-wildcard-fill/expected-diags.txt new file mode 100644 index 0000000..4e7ad2e --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/expected-diags.txt @@ -0,0 +1 @@ +ok (0 diagnostic(s)) diff --git a/project/conformance/082-init-wildcard-fill/expected-init.shcl b/project/conformance/082-init-wildcard-fill/expected-init.shcl new file mode 100644 index 0000000..3029716 --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/expected-init.shcl @@ -0,0 +1,31 @@ +# any, required +a: + +# any, required +a.b: bee + +# any, required +a.b[bee].c: + +# any, required +num: 8 + +# any, required +num["8"].v: + +# any, required +w: v + +# any, required +star: "*" + +# any, required +star["*"].leaf: + +# +# This config file format is SHCL. +# "Simple Hierarchical Config Language" +# Home https://github.com/jim-collier/shcl +# Syntax https://github.com/jim-collier/shcl/blob/main/project/spec.md +# Legal SHCL is Copyright © 2026 Jim Collier. License: MIT. No warranty. +# diff --git a/project/conformance/082-init-wildcard-fill/expected.shcl b/project/conformance/082-init-wildcard-fill/expected.shcl new file mode 100644 index 0000000..d508cf7 --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/expected.shcl @@ -0,0 +1 @@ +x: 1 diff --git a/project/conformance/082-init-wildcard-fill/init-schema.shcl b/project/conformance/082-init-wildcard-fill/init-schema.shcl new file mode 100644 index 0000000..bb3c293 --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/init-schema.shcl @@ -0,0 +1,29 @@ +# A filled wildcard is itself a valued parent, a bare-numeric default has to +# be quoted inside a selector, and a trailing wildcard fills from its own line. +field: a + required: yes + +field: "a[*].b" + required: yes + default: bee + +field: "a[*].b[*].c" + required: yes + +field: num + required: yes + default: 8 + +field: "num[*].v" + required: yes + +field: "w[*]" + required: yes + default: v + +field: star + required: yes + default: "*" + +field: "star[*].leaf" + required: yes diff --git a/project/conformance/082-init-wildcard-fill/input.shcl b/project/conformance/082-init-wildcard-fill/input.shcl new file mode 100644 index 0000000..d508cf7 --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/input.shcl @@ -0,0 +1 @@ +x: 1 diff --git a/project/conformance/082-init-wildcard-fill/reads.tsv b/project/conformance/082-init-wildcard-fill/reads.tsv new file mode 100644 index 0000000..a416066 --- /dev/null +++ b/project/conformance/082-init-wildcard-fill/reads.tsv @@ -0,0 +1,4 @@ +query type expected status +- load ok - +x int 1 Good +- lost 0 - diff --git a/source/c/shcl.h b/source/c/shcl.h index ebb6cb1..882f722 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -5885,7 +5885,16 @@ static const ShclStr *parent_value_for(const ShclParentValues *pv, const ShclVec quoted spelling finds the bare value). */ static ShclStr gen_selector_text(ShclArena *a, ShclStr v) { if (memchr(v.p, '\n', v.n)) return g_default_text(a, v); - if ((memchr(v.p, '[', v.n) || memchr(v.p, ']', v.n) || memchr(v.p, '\\', v.n)) && !quoted_shape(v)) return quote_text(a, v); + // The scanner reads a bare selector body as an index when it is all digits + // (with an optional sign or `#`), and as a wildcard when it is `*`, so a + // default of that shape has to be quoted or the line names an instance + // that is not there. + ShclStr body = s_trim(v); + uint64_t ix; + int reads_as_selector = (body.n == 1 && body.p[0] == '*') + || parse_u64(body, &ix) + || (body.n >= 1 && body.p[0] == '#' && parse_u64(s_slice(body, 1, body.n), &ix)); + if ((memchr(v.p, '[', v.n) || memchr(v.p, ']', v.n) || memchr(v.p, '\\', v.n) || reads_as_selector) && !quoted_shape(v)) return quote_text(a, v); return v; } @@ -6026,6 +6035,10 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { for (size_t s2 = 0; s2 < plen; s2++) if (!s_eq(live[li].data[s2].name, c->segs.data[s2].name)) { eq = 0; break; } hit = eq; } + // A wildcard in the last segment needs no other line to + // materialize its parent: the line generated from it is that + // instance. + if (plen == c->segs.len) hit = 1; if (hit) { fill[i] = 1; LIVE_PUSH(c->segs); changed = 1; } } if (!changed) break; @@ -6040,10 +6053,16 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { pv.data = (ShclParentValue *)arena_alloc(a, (cons.len ? cons.len : 1) * sizeof *pv.data); for (size_t i = 0; i < cons.len; i++) { const ShclVCons *c = &cons.data[i]; - if (!g_has_wild(c) && !g_unwritable(c) && g_must_exist(c) && c->has_default) { pv.data[pv.len].segs = &c->segs; pv.data[pv.len].value = c->default_text; pv.len++; } + // A filled wildcard emits a valued line of its own, so it belongs here too. + if ((!g_has_wild(c) || fill[i]) && !g_unwritable(c) && g_must_exist(c) && c->has_default) { pv.data[pv.len].segs = &c->segs; pv.data[pv.len].value = c->default_text; pv.len++; } } ShclSB out = {0, 0, 0}; ShclVecS wild_path = {0, 0, 0}, wild_type = {0, 0, 0}; + /* Dropping a trailing `[*]` can render the same line a concrete sibling + already wrote; the first spelling wins. Hash first, bytes only on a + hash hit, so the scan stays cheap at the field cap. */ + ShclVecS emitted = {0, 0, 0}; + uint64_t *emitted_hash = (uint64_t *)arena_alloc(a, (cons.len ? cons.len : 1) * sizeof *emitted_hash); int first = 1; for (size_t i = 0; i < cons.len; i++) { ShclVCons *c = &cons.data[i]; @@ -6053,6 +6072,21 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { ShclVecS_push(a, &wild_path, g_escape_nl(a, c->path)); ShclVecS_push(a, &wild_type, tyname); continue; } + // A filled wildcard emits in dotted form, targeting the materialized + // instance - by its value when the materializing line carries one. + // Rebuilt from the parsed segments, not by cutting text out of the + // path: the same path can be written several ways, and only the + // segments say what it means. Otherwise the schema's own spelling. + int under_valued_parent = 0; + for (size_t k = 1; k < c->segs.len && !under_valued_parent; k++) + under_valued_parent = c->segs.data[k - 1].sel.tag == SEL_NONE && parent_value_for(&pv, &c->segs, k) != NULL; + ShclStr path = (fill[i] || under_valued_parent) ? gen_path_text(a, &c->segs, &pv) : c->path; + uint64_t ph = fnv_str(1469598103934665603ull, path); + int dup = 0; + for (size_t k = 0; k < emitted.len && !dup; k++) dup = emitted_hash[k] == ph && s_eq(emitted.data[k], path); + if (dup) continue; + emitted_hash[emitted.len] = ph; + ShclVecS_push(a, &emitted, path); if (!first) sb_putc(a, &out, '\n'); first = 0; if (c->has_desc) { @@ -6068,16 +6102,7 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { } sb_puts(a, &out, "# "); sb_putS(a, &out, g_escape_nl(a, v_gen_annotation(a, c, tyname))); sb_putc(a, &out, '\n'); if (!g_must_exist(c)) sb_putc(a, &out, '#'); - // A filled wildcard emits in dotted form, targeting the materialized - // instance - by its value when the materializing line carries one. - // Rebuilt from the parsed segments, not by cutting text out of the - // path: the same path can be written several ways, and only the - // segments say what it means. Otherwise the schema's own spelling. - int under_valued_parent = 0; - for (size_t k = 1; k < c->segs.len && !under_valued_parent; k++) - under_valued_parent = c->segs.data[k - 1].sel.tag == SEL_NONE && parent_value_for(&pv, &c->segs, k) != NULL; - if (fill[i] || under_valued_parent) sb_putS(a, &out, gen_path_text(a, &c->segs, &pv)); - else sb_putS(a, &out, c->path); + sb_putS(a, &out, path); if (c->has_default) { sb_puts(a, &out, ": "); sb_putS(a, &out, g_default_text(a, c->default_text)); } else sb_putc(a, &out, ':'); sb_putc(a, &out, '\n'); diff --git a/source/go/shcl.go b/source/go/shcl.go index bb37926..c759c18 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -6121,6 +6121,15 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { } } parent := namesOf(c.segs[:k+1]) + // A wildcard in the last segment needs no other line to + // materialize its parent: the line generated from it is that + // instance. + if k+1 == len(c.segs) { + fill[i] = true + live = append(live, namesOf(c.segs)) + changed = true + continue + } for _, p := range live { if isPrefix(p, parent) { fill[i] = true @@ -6139,15 +6148,19 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { // web` followed by `srv.port:` is two `srv` nodes, and the child never // lands where the schema looks. Any line under such a parent selects it // by its value: `srv[web].port:`. + // A filled wildcard emits a valued line of its own, so it belongs here too. parentValues := map[string]string{} for i := range cons { c := &cons[i] - if !hasWild(c) && !unwritable(c) && mustExist(c) && c.defaultText != nil { + if (!hasWild(c) || fill[i]) && !unwritable(c) && mustExist(c) && c.defaultText != nil { parentValues[namesKey(namesOf(c.segs))] = *c.defaultText } } var b strings.Builder var wild [][2]string + // Dropping a trailing `[*]` can render the same line a concrete sibling + // already wrote; the first spelling wins. + emitted := map[string]bool{} first := true for i := range cons { c := &cons[i] @@ -6159,22 +6172,6 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { wild = append(wild, [2]string{strings.ReplaceAll(c.path, "\n", "\\n"), tyname}) continue } - if !first { - b.WriteByte('\n') - } - first = false - if c.desc != nil { - for _, line := range strings.Split(*c.desc, "\n") { - b.WriteString("# ") - b.WriteString(line) - b.WriteByte('\n') - } - } - b.WriteString("# ") - // The annotation is a comment: a newline smuggled in via an allowed - // string value must not break out of it. - b.WriteString(strings.ReplaceAll(genAnnotation(c, tyname), "\n", "\\n")) - b.WriteByte('\n') // A filled wildcard emits in dotted form, targeting the materialized // instance - by its value when the materializing line carries one. // Rebuilt from the parsed segments, not by cutting text out of the @@ -6191,6 +6188,26 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { if fill[i] || underValuedParent { path = genPathText(c.segs, parentValues) } + if emitted[path] { + continue + } + emitted[path] = true + if !first { + b.WriteByte('\n') + } + first = false + if c.desc != nil { + for _, line := range strings.Split(*c.desc, "\n") { + b.WriteString("# ") + b.WriteString(line) + b.WriteByte('\n') + } + } + b.WriteString("# ") + // The annotation is a comment: a newline smuggled in via an allowed + // string value must not break out of it. + b.WriteString(strings.ReplaceAll(genAnnotation(c, tyname), "\n", "\\n")) + b.WriteByte('\n') prefix := "#" if mustExist(c) { prefix = "" @@ -6294,7 +6311,17 @@ func genSelectorText(v string) string { if strings.Contains(v, "\n") { return genDefaultText(v) } - if strings.ContainsAny(v, "[]\\") && !quotedShape(v) { + // The scanner reads a bare selector body as an index when it is all digits + // (with an optional sign or `#`), and as a wildcard when it is `*`, so a + // default of that shape has to be quoted or the line names an instance + // that is not there. + body := strings.TrimSpace(v) + _, isIndex := parseIndex(body) + if !isIndex { + _, isIndex = hashIndex(body) + } + readsAsSelector := body == "*" || isIndex + if (strings.ContainsAny(v, "[]\\") || readsAsSelector) && !quotedShape(v) { return quoteText(v) } return v diff --git a/source/python/shcl.py b/source/python/shcl.py index 71fdbdc..2db0ac3 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -4943,7 +4943,9 @@ def names_of(segs): continue k = next(j for j, s in enumerate(c.segs) if s.selector is not None and s.selector[0] == "wild") parent = names_of(c.segs[: k + 1]) - if any(len(p) >= len(parent) and p[: len(parent)] == parent for p in live): + # A wildcard in the last segment needs no other line to materialize + # its parent: the line generated from it is that instance. + if k + 1 == len(c.segs) or any(len(p) >= len(parent) and p[: len(parent)] == parent for p in live): fill[i] = True live.append(names_of(c.segs)) changed = True @@ -4954,28 +4956,23 @@ def names_of(segs): # web` followed by `srv.port:` is two `srv` nodes, and the child never # lands where the schema looks. Any line under such a parent selects it by # its value: `srv[web].port:`. + # A filled wildcard emits a valued line of its own, so it belongs here too. parent_values = { tuple(names_of(c.segs)): c.default_text - for c in cons - if not has_wild(c) and not unwritable(c) and must_exist(c) and c.default_text is not None + for i, c in enumerate(cons) + if (not has_wild(c) or fill[i]) and not unwritable(c) and must_exist(c) and c.default_text is not None } out = [] wild = [] + # Dropping a trailing `[*]` can render the same line a concrete sibling + # already wrote; the first spelling wins. + emitted = set() first = True for i, c in enumerate(cons): tyname = c.ty if c.ty is not None else "any" if unwritable(c) or (has_wild(c) and not fill[i]): wild.append((c.path.replace("\n", "\\n"), tyname)) continue - if not first: - out.append("\n") - first = False - if c.desc is not None: - for line in c.desc.split("\n"): - out.append("# " + line + "\n") - # The annotation is a comment: a newline smuggled in via an allowed - # string value must not break out of it. - out.append("# " + _gen_annotation(c, tyname).replace("\n", "\\n") + "\n") # A filled wildcard emits in dotted form, targeting the materialized # instance - by its value when the materializing line carries one. # Rebuilt from the parsed segments, not by cutting text out of the @@ -4986,6 +4983,18 @@ def names_of(segs): for k in range(1, len(c.segs)) ) path = _gen_path_text(c.segs, parent_values) if fill[i] or under_valued_parent else c.path + if path in emitted: + continue + emitted.add(path) + if not first: + out.append("\n") + first = False + if c.desc is not None: + for line in c.desc.split("\n"): + out.append("# " + line + "\n") + # The annotation is a comment: a newline smuggled in via an allowed + # string value must not break out of it. + out.append("# " + _gen_annotation(c, tyname).replace("\n", "\\n") + "\n") prefix = "" if must_exist(c) else "#" if c.default_text is not None: out.append(f"{prefix}{path}: {_gen_default_text(c.default_text)}\n") @@ -5059,7 +5068,13 @@ def _gen_selector_text(v): quoted spelling finds the bare value).""" if "\n" in v: return _gen_default_text(v) - if any(ch in v for ch in "[]\\") and not _quoted_shape(v): + # The scanner reads a bare selector body as an index when it is all digits + # (with an optional sign or `#`), and as a wildcard when it is `*`, so a + # default of that shape has to be quoted or the line names an instance that + # is not there. + body = v.strip() + reads_as_selector = body == "*" or _parse_uint(body) is not None or (body[:1] == "#" and _parse_uint(body[1:]) is not None) + if (any(ch in v for ch in "[]\\") or reads_as_selector) and not _quoted_shape(v): return _quote_text(v) return v diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 285caba..6672be7 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -8,7 +8,7 @@ //! Every other binding mirrors this file's structure on purpose (parity over //! idiom - see style-guide.md), so restructuring here means restructuring all. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; // --------------------------------------------------------------------------- // Public surface @@ -5804,9 +5804,13 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result= parent.len() && p[..parent.len()] == parent[..]) + // A wildcard in the last segment needs no other line to + // materialize its parent: the line generated from it is that + // instance. + if k + 1 == c.segs.len() + || live + .iter() + .any(|p| p.len() >= parent.len() && p[..parent.len()] == parent[..]) { fill[i] = true; live.push(names_of(&c.segs)); @@ -5822,13 +5826,18 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result, &str> = cons .iter() - .filter(|c| !has_wild(c) && !unwritable(c) && must_exist(c)) - .filter_map(|c| c.default_text.as_deref().map(|d| (names_of(&c.segs), d))) + .enumerate() + .filter(|(i, c)| (!has_wild(c) || fill[*i]) && !unwritable(c) && must_exist(c)) + .filter_map(|(_, c)| c.default_text.as_deref().map(|d| (names_of(&c.segs), d))) .collect(); let mut out = String::new(); let mut wild: Vec<(String, String)> = Vec::new(); + // Dropping a trailing `[*]` can render the same line a concrete sibling + // already wrote; the first spelling wins. + let mut emitted: HashSet = HashSet::new(); let mut first = true; for (i, c) in cons.iter().enumerate() { let tyname = c.ty.clone().unwrap_or_else(|| "any".to_string()); @@ -5836,6 +5845,22 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result Result out.push_str(&format!("{}{}: {}\n", prefix, path, gen_default_text(v))), @@ -5991,7 +6003,17 @@ fn gen_selector_text(v: &str) -> String { if v.contains('\n') { return gen_default_text(v); } - if v.contains(['[', ']', '\\']) && !quoted_shape(v) { + // The scanner reads a bare selector body as an index when it is all digits + // (with an optional sign or `#`), and as a wildcard when it is `*`, so a + // default of that shape has to be quoted or the line names an instance + // that is not there. + let body = v.trim(); + let reads_as_selector = body == "*" + || body.parse::().is_ok() + || body + .strip_prefix('#') + .is_some_and(|d| d.parse::().is_ok()); + if (v.contains(['[', ']', '\\']) || reads_as_selector) && !quoted_shape(v) { return quote_text(v); } v.to_string() From 95d85161dc117f7c6ee7f192ca8ce677064760cc Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:41:23 -0700 Subject: [PATCH 06/27] stream write failures exit 8 Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 4 + cicd/utility/cli-regress.bash | 19 +++- project/backlog.md | 13 ++- source/c/cmd/shcl/main.c | 28 ++++- source/go/cmd/shcl/main.go | 83 ++++++++++---- source/python/cmd/shcl/main.py | 48 +++++++- source/rust/src/main.rs | 193 ++++++++++++++++++++------------- 7 files changed, 286 insertions(+), 102 deletions(-) diff --git a/changelog.md b/changelog.md index ef35e23..6ec5c1a 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- A stdout that cannot be written exits 8 instead of reporting success. `shcl fmt f.shcl > /dev/full` exited 0 with an empty stderr in three of the four CLIs and killed the Python one with an interpreter message; the help and the man page have said 8 for a stream that could not be written all along. A reader that closed early is still the quiet exit, since nobody is there to read a complaint. + +- A stderr that cannot be written no longer costs the document. The reference aborted with nothing on stdout at all when a diagnostic could not be printed, which turned an unwritable log into a lost `fmt`. Diagnostics are best-effort now; the exit code still carries the outcome. + - A wildcard read whose parent does not exist reports `NotFound` instead of `Empty`. `x[*]` on a document with no `x` said the path was there and empty, which is the answer for a field written with nothing after the colon; `x` on its own said `NotFound`. The two agree now. - A wildcard after a wildcard flattens instead of answering `Multiple` for every slot. `server[*].*` reported one unreadable slot per instance, `count` counted instances rather than leaves, and `Remove` on such a path removed nothing. The inner slots now join the outer run, so the result is one slot per resolved leaf and the two wildcards compose the way the spec says they do. diff --git a/cicd/utility/cli-regress.bash b/cicd/utility/cli-regress.bash index 6152863..ee9c255 100755 --- a/cicd/utility/cli-regress.bash +++ b/cicd/utility/cli-regress.bash @@ -76,7 +76,8 @@ printf 'a: 1\nb: 2\n' > "${tmpDir}/two.shcl" ## instance whose discriminator holds an '=', %T% a document with a name that ## needs quoting in a path, %F2% a two-key file for the edit options, %M% a ## path with no file at it. -## stdin: printf %b text, '-' none, '@closedin' / '@closedout' close that stream. +## stdin: printf %b text, '-' none, '@closedin' / '@closedout' close that +## stream, '@fullout' / '@fullerr' point it at a device that is always full. ## stdout and stderr: '-' means unchecked; an empty stdout field means exactly empty. ## A stderr regex starting with '!' must match NO line. ## Each row names the round and item it pins. @@ -153,6 +154,14 @@ rows=( 'children-quoted|children %T% db|-|0|host\n"odd.key"|-' 'children-missing|children %T% nope|-|0||-' 'paths-all|paths %T%|-|0|db\ndb.host\ndb."odd.key"\nweb\nweb.port|-' + ## 20260902 items 8 and 9: a stdout that could not be written was reported + ## as success by three CLIs, and a stderr that could not be written aborted + ## the reference with nothing on stdout at all. + 'full-stdout-fmt|fmt %F%|@fullout|8|-|[Nn]o space left' + 'full-stdout-check|check %F%|@fullout|8|-|-' + 'full-stdout-get|get %F% a|@fullout|8|-|-' + 'full-stdout-set|set --set=a=2 %F%|@fullout|8|-|-' + 'full-stderr-keeps-stdout|fmt %B%|@fullerr|0|a: 1\n\tbad:\nb 2\n|-' ## Found working 20260830b item 18: a merge does not carry diagnostics, so ## reading them off the merged doc reported the lowest layer and stayed ## silent about FILE - the one file the caller actually named. @@ -176,6 +185,12 @@ for row in "${rows[@]}"; do argv="${argv//%T%/${tmpDir}/tree.shcl}" argv="${argv//%F2%/${tmpDir}/two.shcl}" argv="${argv//%M%/${tmpDir}/not-there.shcl}" + ## A device that is always full exists on linux and not on windows; the + ## rows that need one are skipped out loud rather than passing vacuously. + if [[ "${stdinSpec}" == @full* && ! -w /dev/full ]]; then + echo "cli-regress: skipping ${id} (no /dev/full here)" + continue + fi read -r -a args <<<"${argv}" for b in "${bindings[@]}"; do name="${b%%|*}"; cli="${b#*|}" @@ -183,6 +198,8 @@ for row in "${rows[@]}"; do case "${stdinSpec}" in @closedin) "${cli}" "${args[@]}" >"${tmpDir}/out" 2>"${tmpDir}/err" 0<&- || rc=$? ;; @closedout) "${cli}" "${args[@]}" 2>"${tmpDir}/err" >&- || rc=$?; : >"${tmpDir}/out" ;; + @fullout) "${cli}" "${args[@]}" 2>"${tmpDir}/err" >/dev/full || rc=$?; : >"${tmpDir}/out" ;; + @fullerr) "${cli}" "${args[@]}" >"${tmpDir}/out" 2>/dev/full || rc=$?; : >"${tmpDir}/err" ;; -) "${cli}" "${args[@]}" >"${tmpDir}/out" 2>"${tmpDir}/err" "${tmpDir}/out" 2>"${tmpDir}/err" || rc=$? ;; esac diff --git a/project/backlog.md b/project/backlog.md index 6bf3da7..ef984c5 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -108,17 +108,22 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170600 - Closed: 20260902-201500 - - 🔘 Item 8: a stdout that cannot be written is reported as success by three CLIs, and as 120 by the fourth. + - ✅ Item 8: a stdout that cannot be written is reported as success by three CLIs, and as 120 by the fourth. - Reproduced: `fmt f.shcl > /dev/full` exits 0 with an empty stderr in Rust, Go and C; Python exits 120 with an interpreter message. Same for `check`, `get` and `set`. The help and man page say 8 for a stream that could not be written. - Cause: the reference's `out!`/`outln!` treat every write error as a broken pipe and exit 0 (on unix SIGPIPE is restored, so EPIPE never reaches the branch and every error that does is not a broken pipe); Go drops `fmt.Print` results; C never checks `ferror(stdout)`; Python never flushes inside `main`. - - Note: `BrokenPipe` stays the quiet exit; anything else is exit 8 with the OS message. Flush stdout before returning the code. Same shape in all four. + - Fixed: every stdout write in all four goes through one pair of helpers that exit 8 with the OS message on a write error, and the buffered tail is flushed before the exit code is returned so a failure there is caught too. A broken pipe stays the quiet exit. The C CLI also points a standard stream that was closed before the start at the null device, which the other three runtimes already do - without it every write failed with EBADF and the next file opened landed on the closed descriptor. + - Pinned by four `cli-regress` rows (`fmt`, `check`, `get` and `set` with stdout on a device that is always full). All four reported success before; the rows skip out loud where there is no such device. - Opened: 20260902-170700 + - Closed: 20260902-213000 - - 🔘 Item 9: the reference aborts with nothing on stdout when stderr cannot be written. + - ✅ Item 9: the reference aborts with nothing on stdout when stderr cannot be written. - Reproduced: `fmt bad.shcl 2>/dev/full` on a document with one diagnostic exits 134 (SIGABRT) with an empty stdout; Go and C exit 0 with the document; Python exits 120 with nothing. A single hint is enough, since every loading subcommand prints its diagnostics. - Cause: `eprintln!` panics on a write error and the release profile has `panic = "abort"`, so the abort lands before `out!` has printed anything. A closed stderr is fine (EBADF is discarded); a full or failing one is not. - - Note: an `err!`/`errln!` pair beside `out!` that ignores the write result, or exits 8 on a non-EPIPE error; Python the same. + - Decided: a diagnostic that cannot be printed is dropped. There is nowhere to report the failure, the document on stdout is still good, and the exit code already carries the outcome - Go and C did this already. + - Fixed: the reference's diagnostics go through an `errln!` that ignores the write result, and Python's stderr is wrapped so a failed write is dropped instead of raising. + - Pinned by a `cli-regress` row that runs `fmt` on a document with a diagnostic, stderr on a full device, and requires the canonical text on stdout at exit 0. The reference exited 101 and Python 120 before, both with an empty stdout. - Opened: 20260902-170800 + - Closed: 20260902-213000 - 🔘 Item 10: a C parse leaves its temporaries in the scratch arena until the first resolve, so a parsed document holds about ten times its input in dead memory. - Reproduced: a 49 MiB input parses to 970 MB RSS with 487 MB of it in `d->scratch`; 10 MiB gives 204 MB with 97 MB dead. One `arena_free(&d->scratch)` at the end of `do_parse` gives 766 MB and 157 MB with parse time unchanged, and 46,600 writer, merge and compact operations under ASan and UBSan with that free applied show nothing live pointed into it. diff --git a/source/c/cmd/shcl/main.c b/source/c/cmd/shcl/main.c index 0e54952..8ab0a29 100644 --- a/source/c/cmd/shcl/main.c +++ b/source/c/cmd/shcl/main.c @@ -1243,8 +1243,19 @@ static char **utf8_argv(int *argc) { } #endif -int main(int argc, char **argv) { +static int cli_main(int argc, char **argv) { setlocale(LC_ALL, "C"); // strtod/printf must use '.' regardless of environment +#ifndef _WIN32 + // The Rust, Go and Python runtimes point a standard stream that was closed + // before the start at the null device; C's does not, so every write would + // fail with EBADF where the other three quietly drop it - and the next + // file opened would land on fd 1 and be written over. + for (int fd = 0; fd <= 2; fd++) + if (fcntl(fd, F_GETFD) == -1 && errno == EBADF) { + int nfd = open("/dev/null", fd == 0 ? O_RDONLY : O_WRONLY); + if (nfd != fd && nfd >= 0) close(nfd); + } +#endif #ifdef _WIN32 // Byte-for-byte with the reference: no CRLF translation on any stream. _setmode(_fileno(stdin), _O_BINARY); @@ -1299,3 +1310,18 @@ int main(int argc, char **argv) { opts_free(&o); return rc; } + +int main(int argc, char **argv) { + int code = cli_main(argc, argv); + // A tail still sitting in the buffer when the work is done fails the same + // way a write does. A reader that closed early is nothing to report - + // nobody is there to read it - so that leaves quietly; anything else lost + // the output, which is the same failure as a file that could not be + // written. + if (fflush(stdout) != 0 || ferror(stdout)) { + if (errno == EPIPE) return 0; + fprintf(stderr, "stdout: %s\n", strerror(errno)); + return 8; + } + return code; +} diff --git a/source/go/cmd/shcl/main.go b/source/go/cmd/shcl/main.go index 7e6be71..99ff4b3 100644 --- a/source/go/cmd/shcl/main.go +++ b/source/go/cmd/shcl/main.go @@ -12,13 +12,56 @@ import ( "io" "math" "os" + "runtime" "strconv" "strings" + "syscall" "unicode/utf8" shcl "github.com/jim-collier/shcl/source/go/v2" ) +// Every stdout write goes through these. A reader that closed early is not an +// error worth reporting - nobody is there to read one - so that leaves +// quietly; anything else lost the output, which is the same failure as a file +// that could not be written. +func outf(format string, a ...any) { + if _, err := fmt.Fprintf(os.Stdout, format, a...); err != nil { + writeFailed(err) + } +} + +func outln(a ...any) { + if _, err := fmt.Fprintln(os.Stdout, a...); err != nil { + writeFailed(err) + } +} + +func outs(a ...any) { + if _, err := fmt.Fprint(os.Stdout, a...); err != nil { + writeFailed(err) + } +} + +// EPIPE on unix, where the SIGPIPE default usually ends the process before the +// error is seen at all; on windows a closed reader arrives as +// ERROR_BROKEN_PIPE, which Go surfaces as errno 109 rather than mapping it. +func brokenPipe(err error) bool { + if errors.Is(err, syscall.EPIPE) { + return true + } + var errno syscall.Errno + return runtime.GOOS == "windows" && errors.As(err, &errno) && errno == 109 +} + +func writeFailed(err error) { + if brokenPipe(err) { + os.Exit(0) + } + fmt.Fprintf(os.Stderr, "stdout: %v\n", err) + os.Exit(8) +} + // Keep in step with source/rust/Cargo.toml, the canonical version source. const version = "2.0.0" @@ -886,9 +929,9 @@ func doGet(o *opts) int { emit := func(lines []string) { for i, l := range lines { if o.slots { - fmt.Printf("%s\t%s\n", slotAt(i), l) + outf("%s\t%s\n", slotAt(i), l) } else { - fmt.Println(l) + outln(l) } } } @@ -938,9 +981,9 @@ func doGet(o *opts) int { } emit(subbed) } else if o.slots { - fmt.Printf("%s\t%s\n", status, o.def) + outf("%s\t%s\n", status, o.def) } else { - fmt.Println(o.def) + outln(o.def) } return 0 case o.onBad == onBadError: @@ -1002,7 +1045,7 @@ func doFmt(o *opts) int { if o.write { return writeBack(doc, file, o) } - fmt.Print(doc.ToCanonical()) + outs(doc.ToCanonical()) return 0 } @@ -1434,7 +1477,7 @@ func doSet(o *opts) int { if o.write { return writeBack(doc, file, o) } - fmt.Print(doc.ToCanonical()) + outs(doc.ToCanonical()) return 0 } @@ -1493,7 +1536,7 @@ func doCheck(o *opts) int { // prose names the file so the two number spaces cannot be confused. errorCount := 0 for _, d := range diags { - fmt.Printf("line %d: %s: %s\n", d.Line, d.Severity, d.Code) + outf("line %d: %s: %s\n", d.Line, d.Severity, d.Code) if d.Severity == shcl.SeverityError { errorCount++ } @@ -1501,14 +1544,14 @@ func doCheck(o *opts) int { sayDiagnostics(diags) switch { case strictFailed: - fmt.Printf("strict load failed: %d diagnostic(s)\n", len(diags)) + outf("strict load failed: %d diagnostic(s)\n", len(diags)) return 6 case errorCount > 0: // Loaded, but lines were dropped: nonzero so a CI gate on check catches it. - fmt.Printf("failed: %d diagnostic(s), %d error(s)\n", len(diags), errorCount) + outf("failed: %d diagnostic(s), %d error(s)\n", len(diags), errorCount) return 6 default: - fmt.Printf("ok (%d diagnostic(s))\n", len(diags)) + outf("ok (%d diagnostic(s))\n", len(diags)) return 0 } } @@ -1552,7 +1595,7 @@ func doInit(o *opts) int { fmt.Fprintln(os.Stderr, "init: schema has faults") return 6 } - fmt.Print(text) + outs(text) return 0 } @@ -1575,10 +1618,10 @@ func doEnum(o *opts, wantCount bool) int { // One report per invocation, so a read in a loop is one line per call. sayDiagnostics(diags) if wantCount { - fmt.Println(doc.Count(path)) + outln(doc.Count(path)) } else { for _, v := range doc.Instances(path) { - fmt.Println(v) + outln(v) } } return 0 @@ -1605,7 +1648,7 @@ func doChildren(o *opts) int { } sayDiagnostics(diags) for _, name := range doc.Children(path) { - fmt.Println(shcl.QuoteSegment(name)) + outln(shcl.QuoteSegment(name)) } return 0 } @@ -1623,7 +1666,7 @@ func doPaths(o *opts) int { } sayDiagnostics(diags) for _, p := range doc.Paths() { - fmt.Println(p) + outln(p) } return 0 } @@ -1644,23 +1687,23 @@ func run() int { // block from the surrounding prompts. A bare run used to print the same // text unpadded and exit 1, which read as neither a help nor an error. if len(argv) == 0 { - fmt.Printf("\n%s\n", help) + outf("\n%s\n", help) return 0 } if asked == "help" || argv[0] == "help" { - fmt.Printf("\n%s\n", help) + outf("\n%s\n", help) return 0 } if asked == "version" || argv[0] == "version" { - fmt.Printf("shcl %s\n", version) + outf("shcl %s\n", version) return 0 } if asked == "about" || argv[0] == "about" { - fmt.Printf("\n%s\n", about) + outf("\n%s\n", about) return 0 } if asked == "donate" || argv[0] == "donate" { - fmt.Printf("\n%s\n", donate) + outf("\n%s\n", donate) return 0 } cmd := argv[0] diff --git a/source/python/cmd/shcl/main.py b/source/python/cmd/shcl/main.py index fe6d385..4dc3d4c 100755 --- a/source/python/cmd/shcl/main.py +++ b/source/python/cmd/shcl/main.py @@ -16,6 +16,44 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))) import shcl +class _BestEffort: + """stderr, wrapped so a write that fails is dropped. A stream that cannot + be written has nowhere to report that fact, and the document on stdout is + still good - where an uncaught OSError would lose that too.""" + + def __init__(self, stream): + self._stream = stream + + def write(self, text): + try: + self._stream.write(text) + except OSError: + pass + return len(text) + + def flush(self): + try: + self._stream.flush() + except OSError: + pass + + def __getattr__(self, name): + return getattr(self._stream, name) + + +def write_failed(e): + """A stdout write that failed. A reader that closed early is nothing to + report - nobody is there to read it - so that leaves quietly; anything + else lost the output, which is the same failure as a file that could not + be written. The stream is swapped for a sink so the interpreter's own + exit-time flush does not fail again over the top of the exit code.""" + sys.stdout = open(os.devnull, "w", encoding="utf-8") # noqa: SIM115 + if isinstance(e, BrokenPipeError): + return 0 + sys.stderr.write(f"stdout: {e.strerror or e}\n") + return 8 + + # Keep in step with source/rust/Cargo.toml, the canonical version source. VERSION = "2.0.0" @@ -1242,7 +1280,15 @@ def main(): reconfigure(encoding="utf-8", newline="\n") except (ValueError, OSError): pass - return run(sys.argv[1:]) + sys.stderr = _BestEffort(sys.stderr) + try: + code = run(sys.argv[1:]) + # A tail still sitting in the buffer when the work is done fails the + # same way a write does. + sys.stdout.flush() + except OSError as e: + return write_failed(e) + return code if __name__ == "__main__": diff --git a/source/rust/src/main.rs b/source/rust/src/main.rs index 0d1a686..0714ece 100644 --- a/source/rust/src/main.rs +++ b/source/rust/src/main.rs @@ -18,24 +18,41 @@ use std::process::ExitCode; macro_rules! out { ($($arg:tt)*) => {{ use std::io::Write; - if write!(std::io::stdout(), $($arg)*).is_err() { - broken_pipe(); + if let Err(e) = write!(std::io::stdout(), $($arg)*) { + write_failed(&e); } }}; } macro_rules! outln { ($($arg:tt)*) => {{ use std::io::Write; - if writeln!(std::io::stdout(), $($arg)*).is_err() { - broken_pipe(); + if let Err(e) = writeln!(std::io::stdout(), $($arg)*) { + write_failed(&e); } }}; } -/// Nothing more can be delivered, and nobody is there to read an error -/// either: leave quietly, the way Go and C do here. -fn broken_pipe() -> ! { - std::process::exit(0) +// Diagnostics and error messages. A stderr that cannot be written has nowhere +// to report that fact, and the document on stdout is still good, so the write +// result is dropped - what the print macros do instead is panic, which the +// release build turns into an abort with nothing delivered. +macro_rules! errln { + ($($arg:tt)*) => {{ + use std::io::Write; + let _ = writeln!(std::io::stderr(), $($arg)*); + }}; +} + +/// A stdout write that failed. A reader that closed early is nothing to +/// report, since nobody is there to read it, so that leaves quietly the way Go +/// and C do. Anything else lost the output, which is the same failure as a +/// file that could not be written. +fn write_failed(e: &std::io::Error) -> ! { + if e.kind() == std::io::ErrorKind::BrokenPipe { + std::process::exit(0); + } + errln!("stdout: {}", e); + std::process::exit(8) } const HELP: &str = "\ @@ -580,12 +597,12 @@ fn check_opts(cmd: &str, o: &Opts) -> Result<(), u8> { for s in &o.seen { if !allowed.contains(s) { if *s == "--" { - eprintln!("type options are not valid for {} (see --help)", cmd); + errln!("type options are not valid for {} (see --help)", cmd); } else if cmd == "init" && *s == "--strictness" { // Deliberate, not an oversight: the schema is a program artifact, // so it always loads at Standard - the same rule `check --schema` // follows for the schema half. - eprintln!( + errln!( "option --strictness not valid for init: a schema always loads at standard strictness, being a program artifact rather than user data" ); } else if cmd == "check" && matches!(*s, "--layer" | "--set" | "--set-literal") { @@ -593,12 +610,13 @@ fn check_opts(cmd: &str, o: &Opts) -> Result<(), u8> { // line numbers, and a merged document has no single file to // number against. Naming the pipeline turns a dead end into a // one-liner. - eprintln!( + errln!( "option {} not valid for check: diagnostics cite line numbers, which a merged document has none of. Pipe instead: shcl fmt {} ... FILE | shcl check --schema=SCHEMA -", - s, s + s, + s ); } else { - eprintln!("option {} not valid for {} (see --help)", s, cmd); + errln!("option {} not valid for {} (see --help)", s, cmd); } return Err(1); } @@ -608,11 +626,11 @@ fn check_opts(cmd: &str, o: &Opts) -> Result<(), u8> { // the --set values are edits to the document rather than a layer over it, so // persisting them is the whole point; everywhere else they stay ephemeral. if o.write && !o.layers.is_empty() { - eprintln!("--write cannot be combined with --layer (see --help)"); + errln!("--write cannot be combined with --layer (see --help)"); return Err(1); } if o.write && !o.sets.is_empty() && cmd != "set" { - eprintln!( + errln!( "--write cannot be combined with {} (see --help)", o.sets[0].opt() ); @@ -621,12 +639,12 @@ fn check_opts(cmd: &str, o: &Opts) -> Result<(), u8> { // --lossy only overrides the in-place write's refusal, so on its own it says // nothing and would read as protection the command never had. if o.lossy && !o.write { - eprintln!("--lossy is only meaningful with --write (see --help)"); + errln!("--lossy is only meaningful with --write (see --help)"); return Err(1); } // The ops script already has stdin, so a layer cannot read it too. if cmd == "set" && o.layers.iter().any(|l| l == "-") { - eprintln!("--layer=- is not valid for set (stdin carries the ops script or the document)"); + errln!("--layer=- is not valid for set (stdin carries the ops script or the document)"); return Err(1); } // Stdin reads once; a second '-' would silently get an empty document. @@ -634,7 +652,7 @@ fn check_opts(cmd: &str, o: &Opts) -> Result<(), u8> { + usize::from(o.schema.as_deref() == Some("-")) + usize::from(o.args.first().map(|f| f == "-").unwrap_or(false)); if stdin_uses > 1 { - eprintln!("'-' (stdin) can be named only once across FILE, --layer and --schema"); + errln!("'-' (stdin) can be named only once across FILE, --layer and --schema"); return Err(1); } Ok(()) @@ -662,9 +680,13 @@ fn say_diagnostics(diags: &[Diagnostic]) { } else { "line" }; - eprintln!( + errln!( "{} {}: {:?}: {} {}", - space, d.line, d.severity, d.code, d.message + space, + d.line, + d.severity, + d.code, + d.message ); } } @@ -683,12 +705,12 @@ fn load_layered(o: &Opts, file: &str) -> Result<(Document, Vec), u8> let mut texts: Vec = Vec::with_capacity(o.layers.len() + 1); for lf in &o.layers { texts.push(read_input(lf).map_err(|e| { - eprintln!("{}", e); + errln!("{}", e); EXIT_IO })?); } let base_text = read_input(file).map_err(|e| { - eprintln!("{}", e); + errln!("{}", e); EXIT_IO })?; texts.push(base_text); @@ -701,7 +723,7 @@ fn load_layered(o: &Opts, file: &str) -> Result<(Document, Vec), u8> } for s in &o.sets { if !s.apply(&mut doc) { - eprintln!( + errln!( "{}: cannot write {}: {}", s.opt(), s.path, @@ -729,14 +751,15 @@ fn write_back(doc: &Document, file: &str, o: &Opts) -> u8 { Err(SaveError::Refused { lost, .. }) => { // The rule stays in the library; only the wording is the CLI's, // because the override a user has here is a flag, not a function. - eprintln!( + errln!( "{}: refusing to rewrite: the load dropped {} line(s)/value(s) this write would delete (--lossy overrides)", - file, lost + file, + lost ); 7 } Err(e) => { - eprintln!("{}", e); + errln!("{}", e); EXIT_IO } } @@ -770,7 +793,7 @@ fn load(text: &str, strictness: Strictness) -> Result { .iter() .filter(|d| d.severity == Severity::Error) .count(); - eprintln!("strict load failed: {} error diagnostic(s)", errors); + errln!("strict load failed: {} error diagnostic(s)", errors); Err(6) } } @@ -780,7 +803,7 @@ fn load(text: &str, strictness: Strictness) -> Result { /// one element per line. fn do_get(o: &Opts) -> u8 { let [file, path] = o.args.as_slice() else { - eprintln!("usage: shcl get [type] [options] FILE PATH (see --help)"); + errln!("usage: shcl get [type] [options] FILE PATH (see --help)"); return 1; }; let (doc, diags) = match load_layered(o, file) { @@ -826,7 +849,7 @@ fn do_get(o: &Opts) -> u8 { ) } Kind::Raw | Kind::RawInfo => { - eprintln!("--{} has no --array form", o.kind.name()); + errln!("--{} has no --array form", o.kind.name()); return 1; } Kind::String => { @@ -903,9 +926,12 @@ fn do_get(o: &Opts) -> u8 { Status::Multiple => "the path matches multiple instances".to_string(), Status::Good => String::new(), // handled above; keep the match total }; - eprintln!( + errln!( "cannot read {} as {}: {} (in {})", - path, type_name, reason, file + path, + type_name, + reason, + file ); } match (status, o.on_bad) { @@ -974,11 +1000,11 @@ fn quoted(s: &str) -> String { fn do_fmt(o: &Opts) -> u8 { let [file] = o.args.as_slice() else { - eprintln!("usage: shcl fmt [--write|-w] [options] FILE (see --help)"); + errln!("usage: shcl fmt [--write|-w] [options] FILE (see --help)"); return 1; }; if o.write && file == "-" { - eprintln!("fmt --write cannot rewrite stdin; drop --write to print, or pass a FILE"); + errln!("fmt --write cannot rewrite stdin; drop --write to print, or pass a FILE"); return 1; } let (doc, diags) = match load_layered(o, file) { @@ -1134,11 +1160,11 @@ fn apply_op(doc: &mut Document, line: &str) -> Result<(), String> { fn do_set(o: &Opts) -> u8 { let [file] = o.args.as_slice() else { - eprintln!("usage: shcl set [--write|-w] [options] FILE (see --help)"); + errln!("usage: shcl set [--write|-w] [options] FILE (see --help)"); return 1; }; if o.write && file == "-" { - eprintln!("set --write cannot rewrite stdin; drop --write to print, or pass a FILE"); + errln!("set --write cannot rewrite stdin; drop --write to print, or pass a FILE"); return 1; } // Base doc: with the edits given as options no ops script is read, so a '-' @@ -1151,7 +1177,7 @@ fn do_set(o: &Opts) -> u8 { match read_input(lf) { Ok(t) => layer_texts.push(t), Err(e) => { - eprintln!("{}", e); + errln!("{}", e); return EXIT_IO; } } @@ -1168,7 +1194,7 @@ fn do_set(o: &Opts) -> u8 { match read_input(file) { Ok(t) => t, Err(e) => { - eprintln!("{}", e); + errln!("{}", e); return EXIT_IO; } } @@ -1190,7 +1216,7 @@ fn do_set(o: &Opts) -> u8 { } for s in &o.sets { if !s.apply(&mut doc) { - eprintln!( + errln!( "{}: cannot write {}: {}", s.opt(), s.path, @@ -1208,11 +1234,9 @@ fn do_set(o: &Opts) -> u8 { // silently, which reads as a hang rather than as a prompt; the note is // unconditional so a pipeline and a terminal behave identically. The // program-name prefix marks it as a notice; errors carry none. - eprintln!( - "shcl: reading write-ops from stdin (one op per line, tab-separated; end with EOF)" - ); + errln!("shcl: reading write-ops from stdin (one op per line, tab-separated; end with EOF)"); if let Err(e) = std::io::stdin().read_to_string(&mut ops) { - eprintln!("stdin: {}", e); + errln!("stdin: {}", e); return EXIT_IO; } } @@ -1222,7 +1246,7 @@ fn do_set(o: &Opts) -> u8 { continue; } if let Err(e) = apply_op(&mut doc, line) { - eprintln!("op line {}: {}", n + 1, e); + errln!("op line {}: {}", n + 1, e); return 1; } } @@ -1236,13 +1260,13 @@ fn do_set(o: &Opts) -> u8 { fn do_check(o: &Opts) -> u8 { let [file] = o.args.as_slice() else { - eprintln!("usage: shcl check [options] FILE (see --help)"); + errln!("usage: shcl check [options] FILE (see --help)"); return 1; }; let text = match read_input(file) { Ok(t) => t, Err(e) => { - eprintln!("{}", e); + errln!("{}", e); return EXIT_IO; } }; @@ -1256,7 +1280,7 @@ fn do_check(o: &Opts) -> u8 { let stext = match read_input(schema_file) { Ok(t) => t, Err(e) => { - eprintln!("{}", e); + errln!("{}", e); return EXIT_IO; } }; @@ -1267,9 +1291,12 @@ fn do_check(o: &Opts) -> u8 { .any(|d| d.severity == Severity::Error) { for d in sdoc.diagnostics() { - eprintln!( + errln!( "schema line {}: {:?}: {} {}", - d.line, d.severity, d.code, d.message + d.line, + d.severity, + d.code, + d.message ); } diags.push(Diagnostic { @@ -1315,17 +1342,17 @@ fn do_check(o: &Opts) -> u8 { fn do_init(o: &Opts) -> u8 { if !o.args.is_empty() { - eprintln!("init takes no file argument (see --help)"); + errln!("init takes no file argument (see --help)"); return 1; } let Some(schema_file) = &o.schema else { - eprintln!("init needs --schema=FILE (see --help)"); + errln!("init needs --schema=FILE (see --help)"); return 1; }; let stext = match read_input(schema_file) { Ok(t) => t, Err(e) => { - eprintln!("{}", e); + errln!("{}", e); return EXIT_IO; } }; @@ -1337,12 +1364,15 @@ fn do_init(o: &Opts) -> u8 { .any(|d| d.severity == Severity::Error) { for d in sdoc.diagnostics() { - eprintln!( + errln!( "schema line {}: {:?}: {} {}", - d.line, d.severity, d.code, d.message + d.line, + d.severity, + d.code, + d.message ); } - eprintln!("init: schema failed to load"); + errln!("init: schema failed to load"); // A broken schema is a config-semantics failure, not a usage error: // same exit as `check --schema` reporting it. return 6; @@ -1354,12 +1384,15 @@ fn do_init(o: &Opts) -> u8 { } Err(faults) => { for d in &faults { - eprintln!( + errln!( "schema line {}: {:?}: {} {}", - d.line, d.severity, d.code, d.message + d.line, + d.severity, + d.code, + d.message ); } - eprintln!("init: schema has faults"); + errln!("init: schema has faults"); 6 } } @@ -1368,7 +1401,7 @@ fn do_init(o: &Opts) -> u8 { fn do_enum(o: &Opts, want_count: bool) -> u8 { let [file, path] = o.args.as_slice() else { let name = if want_count { "count" } else { "instances" }; - eprintln!("usage: shcl {} [options] FILE PATH (see --help)", name); + errln!("usage: shcl {} [options] FILE PATH (see --help)", name); return 1; }; let (doc, diags) = match load_layered(o, file) { @@ -1398,7 +1431,7 @@ fn do_children(o: &Opts) -> u8 { [file] => (file.as_str(), ""), [file, path] => (file.as_str(), path.as_str()), _ => { - eprintln!("usage: shcl children [options] FILE [PATH] (see --help)"); + errln!("usage: shcl children [options] FILE [PATH] (see --help)"); return 1; } }; @@ -1417,7 +1450,7 @@ fn do_children(o: &Opts) -> u8 { /// deduplicated - the whole-document counterpart of `children`. fn do_paths(o: &Opts) -> u8 { let [file] = o.args.as_slice() else { - eprintln!("usage: shcl paths [options] FILE (see --help)"); + errln!("usage: shcl paths [options] FILE (see --help)"); return 1; }; let (doc, diags) = match load_layered(o, file) { @@ -1463,7 +1496,7 @@ fn run(cmd: &str, o: &Opts) -> u8 { "children" => do_children(o), "paths" => do_paths(o), other => { - eprintln!("{}: no dispatch arm (see --help)", other); + errln!("{}: no dispatch arm (see --help)", other); 1 } } @@ -1496,7 +1529,7 @@ fn run_profiled(cmd: &str, o: &Opts, out: &str) -> u8 { report .flamegraph(file) .expect("pprof: failed to write flamegraph"); - eprintln!("shcl: wrote flamegraph -> {}", out); + errln!("shcl: wrote flamegraph -> {}", out); code } @@ -1519,6 +1552,16 @@ fn reset_sigpipe() { fn reset_sigpipe() {} fn main() -> ExitCode { + let code = run_cli(); + // A tail still sitting in the buffer when the work is done fails the same + // way a write does. + if let Err(e) = std::io::Write::flush(&mut std::io::stdout()) { + write_failed(&e); + } + ExitCode::from(code) +} + +fn run_cli() -> u8 { reset_sigpipe(); let argv: Vec = match std::env::args_os() .skip(1) @@ -1527,8 +1570,8 @@ fn main() -> ExitCode { { Ok(v) => v, Err(_) => { - eprintln!("invalid argument encoding (expected UTF-8)"); - return ExitCode::from(1); + errln!("invalid argument encoding (expected UTF-8)"); + return 1; } }; let first = argv.first().map(|s| s.as_str()); @@ -1539,41 +1582,41 @@ fn main() -> ExitCode { // text unpadded and exit 1, which read as neither a help nor an error. if asked == Some("help") || first == Some("help") || argv.is_empty() { out!("\n{}\n", HELP); - return ExitCode::from(0); + return 0; } if asked == Some("version") || first == Some("version") { outln!("shcl {}", env!("CARGO_PKG_VERSION")); - return ExitCode::from(0); + return 0; } if asked == Some("about") || first == Some("about") { out!("\n{}\n", ABOUT); - return ExitCode::from(0); + return 0; } if asked == Some("donate") || first == Some("donate") { out!("\n{}\n", DONATE); - return ExitCode::from(0); + return 0; } let cmd = argv[0].clone(); if !COMMANDS.contains(&cmd.as_str()) { // Before the options are judged, so a typo in the command is reported // as that and not as an option the wrong command cannot take. if cmd.starts_with('-') && cmd != "--" { - eprintln!("unknown option: {} (see --help)", cmd); + errln!("unknown option: {} (see --help)", cmd); } else { - eprintln!("unknown command: {} (see --help)", cmd); + errln!("unknown command: {} (see --help)", cmd); } - return ExitCode::from(1); + return 1; } let o = match parse_opts(&argv[1..]) { Ok(o) => o, Err(e) => { - eprintln!("{}", e); - return ExitCode::from(1); + errln!("{}", e); + return 1; } }; #[cfg(feature = "profiling")] if let Ok(out) = std::env::var("SHCL_PROFILE_OUT") { - return ExitCode::from(run_profiled(&cmd, &o, &out)); + return run_profiled(&cmd, &o, &out); } - ExitCode::from(run(&cmd, &o)) + run(&cmd, &o) } From 333e23ea6f0ebfadd508c808f918ba58b55bdf96 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:43:50 -0700 Subject: [PATCH 07/27] free parse scratch in C Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- project/backlog.md | 6 +++++- source/c/shcl.h | 5 +++++ source/c/tests/mem_bounds.c | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/project/backlog.md b/project/backlog.md index ef984c5..cf301c2 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -125,10 +125,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170800 - Closed: 20260902-213000 - - 🔘 Item 10: a C parse leaves its temporaries in the scratch arena until the first resolve, so a parsed document holds about ten times its input in dead memory. + - ✅ Item 10: a C parse leaves its temporaries in the scratch arena until the first resolve, so a parsed document holds about ten times its input in dead memory. - Reproduced: a 49 MiB input parses to 970 MB RSS with 487 MB of it in `d->scratch`; 10 MiB gives 204 MB with 97 MB dead. One `arena_free(&d->scratch)` at the end of `do_parse` gives 766 MB and 157 MB with parse time unchanged, and 46,600 writer, merge and compact operations under ASan and UBSan with that free applied show nothing live pointed into it. - Cause: `parse_body` takes `P.tmp = &d->scratch` for its lines vector, per-parent maps, stack and pending lists, and `do_parse` frees only the two `own` arenas on exit. The header describes `scratch` as per-resolve temporaries reset on entry to each resolve, and says nothing about the parser using it. The 20260901 round's C amplification numbers were measured after a read and so did not see it. + - Fixed: `do_parse` gives the scratch arena back at its single exit. Every resolve resets it anyway, so nothing else changes. + - Measured: a 15 MB document holds 398 MB after the parse instead of 569 MB; peak is unchanged, since the scratch is live while the parse runs. + - Pinned by `mem_bounds.c`, which parses 20000 sections and requires the scratch arena to be empty afterwards. It held 12.3 MB against a 398 KB input before. The sanitizer run is clean, so nothing handed out points into it. - Opened: 20260902-170900 + - Closed: 20260902-215000 - 🔘 Item 11: a refused C setter still consumes document arena, against the header's "nothing is created on failure". - Reproduced: a refused 20 MB `set_string` on a wildcard path costs 85 MB of arena, permanently; ten refused `set_raw` calls with a 20 MB body cost 200 MB; small values cost 48 to 80 bytes per refused call. Rust builds the value first too but drops it when `place` refuses. diff --git a/source/c/shcl.h b/source/c/shcl.h index 882f722..d77da49 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -2741,6 +2741,11 @@ static shcl_doc *do_parse(const char *text, size_t len, shcl_strictness strict, arena_guard(&d->reads, NULL); arena_guard(&d->index_arena, NULL); arena_free(&own->line); arena_free(&own->hints); free(own->cmaps.data); free(own->dmaps.data); free(own); + /* The parser borrows scratch for its lines vector, per-parent maps, stack + and pending lists - about ten times the input, dead the moment the parse + ends. Every resolve resets it anyway, so a document nobody reads would + otherwise carry all of it until it was freed. */ + arena_free(&d->scratch); return d; } #if defined(__GNUC__) && !defined(__clang__) diff --git a/source/c/tests/mem_bounds.c b/source/c/tests/mem_bounds.c index ddd6942..a82ea18 100644 --- a/source/c/tests/mem_bounds.c +++ b/source/c/tests/mem_bounds.c @@ -75,6 +75,21 @@ int main(void) { if (dcapped > tlen * 8) fail("a diagnostic-capped parse held its unlisted diagnostics"); free(text); + // The parser borrows the scratch arena for its lines vector, per-parent + // maps, stack and pending lists - about ten times the input. It used to sit + // there until the first resolve, so a parsed document nobody read carried + // all of it. + tlen = 0; + text = (char *)malloc(reps * 24 + 1); + for (size_t i = 0; i < 20000; i++) tlen += (size_t)sprintf(text + tlen, "sect%zu:\n\tk: %zu\n", i, i); + d = shcl_parse(text, tlen); + size_t leftover = d ? arena_bytes(&d->scratch) : 0; + printf("mem_bounds: parse scratch: %zu bytes left for text %zu\n", leftover, tlen); + if (!d || shcl_get_int_or(d, "sect19999.k", 11, -1) != 19999) fail("scratch check: wrong result"); + if (leftover > 4096) fail("a parse left its temporaries in the scratch arena"); + shcl_free(d); + free(text); + // A read-only loop over a long-lived document, with shcl_reads_release // between passes, must stay flat: every read call, including the two that // take no path and so never pass through the path lookup's scratch reset. From f4c1e4acfb390dd7b611c4206b4049d62acd1412 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:55:59 -0700 Subject: [PATCH 08/27] refused C setters give the arena back Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- project/backlog.md | 7 ++-- source/c/shcl.h | 67 +++++++++++++++++++++++++------------ source/c/tests/mem_bounds.c | 18 ++++++++++ 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/project/backlog.md b/project/backlog.md index cf301c2..136d1f5 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -134,11 +134,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-170900 - Closed: 20260902-215000 - - 🔘 Item 11: a refused C setter still consumes document arena, against the header's "nothing is created on failure". + - ✅ Item 11: a refused C setter still consumes document arena, against the header's "nothing is created on failure". - Reproduced: a refused 20 MB `set_string` on a wildcard path costs 85 MB of arena, permanently; ten refused `set_raw` calls with a 20 MB body cost 200 MB; small values cost 48 to 80 bytes per refused call. Rust builds the value first too but drops it when `place` refuses. - Cause: every setter encodes the value into `d->arena` before `w_set` validates the path. Only `shcl_compact` gives it back. - - Note: probe the path first (`w_write_reason`), or encode into scratch and copy into the arena on success. + - Fixed: the arena takes a mark and a release, and every setter marks before it encodes; a refusal rolls the arena back to the mark. `w_place` allocates only in scratch until its own probe passes, so nothing but the encoded value sits past the mark. No extra path walk, which a probe-first fix would have cost every successful write. + - Measured: a refused 20 MB `set_string` costs nothing where it cost 55 MB; ten refused 20 MB `set_raw` calls cost nothing where they cost 265 MB; 10000 refused `set_int` calls cost nothing where they cost 480 KB. + - Pinned by `mem_bounds.c`, which refuses 24 MB of writes on a wildcard path and requires the arena not to grow. It grew 27 MB before. Sanitizers, the compiler sweep and cppcheck are clean. - Opened: 20260902-171000 + - Closed: 20260902-222000 - 🔘 Item 12: the named-month space forms accept a day token that is not `DD`. - Reproduced in all four. `Jul +12 2026`, `+12 Jul 2026`, `Jul 0012 2026` and `Jul 00000000000000012 2026` all read Good as `2026-07-12`, while `Jul-012-2026` is refused and `Jul 12 +2026` is refused because the year is held to four digits. The spec calls the format list a closed whitelist and spells the day `DD`. diff --git a/source/c/shcl.h b/source/c/shcl.h index d77da49..c79c6d4 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -550,6 +550,24 @@ static void *arena_grow(ShclArena *a, void *old, size_t oldcap, size_t newcap, s // reads never re-malloc. Bump arenas cannot free per-object, so without this // every resolver temporary would live until shcl_free - a long-running process // doing reads would grow without bound. +/* A point to roll back to. A bump arena cannot free one allocation, but it can + give back everything since a mark, which is what a setter needs when the + value it just encoded turns out to be refused. Only sound when nothing + allocated after the mark is still referenced. */ +typedef struct { ShclBlock *head; size_t used; void *last; size_t last_n; } ShclMark; + +static ShclMark arena_mark(ShclArena *a) { + ShclMark m; m.head = a->head; m.used = a->head ? a->head->used : 0; + m.last = a->last; m.last_n = a->last_n; + return m; +} + +static void arena_release(ShclArena *a, ShclMark m) { + while (a->head && a->head != m.head) { ShclBlock *n = a->head->next; free(a->head); a->head = n; } + if (a->head) a->head->used = m.used; + a->last = m.last; a->last_n = m.last_n; +} + static void arena_reset(ShclArena *a) { if (!a->head) return; ShclBlock *b = a->head->next; @@ -3373,9 +3391,14 @@ static void w_collapse_dup(shcl_doc *d, size_t node) { w_fold_dups_below(d, survivor); } -static int w_set(shcl_doc *d, ShclStr path, ShclValue v) { +/* The setters encode into the document arena before the path is validated, and + a bump arena never gives that back - a refused 20 MB write used to cost the + document 85 MB permanently. w_place allocates only in scratch until its + probe passes, so on a refusal nothing but the encoded value sits past the + mark. The mark is taken by the caller, before it encodes. */ +static int w_set_marked(shcl_doc *d, ShclStr path, ShclValue v, ShclMark m) { size_t idx; - if (!w_place(d, path, &idx)) return 0; + if (!w_place(d, path, &idx)) { arena_release(&d->arena, m); return 0; } NODE(d, idx).value = v; w_collapse_dup(d, idx); return 1; @@ -3440,15 +3463,15 @@ int shcl_set_comment(shcl_doc *d, const char *path, size_t plen, const char *tex return 1; } -int shcl_set_empty(shcl_doc *d, const char *path, size_t plen) { ShclStr p; p.p = path; p.n = plen; return w_set(d, p, v_empty()); } -int shcl_set_int(shcl_doc *d, const char *path, size_t plen, int64_t v) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; return w_set(d, p, w_cell1(a, w_int_text(a, v))); } +int shcl_set_empty(shcl_doc *d, const char *path, size_t plen) { ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(&d->arena); return w_set_marked(d, p, v_empty(), m); } +int shcl_set_int(shcl_doc *d, const char *path, size_t plen, int64_t v) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); return w_set_marked(d, p, w_cell1(a, w_int_text(a, v)), m); } /* An infinity or a NaN has no spelling the reader accepts, and neither does a datetime the reader would refuse (month 13, a fraction with no seconds, an empty struct): each fails the write rather than binding text that cannot read back. */ -int shcl_set_float(shcl_doc *d, const char *path, size_t plen, double v) { if (!isfinite(v)) return 0; ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; return w_set(d, p, w_cell1(a, w_float_text(a, v))); } -int shcl_set_bool(shcl_doc *d, const char *path, size_t plen, int v) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; return w_set(d, p, w_cell1(a, w_bool_text(v))); } -int shcl_set_string(shcl_doc *d, const char *path, size_t plen, const char *s, size_t slen) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr in; in.p = s; in.n = slen; return w_set(d, p, w_cell1(a, w_encode_string(a, in))); } +int shcl_set_float(shcl_doc *d, const char *path, size_t plen, double v) { if (!isfinite(v)) return 0; ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); return w_set_marked(d, p, w_cell1(a, w_float_text(a, v)), m); } +int shcl_set_bool(shcl_doc *d, const char *path, size_t plen, int v) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); return w_set_marked(d, p, w_cell1(a, w_bool_text(v)), m); } +int shcl_set_string(shcl_doc *d, const char *path, size_t plen, const char *s, size_t slen) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr in; in.p = s; in.n = slen; ShclMark m = arena_mark(a); return w_set_marked(d, p, w_cell1(a, w_encode_string(a, in)), m); } static int literal_value(ShclArena *a, ShclArena *tmp, ShclStr text, ShclValue *out) { for (size_t i = 0; i < text.n; i++) { if (text.p[i] == '\n' || text.p[i] == '\r') return 0; } @@ -3467,10 +3490,11 @@ static int literal_value(ShclArena *a, ShclArena *tmp, ShclStr text, ShclValue * int shcl_set_literal(shcl_doc *d, const char *path, size_t plen, const char *text, size_t tlen) { ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr in; in.p = text; in.n = tlen; ShclValue v; - if (!literal_value(a, &d->scratch, in, &v)) return 0; - return w_set(d, p, v); + ShclMark m = arena_mark(a); + if (!literal_value(a, &d->scratch, in, &v)) { arena_release(a, m); return 0; } + return w_set_marked(d, p, v, m); } -int shcl_set_datetime(shcl_doc *d, const char *path, size_t plen, const shcl_datetime *dt) { if (!dt_reads_back(&d->scratch, dt)) return 0; ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; return w_set(d, p, w_cell1(a, w_dt_text(a, dt))); } +int shcl_set_datetime(shcl_doc *d, const char *path, size_t plen, const shcl_datetime *dt) { if (!dt_reads_back(&d->scratch, dt)) return 0; ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); return w_set_marked(d, p, w_cell1(a, w_dt_text(a, dt)), m); } // Bind a raw block at a path, picking a fence longer than any content line. // The info-string is stored as a fence line would read it back (trimmed); one // holding a line break or an unquoted `#` has no fence-line spelling (the `#` @@ -3485,40 +3509,41 @@ int shcl_set_raw(shcl_doc *d, const char *path, size_t plen, const char *content if (icomment.n) return 0; for (size_t i = 0; i < clen; i++) if (content[i] == '\r' && (i + 1 == clen || content[i + 1] == '\n')) return 0; it = s_trim(it); + ShclMark m = arena_mark(a); ShclStr c = w_dupz(a, content, clen), inf = s_dup(a, it); unsigned char fc; size_t fl; w_choose_fence(c, &fc, &fl); ShclValue v; memset(&v, 0, sizeof v); v.kind = V_RAW; v.raw = (ShclRawVal *)arena_alloc(a, sizeof(ShclRawVal)); v.raw->content = c; v.raw->info = inf; v.raw->fence_char = fc; v.raw->fence_len = fl; - return w_set(d, p, v); + return w_set_marked(d, p, v, m); } int shcl_set_int_array(shcl_doc *d, const char *path, size_t plen, const int64_t *v, size_t n) { - ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); + ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); for (size_t i = 0; i < n; i++) t[i] = w_int_text(a, v[i]); - return w_set(d, p, w_array(a, t, n)); + return w_set_marked(d, p, w_array(a, t, n), m); } int shcl_set_float_array(shcl_doc *d, const char *path, size_t plen, const double *v, size_t n) { for (size_t i = 0; i < n; i++) if (!isfinite(v[i])) return 0; - ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); + ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); for (size_t i = 0; i < n; i++) t[i] = w_float_text(a, v[i]); - return w_set(d, p, w_array(a, t, n)); + return w_set_marked(d, p, w_array(a, t, n), m); } int shcl_set_bool_array(shcl_doc *d, const char *path, size_t plen, const int *v, size_t n) { - ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); + ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); for (size_t i = 0; i < n; i++) t[i] = w_bool_text(v[i]); - return w_set(d, p, w_array(a, t, n)); + return w_set_marked(d, p, w_array(a, t, n), m); } int shcl_set_string_array(shcl_doc *d, const char *path, size_t plen, const char *const *v, const size_t *lens, size_t n) { - ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); + ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); for (size_t i = 0; i < n; i++) { ShclStr in; in.p = v[i]; in.n = lens[i]; t[i] = w_encode_string(a, in); } - return w_set(d, p, w_array(a, t, n)); + return w_set_marked(d, p, w_array(a, t, n), m); } int shcl_set_datetime_array(shcl_doc *d, const char *path, size_t plen, const shcl_datetime *v, size_t n) { for (size_t i = 0; i < n; i++) if (!dt_reads_back(&d->scratch, &v[i])) return 0; - ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); + ShclArena *a = &d->arena; ShclStr p; p.p = path; p.n = plen; ShclMark m = arena_mark(a); ShclStr *t = (ShclStr *)arena_alloc(a, (n ? n : 1) * sizeof(ShclStr)); for (size_t i = 0; i < n; i++) t[i] = w_dt_text(a, &v[i]); - return w_set(d, p, w_array(a, t, n)); + return w_set_marked(d, p, w_array(a, t, n), m); } int shcl_set_int_default(shcl_doc *d, const char *path, size_t plen, int64_t v) { if (!shcl_exists(d, path, plen)) return shcl_set_int(d, path, plen, v); return 1; } diff --git a/source/c/tests/mem_bounds.c b/source/c/tests/mem_bounds.c index a82ea18..d4ba6c0 100644 --- a/source/c/tests/mem_bounds.c +++ b/source/c/tests/mem_bounds.c @@ -122,6 +122,24 @@ int main(void) { } shcl_free(d); + // A setter encodes into the document arena before the path is validated, so + // a refused write used to cost the document the whole encoded value, for as + // long as it lived. + d = shcl_parse("a: 1\n", 5); + size_t held = arena_bytes(&d->arena); + size_t big = 4u * 1024 * 1024; + char *blob = (char *)malloc(big); + memset(blob, 'x', big); + if (shcl_set_string(d, "a[*]", 4, blob, big)) fail("refused setter: the wildcard write was accepted"); + for (int i = 0; i < 5; i++) if (shcl_set_raw(d, "a[*]", 4, blob, big, "", 0)) fail("refused setter: the raw write was accepted"); + for (int i = 0; i < 10000; i++) if (shcl_set_int(d, "a[*]", 4, i)) fail("refused setter: the int write was accepted"); + size_t after = arena_bytes(&d->arena); + printf("mem_bounds: refused writes: arena %zu -> %zu over 24 MB refused\n", held, after); + if (after > held + 4096) fail("a refused setter kept the value it encoded"); + if (shcl_get_int_or(d, "a", 1, -1) != 1) fail("refused setter: the document changed"); + shcl_free(d); + free(blob); + // A write lands in a bump arena and the value it replaced stays behind, so // a loop rewriting one field grows the document until shcl_free. Compaction // is the way out: the rebuilt document holds what it now contains and no From f213d04d200af283a59da4c5b88ad56d3b61ecc0 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:58:28 -0700 Subject: [PATCH 09/27] named-month day is DD Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ project/backlog.md | 6 +++-- project/conformance/007-dates/expected.shcl | 3 +++ project/conformance/007-dates/input.shcl | 3 +++ project/conformance/007-dates/reads.tsv | 4 ++++ source/c/shcl.h | 14 +++++------- source/go/shcl.go | 23 +++++-------------- source/python/shcl.py | 25 +++++---------------- source/rust/src/lib.rs | 7 ++++-- 9 files changed, 36 insertions(+), 51 deletions(-) diff --git a/changelog.md b/changelog.md index 6ec5c1a..5c1cdda 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- The named-month date forms hold the day to `DD`. `Jul +12 2026`, `Jul 0012 2026` and `+12 Jul 2026` read as 12 July, because the space-separated spellings parsed the day as a plain integer where every delimited spelling holds it to one or two digits. The spec calls the format list a closed whitelist and spells the day `DD`. + - A stdout that cannot be written exits 8 instead of reporting success. `shcl fmt f.shcl > /dev/full` exited 0 with an empty stderr in three of the four CLIs and killed the Python one with an interpreter message; the help and the man page have said 8 for a stream that could not be written all along. A reader that closed early is still the quiet exit, since nobody is there to read a complaint. - A stderr that cannot be written no longer costs the document. The reference aborted with nothing on stdout at all when a diagnostic could not be printed, which turned an unwritable log into a lost `fmt`. Diagnostics are best-effort now; the exit code still carries the outcome. diff --git a/project/backlog.md b/project/backlog.md index 136d1f5..b48ec28 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -143,11 +143,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171000 - Closed: 20260902-222000 - - 🔘 Item 12: the named-month space forms accept a day token that is not `DD`. + - ✅ Item 12: the named-month space forms accept a day token that is not `DD`. - Reproduced in all four. `Jul +12 2026`, `+12 Jul 2026`, `Jul 0012 2026` and `Jul 00000000000000012 2026` all read Good as `2026-07-12`, while `Jul-012-2026` is refused and `Jul 12 +2026` is refused because the year is held to four digits. The spec calls the format list a closed whitelist and spells the day `DD`. - Cause: the space forms parse the day token with a full unsigned parse (Rust `u32` from_str, which takes a leading `+` and any leading zeros), mirrored deliberately by Python's `_parse_u32`, Go's `parseU32` and C's `parse_u32_lenient`; the delimited form uses the 1-2 digit `parse_num2`. - - Note: use `parse_num2` for the day in both space forms; corpus rows for `Jul +12 2026` and `Jul 012 2026` as BadType. + - Fixed: both space forms read the day with the same one-or-two-digit parse the delimited forms use. The lenient u32 parse had no other caller and is gone from all three ports. + - Pinned by corpus `007` (`Jul +12 2026`, `Jul 0012 2026`, `+12 Jul 2026`, all BadType). All four read them as 12 July before. - Opened: 20260902-171100 + - Closed: 20260902-224500 - 🔘 Item 13: Go's `LoadError.Diagnostics` and Python's `diagnostics()` and `LoadError` hand out the document's own list. - Reproduced: in Go, setting `le.Diagnostics[0].Message` on a strict-load error changes `doc.Diagnostics()[0].Message`; in Python `doc.diagnostics().clear()` takes `error_count()` to 0, and `LoadError.diagnostics` is the same list object. 20260901b item 23 fixed Go's `Diagnostics()` and the two suppressors and did not reach the third hand-out; Python was not looked at. diff --git a/project/conformance/007-dates/expected.shcl b/project/conformance/007-dates/expected.shcl index d867222..d65d11d 100644 --- a/project/conformance/007-dates/expected.shcl +++ b/project/conformance/007-dates/expected.shcl @@ -17,3 +17,6 @@ bad2: 2026-02-30 bad3: "25:00" bad4: "July twelfth" bad5: "12:30+€ab" +bad6: "Jul +12 2026" +bad7: "Jul 0012 2026" +bad8: "+12 Jul 2026" diff --git a/project/conformance/007-dates/input.shcl b/project/conformance/007-dates/input.shcl index 86512ac..361008c 100644 --- a/project/conformance/007-dates/input.shcl +++ b/project/conformance/007-dates/input.shcl @@ -17,3 +17,6 @@ bad2: 2026-02-30 bad3: "25:00" bad4: "July twelfth" bad5: "12:30+€ab" +bad6: "Jul +12 2026" +bad7: "Jul 0012 2026" +bad8: "+12 Jul 2026" diff --git a/project/conformance/007-dates/reads.tsv b/project/conformance/007-dates/reads.tsv index 188f90f..231462e 100644 --- a/project/conformance/007-dates/reads.tsv +++ b/project/conformance/007-dates/reads.tsv @@ -19,3 +19,7 @@ bad2 datetime - BadType bad3 datetime - BadType bad4 datetime - BadType bad5 datetime - BadType +bad6 datetime - BadType +bad7 datetime - BadType +bad8 datetime - BadType +- lost 0 - diff --git a/source/c/shcl.h b/source/c/shcl.h index c79c6d4..45601d6 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -1690,13 +1690,6 @@ static int parse_year4(ShclStr s, int32_t *out) { int32_t v = 0; for (size_t i = 0; i < s.n; i++) v = v * 10 + (s.p[i] - '0'); *out = v; return 1; } -static int parse_u32_lenient(ShclStr s, uint32_t *out) { - size_t i = 0; if (i < s.n && s.p[i] == '+') i++; - if (i >= s.n) return 0; - uint64_t v = 0; - for (; i < s.n; i++) { unsigned char c = (unsigned char)s.p[i]; if (!is_adigit(c)) return 0; v = v * 10 + (c - '0'); if (v > 0xFFFFFFFFull) return 0; } - *out = (uint32_t)v; return 1; -} static void split_ws(ShclArena *a, ShclStr s, ShclVecS *out) { size_t i = 0; while (i < s.n) { @@ -1723,12 +1716,15 @@ static ShclDatePart parse_date_part(ShclArena *a, ShclStr s) { ShclStr day_tok = toks.data[1]; if (day_tok.n > 0 && day_tok.p[day_tok.n - 1] == ',') day_tok = s_slice(day_tok, 0, day_tok.n - 1); uint32_t d; int32_t y; - if (parse_u32_lenient(day_tok, &d) && parse_year4(toks.data[2], &y) && valid_date(y, mm, d)) { r.ok = 1; r.y = y; r.m = mm; r.d = d; } + /* The day is DD, like every other form's: a plain integer parse + takes a leading '+' and any number of leading zeros, which the + whitelist does not list and the delimited spellings refuse. */ + if (parse_num2(day_tok, &d) && parse_year4(toks.data[2], &y) && valid_date(y, mm, d)) { r.ok = 1; r.y = y; r.m = mm; r.d = d; } return r; } if ((mm = month_from_name(a, toks.data[1]))) { uint32_t d; int32_t y; - if (parse_u32_lenient(toks.data[0], &d) && parse_year4(toks.data[2], &y) && valid_date(y, mm, d)) { r.ok = 1; r.y = y; r.m = mm; r.d = d; } + if (parse_num2(toks.data[0], &d) && parse_year4(toks.data[2], &y) && valid_date(y, mm, d)) { r.ok = 1; r.y = y; r.m = mm; r.d = d; } return r; } return r; diff --git a/source/go/shcl.go b/source/go/shcl.go index c759c18..1eadb4a 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -4763,22 +4763,6 @@ func validDate(y, m, d int) bool { return m >= 1 && m <= 12 && d >= 1 && d <= daysInMonth(y, m) } -// parseU32 mirrors the reference's u32 parse: optional '+', digits, 32-bit range. -func parseU32(s string) (int, bool) { - t := s - if t != "" && t[0] == '+' { - t = t[1:] - } - if t == "" || !allDigits(t) { - return 0, false - } - v, err := strconv.ParseUint(t, 10, 32) - if err != nil { - return 0, false - } - return int(v), true -} - // The Atoi error discards through this date cluster are safe: every input is // length-bounded and allDigits-checked first, so Atoi cannot fail on it. func parseYear4(s string) (int, bool) { @@ -4813,7 +4797,10 @@ func parseDatePart(s string) (y, m, d int, ok bool) { toks := strings.Fields(s) if len(toks) == 3 { if mo, found := monthFromName(toks[0]); found { - dv, ok1 := parseU32(strings.TrimSuffix(toks[1], ",")) + // The day is DD, like every other form's: a plain integer parse + // takes a leading '+' and any number of leading zeros, which the + // whitelist does not list and the delimited spellings refuse. + dv, ok1 := parseNum2(strings.TrimSuffix(toks[1], ",")) yv, ok2 := parseYear4(toks[2]) if ok1 && ok2 && validDate(yv, mo, dv) { return yv, mo, dv, true @@ -4821,7 +4808,7 @@ func parseDatePart(s string) (y, m, d int, ok bool) { return 0, 0, 0, false } if mo, found := monthFromName(toks[1]); found { - dv, ok1 := parseU32(toks[0]) + dv, ok1 := parseNum2(toks[0]) yv, ok2 := parseYear4(toks[2]) if ok1 && ok2 && validDate(yv, mo, dv) { return yv, mo, dv, true diff --git a/source/python/shcl.py b/source/python/shcl.py index 2db0ac3..27060b1 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -4282,24 +4282,6 @@ def _valid_date(y, m, d): return 1 <= m <= 12 and d >= 1 and d <= _days_in_month(y, m) -def _parse_u32(s): - # Rust u32 parse: optional leading '+', ASCII digits, range-checked. - if not s: - return None - body = s[1:] if s[0] == "+" else s - if not body or not _all_ascii_digits(body): - return None - # Length-gate before int(): CPython 3.11+ refuses >4300 decimal digits, but the - # reference just overflows. Leading zeros are legal and don't count toward range. - digits = body.lstrip("0") or "0" - if len(digits) > 10: - return None - n = int(digits) - if n > 2 ** 32 - 1: - return None - return n - - def _parse_year4(s): if len(s) == 4 and _all_ascii_digits(s): return int(s) @@ -4326,14 +4308,17 @@ def _parse_date_part(s): m = _month_from_name(toks[0]) if m is not None: day_tok = toks[1][:-1] if toks[1].endswith(",") else toks[1] - d = _parse_u32(day_tok) + # The day is DD, like every other form's: a plain integer parse takes + # a leading '+' and any number of leading zeros, which the whitelist + # does not list and the delimited spellings refuse. + d = _parse_num2(day_tok) y = _parse_year4(toks[2]) if d is None or y is None: return None return (y, m, d) if _valid_date(y, m, d) else None m = _month_from_name(toks[1]) if m is not None: - d = _parse_u32(toks[0]) + d = _parse_num2(toks[0]) y = _parse_year4(toks[2]) if d is None or y is None: return None diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 6672be7..3d676c1 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -4518,14 +4518,17 @@ fn parse_date_part(s: &str) -> Option<(i32, u32, u32)> { // Space-separated named-month forms; a comma may follow the day in "Mon DD, YYYY". let toks: Vec<&str> = s.split_whitespace().collect(); if toks.len() == 3 { + // The day is DD, like every other form's: a plain integer parse takes a + // leading '+' and any number of leading zeros, which the whitelist does + // not list and the delimited spellings already refuse. if let Some(m) = month_from_name(toks[0]) { let day_tok = toks[1].strip_suffix(',').unwrap_or(toks[1]); - let d: u32 = day_tok.parse().ok()?; + let d: u32 = parse_num2(day_tok)?; let y: i32 = parse_year4(toks[2])?; return valid_date(y, m, d).then_some((y, m, d)); } if let Some(m) = month_from_name(toks[1]) { - let d: u32 = toks[0].parse().ok()?; + let d: u32 = parse_num2(toks[0])?; let y: i32 = parse_year4(toks[2])?; return valid_date(y, m, d).then_some((y, m, d)); } From 40c514d1d5a310c37a9fefae5d7feb3068dc1a50 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:59:48 -0700 Subject: [PATCH 10/27] diagnostics hand out copies Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ project/backlog.md | 6 ++++-- source/go/shcl.go | 4 ++-- source/go/shcl_test.go | 11 +++++++++++ source/python/shcl.py | 9 ++++++--- source/python/tests/conformance.py | 17 +++++++++++++++++ 6 files changed, 42 insertions(+), 7 deletions(-) diff --git a/changelog.md b/changelog.md index 5c1cdda..c169642 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- Go's `LoadError` and Python's `diagnostics()` and `LoadError` hand back a copy. Each returned the document's own list, so a caller sorting or clearing what it was given silently changed what the document reported, and the document's next append landed in the caller's slot. Go's `Diagnostics()` was fixed for this in 2.0; these were the ones it missed. + - The named-month date forms hold the day to `DD`. `Jul +12 2026`, `Jul 0012 2026` and `+12 Jul 2026` read as 12 July, because the space-separated spellings parsed the day as a plain integer where every delimited spelling holds it to one or two digits. The spec calls the format list a closed whitelist and spells the day `DD`. - A stdout that cannot be written exits 8 instead of reporting success. `shcl fmt f.shcl > /dev/full` exited 0 with an empty stderr in three of the four CLIs and killed the Python one with an interpreter message; the help and the man page have said 8 for a stream that could not be written all along. A reader that closed early is still the quiet exit, since nobody is there to read a complaint. diff --git a/project/backlog.md b/project/backlog.md index b48ec28..52f2b8e 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -151,10 +151,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171100 - Closed: 20260902-224500 - - 🔘 Item 13: Go's `LoadError.Diagnostics` and Python's `diagnostics()` and `LoadError` hand out the document's own list. + - ✅ Item 13: Go's `LoadError.Diagnostics` and Python's `diagnostics()` and `LoadError` hand out the document's own list. - Reproduced: in Go, setting `le.Diagnostics[0].Message` on a strict-load error changes `doc.Diagnostics()[0].Message`; in Python `doc.diagnostics().clear()` takes `error_count()` to 0, and `LoadError.diagnostics` is the same list object. 20260901b item 23 fixed Go's `Diagnostics()` and the two suppressors and did not reach the third hand-out; Python was not looked at. - - Note: Go copies into the error the way `Diagnostics()` now does; Python returns `list(self.diags)` from `diagnostics()` and gives `LoadError` a copy. The suppressors keep their in-place contract, which mirrors Rust's `&mut Vec`. + - Fixed: Go's `LoadError` and Python's `diagnostics()` and `LoadError` each hand back a copy. The suppressors keep their in-place contract, which mirrors the reference's `&mut Vec`. + - Pinned by the Go aliasing test, extended to write through a `LoadError`, and by a new fixture in the Python runner that clears both what `diagnostics()` returned and what a failed strict load carried. Both fail on the old code. - Opened: 20260902-171200 + - Closed: 20260902-230000 - 🔘 Item 14: the Go CLI exits 1 where the other three exit 8 on an ops script that is not UTF-8. - Reproduced: `printf '\xff\n' | shcl set f.shcl` exits 8 in Rust, C and Python and 1 in Go. The same bytes as a document exit 8 in all four. The standing decision makes 1 the usage code alone. diff --git a/source/go/shcl.go b/source/go/shcl.go index 1eadb4a..3e7fccd 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -2474,7 +2474,7 @@ func ParseWith(text string, strictness Strictness) (*Document, error) { if strictness == Strict { for _, d := range doc.diags { if d.Severity == SeverityError { - return doc, &LoadError{Diagnostics: doc.diags, Document: doc} + return doc, &LoadError{Diagnostics: append([]Diagnostic(nil), doc.diags...), Document: doc} } } } @@ -2505,7 +2505,7 @@ func ParseLimited(text string, strictness Strictness, maxNodes, maxElements, max if strictness == Strict { for _, d := range doc.diags { if d.Severity == SeverityError { - return doc, &LoadError{Diagnostics: doc.diags, Document: doc} + return doc, &LoadError{Diagnostics: append([]Diagnostic(nil), doc.diags...), Document: doc} } } } diff --git a/source/go/shcl_test.go b/source/go/shcl_test.go index f014cc1..fc9aac2 100644 --- a/source/go/shcl_test.go +++ b/source/go/shcl_test.go @@ -1567,6 +1567,17 @@ func TestSuppressLeavesTheCallersDiagnosticsAlone(t *testing.T) { t.Fatalf("%s: returned the caller's or the document's own backing array", name) } } + // A failed strict load hands out the same list, and used to hand out the + // document's own: writing through it changed what the document reports. + _, err := ParseWith("a\n", Strict) + var le *LoadError + if !errors.As(err, &le) || len(le.Diagnostics) == 0 { + t.Fatal("want a strict load failure carrying diagnostics") + } + le.Diagnostics[0].Message = "rewritten by the caller" + if bad := le.Document.Diagnostics(); bad[0].Message == "rewritten by the caller" { + t.Fatal("LoadError shares the document's diagnostics list") + } } func TestConvenienceTierFallsBackOnlyOnGood(t *testing.T) { diff --git a/source/python/shcl.py b/source/python/shcl.py index 27060b1..0d4c23c 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -1989,7 +1989,7 @@ def parse_with(text: str, strictness: Strictness) -> Document: alongside the diagnostics.""" doc = _Parser().parse(text, strictness) if strictness == Strictness.Strict and any(d.severity == Severity.Error for d in doc.diags): - raise LoadError(doc.diags, doc) + raise LoadError(list(doc.diags), doc) return doc @staticmethod @@ -2018,7 +2018,7 @@ def parse_limited( p.max_diags = max_diags doc = p.parse(text, strictness) if strictness == Strictness.Strict and any(d.severity == Severity.Error for d in doc.diags): - raise LoadError(doc.diags, doc) + raise LoadError(list(doc.diags), doc) return doc @staticmethod @@ -2066,7 +2066,10 @@ def save_file_lossy(self, path: str | os.PathLike[str]) -> None: raise SaveFailed(err) def diagnostics(self) -> list[Diagnostic]: - return self.diags + # A copy: the reference hands out a borrowed view nobody can append to, + # and the document's own list would let a caller's edit and the + # document's next append overwrite each other. + return list(self.diags) def lost_count(self) -> int: """How many lines or values parsing dropped that canonical output cannot diff --git a/source/python/tests/conformance.py b/source/python/tests/conformance.py index 903dcaf..639a747 100644 --- a/source/python/tests/conformance.py +++ b/source/python/tests/conformance.py @@ -710,6 +710,23 @@ def main(): # Canonical output folds the case, as it always has, and escapes the tab. if sdoc3.to_canonical() != '"ab\\tcd": 2\n': raise SystemExit(f"canonical name spelling got {sdoc3.to_canonical()!r}") + # What a read hands out must not be the document's own list: a caller + # clearing it used to take the document's diagnostics with it, and a failed + # strict load handed out the same list again. Same fixture in Go. + adoc = shcl.Document.parse("a: 1\na: 2\n") + handed = adoc.diagnostics() + if not handed: + raise SystemExit("aliasing fixture: want a diagnostic to work with") + handed.clear() + if not adoc.diagnostics(): + raise SystemExit("diagnostics() handed out the document's own list") + try: + shcl.Document.parse_with("a\n", shcl.Strictness.Strict) + raise SystemExit("aliasing fixture: want a strict load failure") + except shcl.LoadError as e: + e.diagnostics.clear() + if e.document is not None and not e.document.diagnostics(): + raise SystemExit("LoadError shares the document's diagnostics list") from None # parse_limited: the caps exist because a document amplifies to many times # its byte size in memory, so read_file's byte cap alone cannot bound a # load. Same fixture in every runner. From 8e119c2915a5a95be326fd080213f338d9008363 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:00:30 -0700 Subject: [PATCH 11/27] go ops read exits 8 Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/cli-regress.bash | 2 ++ project/backlog.md | 5 ++++- source/go/cmd/shcl/main.go | 5 +++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cicd/utility/cli-regress.bash b/cicd/utility/cli-regress.bash index ee9c255..1de049b 100755 --- a/cicd/utility/cli-regress.bash +++ b/cicd/utility/cli-regress.bash @@ -154,6 +154,8 @@ rows=( 'children-quoted|children %T% db|-|0|host\n"odd.key"|-' 'children-missing|children %T% nope|-|0||-' 'paths-all|paths %T%|-|0|db\ndb.host\ndb."odd.key"\nweb\nweb.port|-' + ## 20260902 item 14: Go read a non-UTF-8 ops script as a usage error. + 'ops-not-utf8|set %F%|\xff\n|8|-|-' ## 20260902 items 8 and 9: a stdout that could not be written was reported ## as success by three CLIs, and a stderr that could not be written aborted ## the reference with nothing on stdout at all. diff --git a/project/backlog.md b/project/backlog.md index 52f2b8e..bd472a5 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -158,10 +158,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171200 - Closed: 20260902-230000 - - 🔘 Item 14: the Go CLI exits 1 where the other three exit 8 on an ops script that is not UTF-8. + - ✅ Item 14: the Go CLI exits 1 where the other three exit 8 on an ops script that is not UTF-8. - Reproduced: `printf '\xff\n' | shcl set f.shcl` exits 8 in Rust, C and Python and 1 in Go. The same bytes as a document exit 8 in all four. The standing decision makes 1 the usage code alone. - Cause: `main.go`'s ops read returns 1 after `utf8.Valid` fails; its `readInput` already takes the exit-8 path for a document. + - Fixed: the ops read takes the same exit-8 path its document read does. + - Pinned by a `cli-regress` row feeding a lone `0xff` to `set`. Go exited 1 before. - Opened: 20260902-171300 + - Closed: 20260902-231000 - 🔘 Item 15: a refused `--set` or a failing op suppresses the load's diagnostics. - Reproduced in all four. On a document with an `E014` line, `get --set='a[*]=1' f a` prints only the refusal at exit 1 and never the `E014`; same for an ops line the writer refuses. The help says every subcommand that loads a document prints the load's diagnostics once per run. diff --git a/source/go/cmd/shcl/main.go b/source/go/cmd/shcl/main.go index 99ff4b3..e294803 100644 --- a/source/go/cmd/shcl/main.go +++ b/source/go/cmd/shcl/main.go @@ -1457,10 +1457,11 @@ func doSet(o *opts) int { fmt.Fprintf(os.Stderr, "stdin: %s\n", err) return exitIO } - // The reference reads ops via read_to_string; mirror its UTF-8 failure. + // The reference reads ops via read_to_string; mirror its UTF-8 failure, + // which is a stream that could not be read, not a usage error. if !utf8.Valid(ops) { fmt.Fprintln(os.Stderr, "stdin: invalid UTF-8") - return 1 + return exitIO } } for n, line := range strings.Split(string(ops), "\n") { From aad66c7f7f329d4834ccf6bde2d4210f7befdfb4 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:07:29 -0700 Subject: [PATCH 12/27] load diagnostics before edits Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ cicd/utility/cli-regress.bash | 4 +++ project/backlog.md | 6 ++-- source/c/cmd/shcl/main.c | 20 ++++--------- source/go/cmd/shcl/main.go | 52 ++++++++++++++-------------------- source/python/cmd/shcl/main.py | 46 +++++++++++++----------------- source/rust/src/main.rs | 42 +++++++++++---------------- 7 files changed, 74 insertions(+), 98 deletions(-) diff --git a/changelog.md b/changelog.md index c169642..3ddbd43 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- A refused `--set` or a failing ops line no longer swallows the load's diagnostics. The edit was applied before anything was printed, so a `get --set` on a file with a dropped line reported the refusal and said nothing about the damage. The diagnostics belong to the load and now go out before any edit runs. + - Go's `LoadError` and Python's `diagnostics()` and `LoadError` hand back a copy. Each returned the document's own list, so a caller sorting or clearing what it was given silently changed what the document reported, and the document's next append landed in the caller's slot. Go's `Diagnostics()` was fixed for this in 2.0; these were the ones it missed. - The named-month date forms hold the day to `DD`. `Jul +12 2026`, `Jul 0012 2026` and `+12 Jul 2026` read as 12 July, because the space-separated spellings parsed the day as a plain integer where every delimited spelling holds it to one or two digits. The spec calls the format list a closed whitelist and spells the day `DD`. diff --git a/cicd/utility/cli-regress.bash b/cicd/utility/cli-regress.bash index 1de049b..f09b5c0 100755 --- a/cicd/utility/cli-regress.bash +++ b/cicd/utility/cli-regress.bash @@ -154,6 +154,10 @@ rows=( 'children-quoted|children %T% db|-|0|host\n"odd.key"|-' 'children-missing|children %T% nope|-|0||-' 'paths-all|paths %T%|-|0|db\ndb.host\ndb."odd.key"\nweb\nweb.port|-' + ## 20260902 item 15: a refused edit returned before the load's diagnostics + ## were printed, so a damaged file said nothing about the damage. + 'refused-set-still-reports|get --set=a[*]=1 %B% a|-|1|-|E015 missing colon' + 'refused-op-still-reports|set %B%|int\ta[*]\t1\n|1|-|E015 missing colon' ## 20260902 item 14: Go read a non-UTF-8 ops script as a usage error. 'ops-not-utf8|set %F%|\xff\n|8|-|-' ## 20260902 items 8 and 9: a stdout that could not be written was reported diff --git a/project/backlog.md b/project/backlog.md index bd472a5..4343e47 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -166,11 +166,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171300 - Closed: 20260902-231000 - - 🔘 Item 15: a refused `--set` or a failing op suppresses the load's diagnostics. + - ✅ Item 15: a refused `--set` or a failing op suppresses the load's diagnostics. - Reproduced in all four. On a document with an `E014` line, `get --set='a[*]=1' f a` prints only the refusal at exit 1 and never the `E014`; same for an ops line the writer refuses. The help says every subcommand that loads a document prints the load's diagnostics once per run. - Cause: `load_layered` returns the refusal before the caller's `say_diagnostics`, and `do_set` returns from the `--set` refusal and the op failure before its own call to it. - - Note: print the load's diagnostics before applying the edit list; they belong to the load, not to the edits. + - Fixed: the layered load prints its own diagnostics, before the edit list runs, in all four; the five callers that printed them afterwards no longer do, so a run still reports once. `set` does the same before its option edits and its ops script. + - Pinned by two `cli-regress` rows: a refused `--set` and a refused ops line on a document with an `E015`, each of which must still report it. All four printed only the refusal before. - Opened: 20260902-171400 + - Closed: 20260902-234500 - 🔘 Item 16: a layer's leading blank line survives the load but not the canonical form, so a merge of a file and a merge of its `fmt` differ. - Reproduced in all four. With `A` holding `a: 1` and `B` holding a blank line then `b: 2`, `fmt --layer=A B` prints `a: 1`, a blank, `b: 2`, while `fmt --layer=A <(fmt B)` prints no blank. Same with a leading comment and with a comment-only layer. In a 700-seed soak, 129 seeds (every layer beginning with a blank line) gave a different result with an in-memory merged document as `over` than with its reparse; as `base` it never differed. diff --git a/source/c/cmd/shcl/main.c b/source/c/cmd/shcl/main.c index 8ab0a29..a89ad66 100644 --- a/source/c/cmd/shcl/main.c +++ b/source/c/cmd/shcl/main.c @@ -417,6 +417,9 @@ static int load_layered(Opts *o, const char *file, LayeredDoc *out) { if (g) { shcl_free(dd); layered_free(out); return g; } layered_push_doc(out, dd); } + // The load's diagnostics belong to the load, so they go out before any edit + // runs: a refused --set used to return with nothing said about them. + say_layered_diagnostics(out); for (int i = 0; i < o->nsets; i++) { if (!set_apply(out->doc, &o->sets[i])) { layered_free(out); return 1; } } @@ -451,10 +454,6 @@ static int do_get(Opts *o) { LayeredDoc L; int gate = load_layered(o, file, &L); if (gate) return gate; shcl_doc *d = L.doc; - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - say_layered_diagnostics(&L); shcl_status status = SHCL_GOOD; const shcl_status *slotSts = NULL; size_t nSlots = 0; @@ -602,9 +601,6 @@ static int do_fmt(Opts *o) { } LayeredDoc L; int gate = load_layered(o, file, &L); if (gate) return gate; - // Printing the canonical form drops what the load dropped, the same as a - // rewrite does, so the diagnostics go out either way. - say_layered_diagnostics(&L); int rc; if (o->write) { rc = write_back(L.doc, file, o); @@ -810,6 +806,9 @@ static int do_set(Opts *o) { layered_push_doc(&L, dd); } shcl_doc *d = L.doc; + // The load's diagnostics belong to the load, so they go out before any edit + // runs: a refused --set or a failing op used to return with nothing said. + say_layered_diagnostics(&L); for (int i = 0; i < o->nsets; i++) { if (!set_apply(d, &o->sets[i])) { layered_free(&L); return 1; } } @@ -841,7 +840,6 @@ static int do_set(Opts *o) { } } if (rc == 0) { - say_layered_diagnostics(&L); if (o->write) rc = write_back(d, file, o); else { shcl_str c = shcl_to_canonical(d); fwrite(c.p, 1, c.n, stdout); } } @@ -971,10 +969,6 @@ static int do_enum(Opts *o, int want_count) { LayeredDoc L; int gate = load_layered(o, file, &L); if (gate) return gate; shcl_doc *d = L.doc; - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - say_layered_diagnostics(&L); if (want_count) printf("%zu\n", shcl_count(d, path, plen)); else { shcl_str *vals; size_t n = shcl_instances(d, path, plen, &vals); for (size_t i = 0; i < n; i++) outln(vals[i].p, vals[i].n); } layered_free(&L); return 0; @@ -1096,7 +1090,6 @@ static int do_children(Opts *o) { else { fprintf(stderr, "usage: shcl children [options] FILE [PATH] (see --help)\n"); return 1; } LayeredDoc L; int gate = load_layered(o, file, &L); if (gate) return gate; - say_layered_diagnostics(&L); shcl_str *names = NULL; size_t n = shcl_children(L.doc, path, strlen(path), &names); for (size_t i = 0; i < n; i++) { @@ -1113,7 +1106,6 @@ static int do_paths(Opts *o) { if (o->nargs != 1) { fprintf(stderr, "usage: shcl paths [options] FILE (see --help)\n"); return 1; } LayeredDoc L; int gate = load_layered(o, o->args[0], &L); if (gate) return gate; - say_layered_diagnostics(&L); shcl_str *ps = NULL; size_t n = shcl_paths(L.doc, &ps); for (size_t i = 0; i < n; i++) { fwrite(ps[i].p, 1, ps[i].n, stdout); putchar('\n'); } diff --git a/source/go/cmd/shcl/main.go b/source/go/cmd/shcl/main.go index e294803..7a0853a 100644 --- a/source/go/cmd/shcl/main.go +++ b/source/go/cmd/shcl/main.go @@ -786,47 +786,50 @@ func writeBack(doc *shcl.Document, file string, o *opts) int { // loadLayered loads file with o's lower-priority --layer files underneath and // its --set overrides on top - the layered-load fold. Every layer parses at the // requested strictness; a strict-load failure on any layer aborts like a -// single-file strict failure (exit 6). Returns (doc, diags, 0) or (nil, nil, code). +// single-file strict failure (exit 6). Returns (doc, 0) or (nil, code). // -// The diagnostics come back per layer, lowest first: a merge does not carry -// them over, so reading them off the merged document drops the ones for FILE -// itself, which is the one the caller named. -func loadLayered(o *opts, file string) (*shcl.Document, []shcl.Diagnostic, int) { +// It prints every layer's diagnostics itself, lowest first, before the --set +// overrides run: they belong to the load, and a refused edit used to return +// with nothing said about them. A merge does not carry diagnostics over, so +// reading them off the merged document drops the ones for FILE itself, which +// is the one the caller named. +func loadLayered(o *opts, file string) (*shcl.Document, int) { texts := make([]string, 0, len(o.layers)+1) for _, lf := range o.layers { t, err := readInput(lf) if err != nil { fmt.Fprintln(os.Stderr, err) - return nil, nil, exitIO + return nil, exitIO } texts = append(texts, t) } base, err := readInput(file) if err != nil { fmt.Fprintln(os.Stderr, err) - return nil, nil, exitIO + return nil, exitIO } texts = append(texts, base) doc, code := loadDoc(texts[0], o.strictness) if code != 0 { - return nil, nil, code + return nil, code } diags := append([]shcl.Diagnostic(nil), doc.Diagnostics()...) for _, t := range texts[1:] { over, c := loadDoc(t, o.strictness) if c != 0 { - return nil, nil, c + return nil, c } diags = append(diags, over.Diagnostics()...) doc.Merge(over) } + sayDiagnostics(diags) for _, s := range o.sets { if !s.apply(doc) { fmt.Fprintf(os.Stderr, "%s: cannot write %s: %s\n", s.opt(), s.path, describeRefusal(doc, s.path)) - return nil, nil, 1 + return nil, 1 } } - return doc, diags, 0 + return doc, 0 } // doGet: one value read, formatted for the shell: scalars print as one line, @@ -837,14 +840,10 @@ func doGet(o *opts) int { return 1 } file, path := o.args[0], o.args[1] - doc, diags, code := loadLayered(o, file) + doc, code := loadLayered(o, file) if doc == nil { return code } - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - sayDiagnostics(diags) var lines []string var status shcl.Status var slots []shcl.Status @@ -1035,13 +1034,10 @@ func doFmt(o *opts) int { fmt.Fprintln(os.Stderr, "fmt --write cannot rewrite stdin; drop --write to print, or pass a FILE") return 1 } - doc, diags, code := loadLayered(o, file) + doc, code := loadLayered(o, file) if doc == nil { return code } - // Printing the canonical form drops what the load dropped, the same as a - // rewrite does, so the diagnostics go out either way. - sayDiagnostics(diags) if o.write { return writeBack(doc, file, o) } @@ -1436,6 +1432,9 @@ func doSet(o *opts) int { diags = append(diags, over.Diagnostics()...) doc.Merge(over) } + // The load's diagnostics belong to the load, so they go out before any edit + // runs: a refused --set or a failing op used to return with nothing said. + sayDiagnostics(diags) for _, s := range o.sets { if !s.apply(doc) { fmt.Fprintf(os.Stderr, "%s: cannot write %s: %s\n", s.opt(), s.path, describeRefusal(doc, s.path)) @@ -1474,7 +1473,6 @@ func doSet(o *opts) int { return 1 } } - sayDiagnostics(diags) if o.write { return writeBack(doc, file, o) } @@ -1610,14 +1608,10 @@ func doEnum(o *opts, wantCount bool) int { return 1 } file, path := o.args[0], o.args[1] - doc, diags, code := loadLayered(o, file) + doc, code := loadLayered(o, file) if doc == nil { return code } - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - sayDiagnostics(diags) if wantCount { outln(doc.Count(path)) } else { @@ -1643,11 +1637,10 @@ func doChildren(o *opts) int { fmt.Fprintln(os.Stderr, "usage: shcl children [options] FILE [PATH] (see --help)") return 1 } - doc, diags, code := loadLayered(o, file) + doc, code := loadLayered(o, file) if doc == nil { return code } - sayDiagnostics(diags) for _, name := range doc.Children(path) { outln(shcl.QuoteSegment(name)) } @@ -1661,11 +1654,10 @@ func doPaths(o *opts) int { fmt.Fprintln(os.Stderr, "usage: shcl paths [options] FILE (see --help)") return 1 } - doc, diags, code := loadLayered(o, o.args[0]) + doc, code := loadLayered(o, o.args[0]) if doc == nil { return code } - sayDiagnostics(diags) for _, p := range doc.Paths() { outln(p) } diff --git a/source/python/cmd/shcl/main.py b/source/python/cmd/shcl/main.py index 4dc3d4c..58fc500 100755 --- a/source/python/cmd/shcl/main.py +++ b/source/python/cmd/shcl/main.py @@ -473,29 +473,32 @@ def load_layered(o, file): # Load file with o's lower-priority --layer files underneath and its --set # overrides on top - the layered-load fold. Every layer parses at the # requested strictness; a strict-load failure on any layer aborts like a - # single-file strict failure. Returns (doc, diags, None) or (None, None, code). - # The diagnostics come back per layer, lowest first: a merge does not carry - # them over, so reading them off the merged document drops the ones for FILE - # itself, which is the one the caller named. + # single-file strict failure. Returns (doc, None) or (None, code). + # It prints every layer's diagnostics itself, lowest first, before the --set + # overrides run: they belong to the load, and a refused edit used to return + # with nothing said about them. A merge does not carry diagnostics over, so + # reading them off the merged document drops the ones for FILE itself, which + # is the one the caller named. texts = [] for lf in o.layers: texts.append(read_input(lf)) texts.append(read_input(file)) doc, code = load_doc(texts[0], o.strictness) if doc is None: - return None, None, code + return None, code diags = list(doc.diagnostics()) for t in texts[1:]: over, c = load_doc(t, o.strictness) if over is None: - return None, None, c + return None, c diags.extend(over.diagnostics()) doc.merge(over) + say_diagnostics(diags) for st in o.sets: if not st.apply(doc): sys.stderr.write(f"{st.opt()}: cannot write {st.path}: {describe_refusal(doc, st.path)}\n") - return None, None, 1 - return doc, diags, None + return None, 1 + return doc, None def check_opts(cmd, o): @@ -607,16 +610,12 @@ def do_get(o): return 1 file, path = o.args[0], o.args[1] try: - doc, diags, code = load_layered(o, file) + doc, code = load_layered(o, file) except (OSError, ValueError) as e: sys.stderr.write(str(e) + "\n") return EXIT_IO if doc is None: return code - # A read reports what the load dropped, the same as fmt and set: below - # strict the value comes back fine and the damage is otherwise silent. - # One report per invocation, so a read in a loop is one line per call. - say_diagnostics(diags) if o.array: if o.kind == "int": r = doc.read_int_array(path) @@ -754,15 +753,12 @@ def do_fmt(o): sys.stderr.write("fmt --write cannot rewrite stdin; drop --write to print, or pass a FILE\n") return 1 try: - doc, diags, code = load_layered(o, file) + doc, code = load_layered(o, file) except (OSError, ValueError) as e: sys.stderr.write(str(e) + "\n") return EXIT_IO if doc is None: return code - # Printing the canonical form drops what the load dropped, the same as a - # rewrite does, so the diagnostics go out either way. - say_diagnostics(diags) if o.write: return write_back(doc, file, o) sys.stdout.write(doc.to_canonical()) @@ -978,6 +974,9 @@ def do_set(o): return c diags.extend(over.diagnostics()) doc.merge(over) + # The load's diagnostics belong to the load, so they go out before any edit + # runs: a refused --set or a failing op used to return with nothing said. + say_diagnostics(diags) for st in o.sets: if not st.apply(doc): sys.stderr.write(f"{st.opt()}: cannot write {st.path}: {describe_refusal(doc, st.path)}\n") @@ -1015,7 +1014,6 @@ def do_set(o): except ValueError as e: sys.stderr.write(f"op line {n + 1}: {e}\n") return 1 - say_diagnostics(diags) if o.write: return write_back(doc, file, o) sys.stdout.write(doc.to_canonical()) @@ -1113,16 +1111,12 @@ def do_enum(o, want_count): return 1 file, path = o.args[0], o.args[1] try: - doc, diags, code = load_layered(o, file) + doc, code = load_layered(o, file) except (OSError, ValueError) as e: sys.stderr.write(str(e) + "\n") return EXIT_IO if doc is None: return code - # A read reports what the load dropped, the same as fmt and set: below - # strict the value comes back fine and the damage is otherwise silent. - # One report per invocation, so a read in a loop is one line per call. - say_diagnostics(diags) if want_count: print(doc.count(path)) else: @@ -1144,13 +1138,12 @@ def do_children(o): sys.stderr.write("usage: shcl children [options] FILE [PATH] (see --help)\n") return 1 try: - doc, diags, code = load_layered(o, file) + doc, code = load_layered(o, file) except (OSError, ValueError) as e: sys.stderr.write(str(e) + "\n") return EXIT_IO if doc is None: return code - say_diagnostics(diags) for name in doc.children(path): print(shcl.quote_segment(name)) return 0 @@ -1163,13 +1156,12 @@ def do_paths(o): sys.stderr.write("usage: shcl paths [options] FILE (see --help)\n") return 1 try: - doc, diags, code = load_layered(o, o.args[0]) + doc, code = load_layered(o, o.args[0]) except (OSError, ValueError) as e: sys.stderr.write(str(e) + "\n") return EXIT_IO if doc is None: return code - say_diagnostics(diags) for p in doc.paths(): print(p) return 0 diff --git a/source/rust/src/main.rs b/source/rust/src/main.rs index 0714ece..b569815 100644 --- a/source/rust/src/main.rs +++ b/source/rust/src/main.rs @@ -696,11 +696,13 @@ fn say_diagnostics(diags: &[Diagnostic]) { /// requested strictness; a strict-load failure on any layer aborts like a /// single-file strict failure (exit 6, nothing printed). /// -/// Returns every layer's diagnostics alongside the merged document, lowest -/// layer first. A merge does not carry them over, so the merged document only -/// holds the lowest layer's - reading them off it drops the diagnostics for -/// FILE itself, which is the one the caller named. -fn load_layered(o: &Opts, file: &str) -> Result<(Document, Vec), u8> { +/// Prints every layer's diagnostics itself, lowest layer first, before the +/// `--set` overrides run: they belong to the load, and a refused edit used to +/// return before anything was said about them. A merge does not carry +/// diagnostics over, so the merged document only holds the lowest layer's - +/// reading them off it drops the diagnostics for FILE itself, which is the one +/// the caller named. +fn load_layered(o: &Opts, file: &str) -> Result { // Lowest -> highest file layer: the --layer files in order, then FILE. let mut texts: Vec = Vec::with_capacity(o.layers.len() + 1); for lf in &o.layers { @@ -721,6 +723,7 @@ fn load_layered(o: &Opts, file: &str) -> Result<(Document, Vec), u8> diags.extend_from_slice(over.diagnostics()); doc.merge(&over); } + say_diagnostics(&diags); for s in &o.sets { if !s.apply(&mut doc) { errln!( @@ -732,7 +735,7 @@ fn load_layered(o: &Opts, file: &str) -> Result<(Document, Vec), u8> return Err(1); } } - Ok((doc, diags)) + Ok(doc) } /// The in-place half of `fmt`/`set`. Overwriting the source is the one place a @@ -806,14 +809,10 @@ fn do_get(o: &Opts) -> u8 { errln!("usage: shcl get [type] [options] FILE PATH (see --help)"); return 1; }; - let (doc, diags) = match load_layered(o, file) { + let doc = match load_layered(o, file) { Ok(d) => d, Err(code) => return code, }; - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - say_diagnostics(&diags); let (lines, status, slots): (Vec, Status, Vec) = if o.array { match o.kind { Kind::Int => { @@ -1007,13 +1006,10 @@ fn do_fmt(o: &Opts) -> u8 { errln!("fmt --write cannot rewrite stdin; drop --write to print, or pass a FILE"); return 1; } - let (doc, diags) = match load_layered(o, file) { + let doc = match load_layered(o, file) { Ok(d) => d, Err(code) => return code, }; - // Printing the canonical form drops what the load dropped, the same as a - // rewrite does, so the diagnostics go out either way. - say_diagnostics(&diags); if o.write { return write_back(&doc, file, o); } @@ -1214,6 +1210,9 @@ fn do_set(o: &Opts) -> u8 { Err(code) => return code, } } + // The load's diagnostics belong to the load, so they go out before any edit + // runs: a refused --set or a failing op used to return with nothing said. + say_diagnostics(&diags); for s in &o.sets { if !s.apply(&mut doc) { errln!( @@ -1250,7 +1249,6 @@ fn do_set(o: &Opts) -> u8 { return 1; } } - say_diagnostics(&diags); if o.write { return write_back(&doc, file, o); } @@ -1404,14 +1402,10 @@ fn do_enum(o: &Opts, want_count: bool) -> u8 { errln!("usage: shcl {} [options] FILE PATH (see --help)", name); return 1; }; - let (doc, diags) = match load_layered(o, file) { + let doc = match load_layered(o, file) { Ok(d) => d, Err(code) => return code, }; - // A read reports what the load dropped, the same as fmt and set: below - // strict the value comes back fine and the damage is otherwise silent. - // One report per invocation, so a read in a loop is one line per call. - say_diagnostics(&diags); if want_count { outln!("{}", doc.count(path)); } else { @@ -1435,11 +1429,10 @@ fn do_children(o: &Opts) -> u8 { return 1; } }; - let (doc, diags) = match load_layered(o, file) { + let doc = match load_layered(o, file) { Ok(d) => d, Err(code) => return code, }; - say_diagnostics(&diags); for name in doc.children(path) { outln!("{}", shcl::quote_segment(&name)); } @@ -1453,11 +1446,10 @@ fn do_paths(o: &Opts) -> u8 { errln!("usage: shcl paths [options] FILE (see --help)"); return 1; }; - let (doc, diags) = match load_layered(o, file) { + let doc = match load_layered(o, file) { Ok(d) => d, Err(code) => return code, }; - say_diagnostics(&diags); for p in doc.paths() { outln!("{}", p); } From 85591b6401babf78b264f25ee3a016015a463f4b Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:19:34 -0700 Subject: [PATCH 13/27] drop the blank before the first line Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ project/backlog.md | 7 +++-- .../expected-diags.txt | 1 + .../expected-merged.shcl | 5 ++++ .../083-merge-leading-blank/expected.shcl | 3 +++ .../083-merge-leading-blank/input.shcl | 5 ++++ .../083-merge-leading-blank/layer1.shcl | 1 + .../083-merge-leading-blank/layer2.shcl | 2 ++ .../083-merge-leading-blank/reads.tsv | 4 +++ source/c/shcl.h | 26 ++++++++++++++++--- source/go/shcl.go | 23 +++++++++++++++- source/python/shcl.py | 21 ++++++++++++++- source/rust/src/lib.rs | 16 +++++++++++- source/rust/tests/fuzz_smoke.rs | 15 +++++++++++ 14 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 project/conformance/083-merge-leading-blank/expected-diags.txt create mode 100644 project/conformance/083-merge-leading-blank/expected-merged.shcl create mode 100644 project/conformance/083-merge-leading-blank/expected.shcl create mode 100644 project/conformance/083-merge-leading-blank/input.shcl create mode 100644 project/conformance/083-merge-leading-blank/layer1.shcl create mode 100644 project/conformance/083-merge-leading-blank/layer2.shcl create mode 100644 project/conformance/083-merge-leading-blank/reads.tsv diff --git a/changelog.md b/changelog.md index 3ddbd43..a1bfe48 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- A blank line before the first thing canonical output prints is dropped at load. Canonical output never starts with a blank, so a document that kept the flag did not survive its own canonical form: merging a layer gave a different result from merging its `fmt`, and the fold placed a blank line the author never wrote. Three shapes did it - a file starting with a blank line, a blank after a leading line the load dropped, and a blank on a later instance that merged into the first. + - A refused `--set` or a failing ops line no longer swallows the load's diagnostics. The edit was applied before anything was printed, so a `get --set` on a file with a dropped line reported the refusal and said nothing about the damage. The diagnostics belong to the load and now go out before any edit runs. - Go's `LoadError` and Python's `diagnostics()` and `LoadError` hand back a copy. Each returned the document's own list, so a caller sorting or clearing what it was given silently changed what the document reported, and the document's next append landed in the caller's slot. Go's `Diagnostics()` was fixed for this in 2.0; these were the ones it missed. diff --git a/project/backlog.md b/project/backlog.md index 4343e47..b33b87e 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -174,11 +174,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171400 - Closed: 20260902-234500 - - 🔘 Item 16: a layer's leading blank line survives the load but not the canonical form, so a merge of a file and a merge of its `fmt` differ. + - ✅ Item 16: a layer's leading blank line survives the load but not the canonical form, so a merge of a file and a merge of its `fmt` differ. - Reproduced in all four. With `A` holding `a: 1` and `B` holding a blank line then `b: 2`, `fmt --layer=A B` prints `a: 1`, a blank, `b: 2`, while `fmt --layer=A <(fmt B)` prints no blank. Same with a leading comment and with a comment-only layer. In a 700-seed soak, 129 seeds (every layer beginning with a blank line) gave a different result with an in-memory merged document as `over` than with its reparse; as `base` it never differed. - Cause: the parser sets the pending blank on the first bound node like any other and the emitter suppresses it only at output start, so `load(emit(load(B)))` differs from `load(B)` on that one bit and `merge` copies it unchanged. - - Note: drop the blank at parse time when nothing precedes it, in all four, so no emitter or merge special case is needed; a corpus case whose layer starts with a blank line pins it. + - Fixed: the parse clears the blank on whatever canonical output would print first - the first root child, its first leading comment, or the first footer line - in all four. One place, so no emitter or merge special case is needed. + - Note: the property found two more shapes of the same defect that the finding did not name: a blank after a leading line the load dropped (a BOM-led one), and a blank on a later instance that merged into the first. Clearing at the emitted-first position covers all three; dropping it only at the start of the file covered one. + - Pinned by corpus `083` (a layer leading with two blank lines, plus a comment-only layer leading with one) and by a new fuzz property: merging a layer must equal merging its canonical form. Both failed in all four before; the property fails within 30000 iterations on each of the three shapes. - Opened: 20260902-171500 + - Closed: 20260903-001500 - 🔘 Item 17: the crosscheck's float-spelling dimension writes `0` for every subnormal power of two under gawk, so its "every power of two" claim is false on this box. - Reproduced: `gawk 'BEGIN{printf "%.17g", 2^-1074}'` prints `0` (it computes a negative power as `1/(2^1074)`, which is `1/inf`); mawk prints the subnormal. The generated ops file starts with 52 `float p 0` rows. The hosted runner's default awk decides which rows it exercises there, and `srand(20260902); rand()` differs per awk as well, so the "fixed" random set is not fixed either. diff --git a/project/conformance/083-merge-leading-blank/expected-diags.txt b/project/conformance/083-merge-leading-blank/expected-diags.txt new file mode 100644 index 0000000..4e7ad2e --- /dev/null +++ b/project/conformance/083-merge-leading-blank/expected-diags.txt @@ -0,0 +1 @@ +ok (0 diagnostic(s)) diff --git a/project/conformance/083-merge-leading-blank/expected-merged.shcl b/project/conformance/083-merge-leading-blank/expected-merged.shcl new file mode 100644 index 0000000..03b2b58 --- /dev/null +++ b/project/conformance/083-merge-leading-blank/expected-merged.shcl @@ -0,0 +1,5 @@ +a: 1 +b: 2 + +c: 3 +# only a comment diff --git a/project/conformance/083-merge-leading-blank/expected.shcl b/project/conformance/083-merge-leading-blank/expected.shcl new file mode 100644 index 0000000..ceeb01d --- /dev/null +++ b/project/conformance/083-merge-leading-blank/expected.shcl @@ -0,0 +1,3 @@ +b: 2 + +c: 3 diff --git a/project/conformance/083-merge-leading-blank/input.shcl b/project/conformance/083-merge-leading-blank/input.shcl new file mode 100644 index 0000000..9ee3540 --- /dev/null +++ b/project/conformance/083-merge-leading-blank/input.shcl @@ -0,0 +1,5 @@ + + +b: 2 + +c: 3 diff --git a/project/conformance/083-merge-leading-blank/layer1.shcl b/project/conformance/083-merge-leading-blank/layer1.shcl new file mode 100644 index 0000000..a8926a5 --- /dev/null +++ b/project/conformance/083-merge-leading-blank/layer1.shcl @@ -0,0 +1 @@ +a: 1 diff --git a/project/conformance/083-merge-leading-blank/layer2.shcl b/project/conformance/083-merge-leading-blank/layer2.shcl new file mode 100644 index 0000000..a3f0dbb --- /dev/null +++ b/project/conformance/083-merge-leading-blank/layer2.shcl @@ -0,0 +1,2 @@ + +# only a comment diff --git a/project/conformance/083-merge-leading-blank/reads.tsv b/project/conformance/083-merge-leading-blank/reads.tsv new file mode 100644 index 0000000..a99254d --- /dev/null +++ b/project/conformance/083-merge-leading-blank/reads.tsv @@ -0,0 +1,4 @@ +query type expected status +- load ok - +b int 2 Good +- lost 0 - diff --git a/source/c/shcl.h b/source/c/shcl.h index 45601d6..0f2052d 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -1971,7 +1971,11 @@ DEFINE_VEC(ShclVecPendMark, ShclPendMark) do_parse's frame: the recovery path is reached by longjmp, which leaves a local the parse has written to indeterminate. */ typedef struct { ShclArena line, hints; ShclVecMapPtr cmaps, dmaps; } ShclParseOwn; -typedef struct { shcl_doc *d; ShclArena *tmp; ShclArena *line; ShclArena *hints; ShclStr src; ShclVecStack stack; ShclVecMapPtr *cmaps; ShclVecMapPtr *dmaps; ShclVecPend pending; ShclVecPendMark pend_marks; int star_open; size_t star_node; uint64_t star_key; uint64_t star_disp; int saw_blank; ShclVecSize reent_node; ShclVecSize reent_line; +typedef struct { shcl_doc *d; ShclArena *tmp; ShclArena *line; ShclArena *hints; ShclStr src; ShclVecStack stack; ShclVecMapPtr *cmaps; ShclVecMapPtr *dmaps; ShclVecPend pending; ShclVecPendMark pend_marks; int star_open; size_t star_node; uint64_t star_key; uint64_t star_disp; int saw_blank; + /* Nothing has been read yet, so a blank line here leads the file and the + emitter would not re-emit it. Dropping it at parse time is what makes + load(emit(load(x))) equal load(x) on that bit, which a merge relies on. */ + int at_start; ShclVecSize reent_node; ShclVecSize reent_line; /* shcl_parse_limited's caps, 0 = uncapped: nodes counted against the arena (root excluded), elements against a single value's cell. */ size_t max_nodes, max_elements; @@ -2500,7 +2504,7 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t for the whole parse. Everything a node keeps is dup'd into the document arena before the next reset. */ ShclParser P; P.d = d; P.tmp = &d->scratch; P.line = &own->line; P.hints = &own->hints; P.cmaps = &own->cmaps; P.dmaps = &own->dmaps; memset(&P.stack, 0, sizeof P.stack); memset(&P.pending, 0, sizeof P.pending); memset(&P.pend_marks, 0, sizeof P.pend_marks); - P.star_open = 0; P.star_node = 0; P.star_key = 0; P.star_disp = 0; P.saw_blank = 0; + P.star_open = 0; P.star_node = 0; P.star_key = 0; P.star_disp = 0; P.saw_blank = 0; P.at_start = 1; P.max_nodes = max_nodes; P.max_elements = max_elements; P.max_diags = max_diags; P.unlisted_errors = 0; P.unlisted_hints = 0; memset(&P.reent_node, 0, sizeof P.reent_node); memset(&P.reent_line, 0, sizeof P.reent_line); ShclStackEnt e0; e0.indent = s_empty(); e0.node = ROOT; ShclVecStack_push(P.tmp, &P.stack, e0); @@ -2550,7 +2554,8 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t size_t ind = 0; while (ind < line.n && (line.p[ind] == ' ' || line.p[ind] == '\t')) ind++; ShclStr indent = s_slice(line, 0, ind); ShclStr rest = s_slice(line, ind, line.n); - if (rest.n == 0) { P.saw_blank = 1; i++; continue; } + if (rest.n == 0) { P.saw_blank = !P.at_start; i++; continue; } + P.at_start = 0; /* Whole-line comment: hold it for the next line that binds a node. It consumes a pending blank into its own flag, so a blank between comment-only regions survives the round-trip. Text and indent are @@ -2701,6 +2706,21 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t hang_deeper_pending(&P, s_empty()); for (size_t k = 0; k < P.pending.len; k++) ShclVecLead_push(a, &d->orphans, lead_make(P.pending.data[k].text, P.pending.data[k].blank_before)); + /* The emitter drops a blank before the first thing it prints, so a document + that kept one there would not survive its own canonical form: + load(emit(load(x))) and load(x) would differ on that bit, and a merge - + where the line is no longer first - would place a blank the author never + wrote. Clear it here, once, wherever output starts. */ + { + ShclVecSize kids = NODE(d, ROOT).children; + if (kids.len) { + ShclNode *n = &NODE(d, kids.data[0]); + if (n->trivia && n->trivia->leading.len) n->trivia->leading.data[0].blank_before = 0; + else n->blank_before = 0; + } else if (d->orphans.len) { + d->orphans.data[0].blank_before = 0; + } + } /* The one entry past the cap: what was not listed, and whether any of it was an error, so a consumer scanning the list for errors still finds one and a strict load still fails. */ diff --git a/source/go/shcl.go b/source/go/shcl.go index 3e7fccd..68313d1 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -1578,6 +1578,10 @@ type parser struct { // quadratic time. pendMarks []pendMark sawBlank bool // a blank line waits to become the next bound node's blankBefore + // Nothing has been read yet, so a blank line here leads the file and the + // emitter would not re-emit it. Dropping it at parse time is what makes + // load(emit(load(x))) equal load(x) on that bit, which a merge relies on. + atStart bool // An open stacked list defers its merge-key remap (rebuilding the key per // element is O(list^2) time); (node, key hash, display hash) at deferral // start, flushed before any map lookup and at end of parse. @@ -1610,6 +1614,7 @@ func newParser() *parser { childMap: []map[uint64]slot{nil}, dispMap: []map[uint64]int{nil}, reentered: map[int]int{}, + atStart: true, } } @@ -2215,10 +2220,11 @@ func (p *parser) parse(text string, strictness Strictness) *Document { indent := leadingWS(line) rest := line[len(indent):] if rest == "" { - p.sawBlank = true + p.sawBlank = !p.atStart i++ continue } + p.atStart = false // Whole-line comment: hold it for the next line that binds a node. // It consumes a pending blank into its own flag, so a blank between // comment-only regions survives the round-trip. @@ -2437,6 +2443,21 @@ func (p *parser) parse(text string, strictness Strictness) *Document { orphans = append(orphans, lead{text: pn.text, blankBefore: pn.blankBefore}) } p.pending = p.pending[:0] + // The emitter drops a blank before the first thing it prints, so a document + // that kept one there would not survive its own canonical form: + // load(emit(load(x))) and load(x) would differ on that bit, and a merge - + // where the line is no longer first - would place a blank the author never + // wrote. Clear it here, once, wherever output starts. + if kids := p.arena[root].children; len(kids) > 0 { + n := &p.arena[kids[0]] + if n.trivia != nil && len(n.trivia.leading) > 0 { + n.trivia.leading[0].blankBefore = false + } else { + n.blankBefore = false + } + } else if len(orphans) > 0 { + orphans[0].blankBefore = false + } // The one entry past the cap: what was not listed, and whether any of it // was an error, so a consumer scanning the list for errors still finds // one and a Strict load still fails. diff --git a/source/python/shcl.py b/source/python/shcl.py index 0d4c23c..50a6e13 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -1194,6 +1194,10 @@ def __init__(self): # line and a plain text file parses in quadratic time. self.pend_marks = [] self.saw_blank = False # a blank line waits to become the next bound node's blank_before + # Nothing has been read yet, so a blank line here leads the file and the + # emitter would not re-emit it. Dropping it at parse time is what makes + # load(emit(load(x))) equal load(x) on that bit, which a merge relies on. + self.at_start = True # An open stacked list defers its merge-key remap (rebuilding the key per # element is O(list^2) time); (node, map key, display key) at deferral # start, flushed before any map lookup and at end of parse. @@ -1679,9 +1683,10 @@ def parse(self, text, strictness): rest = line.lstrip(" \t") indent = line[:len(line) - len(rest)] if not rest: - self.saw_blank = True + self.saw_blank = not self.at_start i += 1 continue + self.at_start = False # Whole-line comment: hold it for the next line that binds a node. # It consumes a pending blank into its own flag, so a blank between # comment-only regions survives the round-trip. @@ -1877,6 +1882,20 @@ def parse(self, text, strictness): self._hang_deeper_pending("") orphans = [_Lead(p.text, p.blank_before) for p in self.pending] self.pending = [] + # The emitter drops a blank before the first thing it prints, so a + # document that kept one there would not survive its own canonical form: + # load(emit(load(x))) and load(x) would differ on that bit, and a merge - + # where the line is no longer first - would place a blank the author + # never wrote. Clear it here, once, wherever output starts. + kids = self.arena[ROOT].children + if kids: + n = self.arena[kids[0]] + if n.trivia is not None and n.trivia.leading: + n.trivia.leading[0].blank_before = False + else: + n.blank_before = False + elif orphans: + orphans[0].blank_before = False # The one entry past the cap: what was not listed, and whether any of # it was an error, so a consumer scanning the list for errors still # finds one and a Strict load still fails. diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 3d676c1..e269854 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -2346,7 +2346,7 @@ impl Parser { self.emit_repeated_leaf_hints(); // Indented tail comments keep their block; only top-level ones orphan. self.hang_deeper_pending(""); - let orphans = self + let mut orphans: Vec = self .pending .drain(..) .map(|p| Lead { @@ -2354,6 +2354,20 @@ impl Parser { blank_before: p.blank_before, }) .collect(); + // The emitter drops a blank before the first thing it prints, so a + // document that kept one there would not survive its own canonical + // form: `load(emit(load(x)))` and `load(x)` would differ on that bit, + // and a merge - where the line is no longer first - would place a blank + // the author never wrote. Clear it here, once, wherever output starts. + if let Some(&first) = self.arena[ROOT].children.first() { + let n = &mut self.arena[first]; + match n.trivia.as_mut().and_then(|t| t.leading.first_mut()) { + Some(c) => c.blank_before = false, + None => n.blank_before = false, + } + } else if let Some(c) = orphans.first_mut() { + c.blank_before = false; + } // The one entry past the cap: what was not listed, and whether any // of it was an error, so a consumer scanning the list for errors // still finds one and a Strict load still fails. diff --git a/source/rust/tests/fuzz_smoke.rs b/source/rust/tests/fuzz_smoke.rs index 01cc0db..b2ea956 100644 --- a/source/rust/tests/fuzz_smoke.rs +++ b/source/rust/tests/fuzz_smoke.rs @@ -278,6 +278,21 @@ fn merge_never_panics_and_stays_fixpoint() { i, b ); + // A layer and its canonical form must merge the same: a load that + // keeps a bit its own emitter cannot re-emit makes the fold depend on + // whether the caller formatted the layer first. + let mut from_text = Document::parse(&a); + from_text.merge(&Document::parse(&b)); + let mut from_canon = Document::parse(&a); + from_canon.merge(&Document::parse(&Document::parse(&b).to_canonical())); + assert_eq!( + from_canon.to_canonical(), + from_text.to_canonical(), + "merging a layer differs from merging its canonical form at iteration {} for:\nA:\n{}\nB:\n{}", + i, + a, + b + ); } } From 4685b4ab04ebb35647ae9d333bc517ef43c566d7 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:22:11 -0700 Subject: [PATCH 14/27] float rows the same under every awk Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/crosscheck.bash | 23 +++++++++++++++++++---- cicd/utility/shell-regress.bash | 23 +++++++++++++++++++++++ project/backlog.md | 6 ++++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/cicd/utility/crosscheck.bash b/cicd/utility/crosscheck.bash index 45e30fc..15cdbaf 100755 --- a/cicd/utility/crosscheck.bash +++ b/cicd/utility/crosscheck.bash @@ -394,11 +394,26 @@ fCompareWrite "fmt --write still refuses a missing file" fFixAbsent fmt --write # random doubles built as exact m * 2^e so the text reads back to the double # it names, through a float write in each binding. awk 'BEGIN{ - for (e = -1074; e <= 1023; e++) printf "float\tp%d\t%.17g\n", e + 1074, 2 ^ e; - srand(20260902); + ## 2^e by exact halving and doubling from 1. `2 ^ e` is not a portable way + ## to reach a subnormal: gawk computes a negative power as 1/(2^1074), + ## which is 1/inf, so every subnormal row came out 0 and tested nothing. + v = 1; + for (e = 0; e <= 1023; e++) { pw[e] = v; v = v * 2 } + v = 1; + for (e = -1; e >= -1074; e--) { v = v / 2; pw[e] = v } + for (e = -1074; e <= 1023; e++) printf "float\tp%d\t%.17g\n", e + 1074, pw[e]; + ## A fixed integer generator rather than srand()/rand(), whose sequence + ## differs between awks - so the "fixed" random set was a different set on + ## every runner. Every product here stays under 2^53, so it is exact. + s = 20260902; for (i = 0; i < 3000; i++) { - m = int(rand() * 9007199254740992); e = int(rand() * 1900) - 1000; - printf "float\tr%d\t%.17g\n", i, m * 2 ^ e; + s = (16807 * s) % 2147483647; a = s % 131072; + s = (16807 * s) % 2147483647; b = s % 131072; + s = (16807 * s) % 2147483647; c = s % 131072; + s = (16807 * s) % 2147483647; + m = (a * 131072 + b) * 131072 + c; + e = (s % 1900) - 1000; + printf "float\tr%d\t%.17g\n", i, m * pw[e]; } }' > "${tmpDir}/floats.ops" fCompareStdin "float spelling" "${tmpDir}/floats.ops" set - diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index 9e04416..58973cc 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -473,6 +473,29 @@ while IFS= read -r f; do fi done < <(find "${repoDir}" -name '*.bash' -not -path '*/target/*' -not -path '*/.git/*' | sort) +## 20260902 item 17: the crosscheck's float-spelling dimension built its powers +## of two as `2 ^ e`, which gawk computes as 1/(2^1074) for a subnormal - so 52 +## of its rows were the value zero and tested nothing - and drew its "fixed" +## random set from srand()/rand(), whose sequence differs between awks. The +## shipped generator is lifted out of crosscheck.bash by name so the check +## cannot drift from it. +gen="$(sed -n "/^awk 'BEGIN{/,/^}' > /p" "${repoDir}/cicd/utility/crosscheck.bash" | sed "1s/^awk '//; \$s/}' > .*/}/")" +[[ -n "${gen}" ]] || fBad "could not lift the float generator out of crosscheck.bash" +printf '%s +' "${gen}" > "${tmpDir}/floats.awk" +first="" +for a in gawk mawk "busybox awk" awk; do + read -r -a acmd <<<"${a}" + command -v "${acmd[0]}" > /dev/null 2>&1 || continue + "${acmd[@]}" -f "${tmpDir}/floats.awk" > "${tmpDir}/floats.out" 2>/dev/null || { fBad "float generator failed under ${a}"; continue; } + zeros="$(grep -cE $'^float p[0-9]+ -?0$' "${tmpDir}/floats.out" || true)" + ((zeros == 0)) || fBad "float generator wrote ${zeros} zero-valued power row(s) under ${a}" + sum="$(sha256sum < "${tmpDir}/floats.out")" + if [[ -z "${first}" ]]; then first="${sum}" + elif [[ "${sum}" != "${first}" ]]; then fBad "float generator differs under ${a}"; fi +done +[[ -n "${first}" ]] || fBad "no awk found to run the float generator" + if ((nBad)); then echo "shell-regress: ${nBad} check(s) failed" >&2 exit 1 diff --git a/project/backlog.md b/project/backlog.md index b33b87e..dc8a54f 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -183,10 +183,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171500 - Closed: 20260903-001500 - - 🔘 Item 17: the crosscheck's float-spelling dimension writes `0` for every subnormal power of two under gawk, so its "every power of two" claim is false on this box. + - ✅ Item 17: the crosscheck's float-spelling dimension writes `0` for every subnormal power of two under gawk, so its "every power of two" claim is false on this box. - Reproduced: `gawk 'BEGIN{printf "%.17g", 2^-1074}'` prints `0` (it computes a negative power as `1/(2^1074)`, which is `1/inf`); mawk prints the subnormal. The generated ops file starts with 52 `float p 0` rows. The hosted runner's default awk decides which rows it exercises there, and `srand(20260902); rand()` differs per awk as well, so the "fixed" random set is not fixed either. - - Note: build the powers by exact halving from 1, which every awk does exactly down to `2^-1074`, and generate the random set from a fixed integer LCG. + - Fixed: the powers are built by exact halving and doubling from 1, and the random set comes from a fixed integer generator whose every product stays under 2^53. gawk, mawk and busybox awk now write byte-identical ops files, subnormals included. + - Pinned by `shell-regress.bash`, which lifts the generator out of `crosscheck.bash` by name, runs it under every awk on the box, and requires no zero-valued power row and identical output. The old generator fails all three ways. - Opened: 20260902-171600 + - Closed: 20260903-003000 - 🔘 Item 18: `sanitize-c.bash` never runs the `children` and `paths` commands, against its claim to replay every `reads.tsv` row. - Reproduced: its row replay has no arm for the two, so a `children` row becomes `get --children`, which the C CLI refuses at exit 1 (not 77), and the run counts as clean. Six corpus cases carry such rows. The two enumeration paths in `main.c`, including the quoted-name spelling, run under no sanitizer. From 2a9e04f2e6edebc9c37c8a8c5cb8ea649948cefb Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:29:11 -0700 Subject: [PATCH 15/27] replay every corpus row in the C sanitizer Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/crosscheck.bash | 26 ++++++++++++++++++++++---- cicd/utility/sanitize-c.bash | 32 +++++++++++++++++++++++++++----- cicd/utility/shell-regress.bash | 16 ++++++++++++++++ project/backlog.md | 7 +++++-- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/cicd/utility/crosscheck.bash b/cicd/utility/crosscheck.bash index 15cdbaf..c7034b8 100755 --- a/cicd/utility/crosscheck.bash +++ b/cicd/utility/crosscheck.bash @@ -223,6 +223,17 @@ fFixLost(){ printf 'a: 1\n\xef\xbb\xbfb: 2\n' >"$1/c.shcl"; chmod 600 "$1/c. ## optional level. expected/status are the corpus contract (each binding's own ## conformance runner asserts those); here only binding-vs-binding agreement ## matters, so the row is just a recipe for an invocation. +## Split a row on tabs, keeping empty fields. `IFS=$'\t' read` cannot: tab is +## IFS whitespace whatever IFS is set to, so a leading or doubled tab +## disappears and every column after it shifts - which silently turned the +## top-level `children` row into a type nothing had an arm for. +fSplitTabs(){ + local rest="$1" + cols=() + while [[ "$rest" == *$'\t'* ]]; do cols+=("${rest%%$'\t'*}"); rest="${rest#*$'\t'}"; done + cols+=("$rest") +} + fReadRow(){ local input="$1" query="$2" type="$3" level="$4" local -a strictArg=() @@ -235,14 +246,19 @@ fReadRow(){ children) fCompare "children ${query}" children "${strictArg[@]}" "$input" "$query" ;; paths) fCompare "paths" paths "${strictArg[@]}" "$input" ;; lost) : ;; ## no CLI surface; the in-place write below is what it reaches - *'[]') fCompare "get ${query} ${type}" get "--${type%[]}" --array "${strictArg[@]}" "$input" "$query" + int'[]'|float'[]'|bool'[]'|datetime'[]'|string'[]') + fCompare "get ${query} ${type}" get "--${type%[]}" --array "${strictArg[@]}" "$input" "$query" fCompare "get ${query} ${type} slots" get "--${type%[]}" --array --slots "${strictArg[@]}" "$input" "$query" ;; - *) fCompare "get ${query} ${type}" get "--${type}" "${strictArg[@]}" "$input" "$query" + int|float|bool|datetime|string|raw|rawinfo) + fCompare "get ${query} ${type}" get "--${type}" "${strictArg[@]}" "$input" "$query" # on-bad=error (exit-code differential; message goes to dropped stderr) # and a default substitution (stdout differential) - the accessor # policy surface, where hand-written ports diverge most easily. fCompare "get ${query} ${type} on-bad=error" get "--${type}" --on-bad=error "${strictArg[@]}" "$input" "$query" fCompare "get ${query} ${type} default" get "--${type}" "--default=" "${strictArg[@]}" "$input" "$query" ;; + ## A row type with no arm used to fall through to `get --`, which + ## every binding refuses the same way - so the row compared nothing. + *) echo "crosscheck: unknown reads.tsv type: ${type}" >&2; exit 2 ;; esac } @@ -311,8 +327,10 @@ for caseDir in "$corpus"/*/; do fi tsv="${caseDir}reads.tsv" if [[ -f "$tsv" ]]; then - while IFS=$'\t' read -r query type _expected _status level _rest || [[ -n "$query" ]]; do - [[ -z "$query" || "$query" == "query" ]] && continue + while IFS= read -r row || [[ -n "$row" ]]; do + [[ -z "$row" || "$row" == query$'\t'* ]] && continue + fSplitTabs "$row" + query="${cols[0]}"; type="${cols[1]}"; level="${cols[4]:-}" fReadRow "$input" "$query" "$type" "${level:-}" done < "$tsv" fi diff --git a/cicd/utility/sanitize-c.bash b/cicd/utility/sanitize-c.bash index e3c77a7..3f9a107 100755 --- a/cicd/utility/sanitize-c.bash +++ b/cicd/utility/sanitize-c.bash @@ -9,7 +9,7 @@ ## fails here. The test programs run as they are; the CLI is driven the way ## crosscheck.bash drives it, over every dimension the corpus has: fmt, ## check, check --schema, the write-ops and bad-ops scripts, the layered -## load, init --schema, and every reads.tsv row as get/count/instances. +## load, init --schema, and every reads.tsv row as the CLI call it names. ## Syntax: ## sanitize-c.bash [CORPUS_DIR] ## CORPUS_DIR conformance corpus root (default project/conformance) @@ -78,6 +78,17 @@ fCli(){ ## One reads.tsv row as the CLI calls crosscheck.bash makes of it. Columns: ## query, type, expected, status, optional level; only the invocation matters ## here, so expected and status are not read. +## Split a row on tabs, keeping empty fields. `IFS=$'\t' read` cannot: tab is +## IFS whitespace whatever IFS is set to, so a leading or doubled tab +## disappears and every column after it shifts - which silently turned the +## top-level `children` row into a type nothing had an arm for. +fSplitTabs(){ + local rest="$1" + cols=() + while [[ "$rest" == *$'\t'* ]]; do cols+=("${rest%%$'\t'*}"); rest="${rest#*$'\t'}"; done + cols+=("$rest") +} + fReadRow(){ local input="$1" query="$2" type="$3" level="$4" local -a strictArg=() @@ -87,11 +98,20 @@ fReadRow(){ fCli fmt "${strictArg[@]}" "$input" ;; count) fCli count "${strictArg[@]}" "$input" "$query" ;; instances) fCli instances "${strictArg[@]}" "$input" "$query" ;; - *'[]') fCli get "--${type%[]}" --array "${strictArg[@]}" "$input" "$query" + children) fCli children "${strictArg[@]}" "$input" "$query" ;; + paths) fCli paths "${strictArg[@]}" "$input" ;; + lost) : ;; ## no CLI surface, like crosscheck.bash's own arm + int'[]'|float'[]'|bool'[]'|datetime'[]'|string'[]') + fCli get "--${type%[]}" --array "${strictArg[@]}" "$input" "$query" fCli get "--${type%[]}" --array --slots "${strictArg[@]}" "$input" "$query" ;; - *) fCli get "--${type}" "${strictArg[@]}" "$input" "$query" + int|float|bool|datetime|string|raw|rawinfo) + fCli get "--${type}" "${strictArg[@]}" "$input" "$query" fCli get "--${type}" --on-bad=error "${strictArg[@]}" "$input" "$query" fCli get "--${type}" "--default=" "${strictArg[@]}" "$input" "$query" ;; + ## A row type with no arm used to fall through to `get --`, which + ## the CLI refuses at exit 1 - so the row ran nothing and the case still + ## counted as clean. + *) echo "sanitize-c: unknown reads.tsv type: ${type}" >&2; exit 2 ;; esac } for caseDir in "${corpus}"/*/; do @@ -135,8 +155,10 @@ for caseDir in "${corpus}"/*/; do fCli init --no-banner "--schema=${caseDir}init-schema.shcl" fi if [[ -f "${caseDir}reads.tsv" ]]; then - while IFS=$'\t' read -r query type _expected _status level _rest || [[ -n "$query" ]]; do - [[ -z "$query" || "$query" == "query" ]] && continue + while IFS= read -r row || [[ -n "$row" ]]; do + [[ -z "$row" || "$row" == query$'\t'* ]] && continue + fSplitTabs "$row" + query="${cols[0]}"; type="${cols[1]}"; level="${cols[4]:-}" fReadRow "${input}" "$query" "$type" "${level:-}" done < "${caseDir}reads.tsv" fi diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index 58973cc..64a04c3 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -473,6 +473,22 @@ while IFS= read -r f; do fi done < <(find "${repoDir}" -name '*.bash' -not -path '*/target/*' -not -path '*/.git/*' | sort) +## 20260902 item 18: the two corpus replays split a reads.tsv row with +## `IFS=$'\t' read`, which drops a leading or doubled tab because tab is IFS +## whitespace whatever IFS is set to - so the top-level `children` row arrived +## as a type nothing had an arm for and ran nothing. Lifted out of +## crosscheck.bash by name so the check cannot drift from the shipped text. +eval "$(sed -n '/^fSplitTabs()/,/^}/p' "${repoDir}/cicd/utility/crosscheck.bash")" +cols=() +fSplitTabs "$(printf '\tchildren\tdb|web\t-')" +[[ "${#cols[@]}" == 4 && -z "${cols[0]}" && "${cols[1]}" == "children" ]] \ + || fBad "fSplitTabs dropped a leading empty field: ${cols[*]@Q}" +fSplitTabs "$(printf 'nope\tchildren\t\t-')" +[[ "${#cols[@]}" == 4 && "${cols[2]}" == "" && "${cols[3]}" == "-" ]] \ + || fBad "fSplitTabs dropped a middle empty field: ${cols[*]@Q}" +fSplitTabs "one" +[[ "${#cols[@]}" == 1 && "${cols[0]}" == "one" ]] || fBad "fSplitTabs mangled a single field: ${cols[*]@Q}" + ## 20260902 item 17: the crosscheck's float-spelling dimension built its powers ## of two as `2 ^ e`, which gawk computes as 1/(2^1074) for a subnormal - so 52 ## of its rows were the value zero and tested nothing - and drew its "fixed" diff --git a/project/backlog.md b/project/backlog.md index dc8a54f..51f50a6 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -190,10 +190,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171600 - Closed: 20260903-003000 - - 🔘 Item 18: `sanitize-c.bash` never runs the `children` and `paths` commands, against its claim to replay every `reads.tsv` row. + - ✅ Item 18: `sanitize-c.bash` never runs the `children` and `paths` commands, against its claim to replay every `reads.tsv` row. - Reproduced: its row replay has no arm for the two, so a `children` row becomes `get --children`, which the C CLI refuses at exit 1 (not 77), and the run counts as clean. Six corpus cases carry such rows. The two enumeration paths in `main.c`, including the quoted-name spelling, run under no sanitizer. - - Note: copy the two arms from `crosscheck.bash`'s `fReadRow`. + - Fixed: the two arms are there, and a row type with no arm is now an error in both replays instead of a `get --` every binding refuses the same way. + - Note: found while working it, and fixed with it - both replays split a row with `IFS=$'\t' read`, and tab is IFS whitespace whatever IFS is set to, so a leading or doubled tab disappeared and every later column shifted. The top-level `children` row (empty query) arrived as a type nothing had an arm for, and had never been replayed by either gate. Both split by hand now. + - Pinned by `shell-regress.bash`: the splitter is lifted out of `crosscheck.bash` by name and must keep a leading and a middle empty field, and the unknown-type guard fires with the arms removed. - Opened: 20260902-171700 + - Closed: 20260903-005000 - 🔘 Item 19: a must-exist path with a `[#N]` selector, one past the depth cap, or one carrying a literal newline gets V097 where the spec describes the trailing block. - Reproduced in all four. `field: "srv[#1].port"` required (alone or beside a live `field: srv`) exits 6 with `V097 required path missing`; same for a 513-segment required path and for `field: "\"a\nb\""` required. The spec's trailing-block sentence lists all three as "collected into a trailing comment block", and its self-check sentence requires the output to validate, and a must-exist path in the trailing block can never satisfy both. From 060bb42aac888df895f8df9598ceab2f7a0b900e Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:34:06 -0700 Subject: [PATCH 16/27] init names the path it cannot write Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 + cicd/utility/cli-regress.bash | 14 +++++- project/backlog.md | 8 +++- .../082-init-wildcard-fill/expected-init.shcl | 3 ++ .../082-init-wildcard-fill/init-schema.shcl | 4 ++ project/spec.md | 4 +- source/c/shcl.h | 39 +++++++++++++-- source/go/shcl.go | 36 +++++++++++--- source/python/shcl.py | 36 +++++++++++--- source/rust/src/lib.rs | 47 +++++++++++++++---- 10 files changed, 158 insertions(+), 35 deletions(-) diff --git a/changelog.md b/changelog.md index a1bfe48..1392292 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- `init` names the path it cannot generate. A required path with a `[#N]` selector, or one past the nesting cap, went to the trailing comment block and then failed the self-check with "required path missing", which points at the generated config rather than at the schema line nothing can satisfy. It is a `V097` fault naming the path now. A name carrying a newline is generated rather than refused: names have been stored escape-resolved since 2.0 and the name escaper spells one. + - A blank line before the first thing canonical output prints is dropped at load. Canonical output never starts with a blank, so a document that kept the flag did not survive its own canonical form: merging a layer gave a different result from merging its `fmt`, and the fold placed a blank line the author never wrote. Three shapes did it - a file starting with a blank line, a blank after a leading line the load dropped, and a blank on a later instance that merged into the first. - A refused `--set` or a failing ops line no longer swallows the load's diagnostics. The edit was applied before anything was printed, so a `get --set` on a file with a dropped line reported the refusal and said nothing about the damage. The diagnostics belong to the load and now go out before any edit runs. diff --git a/cicd/utility/cli-regress.bash b/cicd/utility/cli-regress.bash index f09b5c0..1362dff 100755 --- a/cicd/utility/cli-regress.bash +++ b/cicd/utility/cli-regress.bash @@ -55,6 +55,11 @@ printf 'field: server.port\n\ttype: int\n\trequired: yes\n\tmin: 1\n\tmax: 10\n\ ## documented shortfall and generates. printf 'field: "*"\n\ttype: int\n\trepeat: 1\n' > "${tmpDir}/star1.shcl" printf 'field: "*"\n\ttype: int\n\trepeat: 2\n' > "${tmpDir}/star2.shcl" +## A must-exist path with nothing to generate from: an index selector needs an +## instance that is not there, and a path past the nesting cap would draw E016 +## on the way back in. Either way the fault names the path rather than reporting +## the generated config as missing it. +printf 'field: "srv[#1].port"\n\trequired: yes\n' > "${tmpDir}/idxreq.shcl" ## A schema that does not build: the report is the build faults alone, not the ## faults plus what an empty document would owe the schema. printf 'field: a\n\ttype: int\n\trequired: yes\nfield: b\n\ttype: nope\n' > "${tmpDir}/nobuild.shcl" @@ -72,7 +77,7 @@ printf 'a: 1\nb: 2\n' > "${tmpDir}/two.shcl" ## argv placeholders: %F% the good file, %B% the two-error file, %D% a directory, ## %P% the deepest legal document, %S% the self-contradicting schema, %S1%/%S2% ## a nameless must-exist path at repeat 1 and 2, %S3% a schema that does not -## build, %X% an +## build, %S4% a required path with an index selector, %X% an ## instance whose discriminator holds an '=', %T% a document with a name that ## needs quoting in a path, %F2% a two-key file for the edit options, %M% a ## path with no file at it. @@ -107,8 +112,12 @@ rows=( ## 20260901 item 5: the self-check waved every V007 through, so a repeat ## lower bound of 1 - a must-exist path - went out as a config that fails ## its own schema at exit 0. - 'init-star-repeat1|init --schema=%S1%|-|6||V097 .*not in 1\.\.1' + 'init-star-repeat1|init --schema=%S1%|-|6||V097 required path cannot be generated' 'init-star-repeat2|init --schema=%S2%|-|0|-|^$' + ## 20260902 item 19: an index selector or a path past the cap got the + ## self-check's "required path missing", which points at the config rather + ## than at the schema line nothing can generate. + 'init-index-required|init --schema=%S4%|-|6||V097 required path cannot be generated: srv\[#1\].port' 'init-build-fault|init --schema=%S3%|-|6||V091 unknown schema type' 'init-build-fault-only|init --schema=%S3%|-|6||!V002' ## 20260830 item 35: -h and --help after FILE were an unknown option, though @@ -187,6 +196,7 @@ for row in "${rows[@]}"; do argv="${argv//%S1%/${tmpDir}/star1.shcl}" argv="${argv//%S2%/${tmpDir}/star2.shcl}" argv="${argv//%S3%/${tmpDir}/nobuild.shcl}" + argv="${argv//%S4%/${tmpDir}/idxreq.shcl}" argv="${argv//%X%/${tmpDir}/sel.shcl}" argv="${argv//%T%/${tmpDir}/tree.shcl}" argv="${argv//%F2%/${tmpDir}/two.shcl}" diff --git a/project/backlog.md b/project/backlog.md index 51f50a6..c182af3 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -198,11 +198,15 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171700 - Closed: 20260903-005000 - - 🔘 Item 19: a must-exist path with a `[#N]` selector, one past the depth cap, or one carrying a literal newline gets V097 where the spec describes the trailing block. + - ✅ Item 19: a must-exist path with a `[#N]` selector, one past the depth cap, or one carrying a literal newline gets V097 where the spec describes the trailing block. - Reproduced in all four. `field: "srv[#1].port"` required (alone or beside a live `field: srv`) exits 6 with `V097 required path missing`; same for a 513-segment required path and for `field: "\"a\nb\""` required. The spec's trailing-block sentence lists all three as "collected into a trailing comment block", and its self-check sentence requires the output to validate, and a must-exist path in the trailing block can never satisfy both. - Cause: `unwritable` sends them to the trailing block and the self-check then reports the missing path. The newline clause is also stale on its own: names resolve escapes since 2026-08-18, `emit_name` spells a newline as `\n`, and `gen_path_text` already goes through it, so the path is writable. - - Decided: needs a call on which sentence wins. Refuse with a message naming the ungenerable path, or drop the must-exist requirement from the trailing block. Render newline-in-name paths through `gen_path_text` either way. + - Decided: refuse, naming the path. The trailing block can never satisfy a must-exist path, so the self-check's "required path missing" points a reader at the generated config when the problem is the schema line. A `repeat` lower bound of 2 or more keeps its documented shortfall and still generates. + - Fixed: a must-exist path that cannot be written is a `V097` fault carrying the path, in all four, before anything is emitted. The newline clause is gone from the unwritable test - only a newline inside a selector is unwritable now - and a path whose text holds one renders through the segment renderer, which escapes it. + - Pinned by a `cli-regress` row (`field: "srv[#1].port"` required, message and exit) and corpus `082`, whose schema now carries a quoted escaped name. All four gave the old message before. + - Note: a literal newline in a schema path is unreachable from a file - a schema value holding one is an unterminated quote - so the newline half of the fix is defensive and has no case of its own. Its old clause never fired either, which is why the escaped spelling already worked. - Opened: 20260902-171800 + - Closed: 20260903-011500 - 🔘 Item 20: V096 fires at exactly 10000 fields with a message that says the schema expands past 10000. - Reproduced in all four: 9999 plain `field:` lines generate, 10000 give `V096 schema expands past 10000 fields; fragments mounted at more than one path multiply`, on a schema with no fragments. diff --git a/project/conformance/082-init-wildcard-fill/expected-init.shcl b/project/conformance/082-init-wildcard-fill/expected-init.shcl index 3029716..8055ef3 100644 --- a/project/conformance/082-init-wildcard-fill/expected-init.shcl +++ b/project/conformance/082-init-wildcard-fill/expected-init.shcl @@ -22,6 +22,9 @@ star: "*" # any, required star["*"].leaf: +# any, required +"a\nb": v + # # This config file format is SHCL. # "Simple Hierarchical Config Language" diff --git a/project/conformance/082-init-wildcard-fill/init-schema.shcl b/project/conformance/082-init-wildcard-fill/init-schema.shcl index bb3c293..f011fa4 100644 --- a/project/conformance/082-init-wildcard-fill/init-schema.shcl +++ b/project/conformance/082-init-wildcard-fill/init-schema.shcl @@ -27,3 +27,7 @@ field: star field: "star[*].leaf" required: yes + +field: "\"a\\nb\"" + required: yes + default: v diff --git a/project/spec.md b/project/spec.md index e7f82a9..37cc3ad 100644 --- a/project/spec.md +++ b/project/spec.md @@ -575,7 +575,7 @@ Diagnostic codes ride the existing structure (line, severity, stable code, prose | `V094` | bad fragment declaration (no name, duplicate, or a non-`field` key inside) | schema file | `V095` | `inherits` names no declared fragment | schema file | `V096` | schema expands to more fields than generation allows | 0 -| `V097` | generated output fails the schema that produced it (a `default` outside its own field's constraints, say) | 0 +| `V097` | generated output fails the schema that produced it (a `default` outside its own field's constraints, say), or a must-exist path nothing can generate | 0 | `V099` | schema failed to load (schema had error diagnostics) | 0 A schema fault (`V090`+) does not silence the rest of the result: the constraints that parsed cleanly still check the document (a broken key drops that key, a broken `field:` drops that field), so a typo in one constraint cannot hide a real violation of another. The unknown-field sweep needs the complete declared vocabulary of *names*, and a key-level fault keeps its entry's path - so the sweep still runs; it turns off only when a fault cost a path spelling outright (an unreadable `field:` path, or a mount naming no declared fragment). Generation (`shcl init`) still requires a fault-free schema - a partial starter config would be worse than an error. `check --schema` folds validation diagnostics into `check`'s existing output: same `line N: severity: CODE` stdout lines, the same line plus its prose on stderr, same summary line and exit-6-on-any-error rule. Both streams carry the code; only stdout is the contract. A `V090`-`V093` line number is a schema-file line (the table above says which); the stderr prose spells those `schema line N` so the two number spaces cannot be confused, while the compared stdout keeps the uniform `line N` shape - the code already names the space. @@ -628,7 +628,7 @@ The output, per schema field in schema order: - The field line itself: fields that **must exist** (`required`, or a `repeat` lower bound of 1 or more) are live (`path: `, or `path:` with an empty value when there is no `default`; a quoted plain-string `default` keeps its quotes, and one containing a newline is written in its quoted escaped spelling); **optional** fields are the same line commented out (`#path: ...`), so the starter is valid and minimal as-is. -- A must-exist field whose path contains a wildcard is generated in dotted form (the wildcard dropped, targeting the instance it materializes) when some other live line materializes the wildcard's parent - otherwise the very instance that line creates would fail the schema. When that parent's line carries a value, a dotted child would name the empty-valued instance instead of the one the line made, so any line under a valued live parent selects it by the value: `srv: web` is followed by `srv[web].port:`, not `srv.port:`. Remaining wildcard paths, and paths that cannot be written at all (a name-wildcard `*` segment has no name to drop to; `[#N]` needs a pre-existing instance and its `#` would start a comment; a path carrying a literal newline has no one-line spelling), are collected into a trailing `# Paths needing an instance name (not generated):` comment block, one `# ` per line. +- A must-exist field whose path contains a wildcard is generated in dotted form (the wildcard dropped, targeting the instance it materializes) when some other live line materializes the wildcard's parent - otherwise the very instance that line creates would fail the schema. When that parent's line carries a value, a dotted child would name the empty-valued instance instead of the one the line made, so any line under a valued live parent selects it by the value: `srv: web` is followed by `srv[web].port:`, not `srv.port:`. Remaining wildcard paths, and paths that cannot be written at all (a name-wildcard `*` segment has no name to drop to; `[#N]` needs a pre-existing instance and its `#` would start a comment; a selector carrying a literal newline has no one-line spelling), are collected into a trailing `# Paths needing an instance name (not generated):` comment block, one `# ` per line. A path that must exist and cannot be written at all is a `V097` fault naming the path instead, since the trailing block can never satisfy it - except a `repeat` lower bound of 2 or more, which is the documented shortfall below. - After the last field, and after the trailing block if there is one, a footer names the format and points at its spec, separated from what precedes it by one blank line: diff --git a/source/c/shcl.h b/source/c/shcl.h index 0f2052d..0811862 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -5838,12 +5838,24 @@ static int g_has_wild(const ShclVCons *c) { return 0; } // `[#N]` needs a pre-existing instance and its `#` would start a comment on a -// binding line; a path with a literal newline cannot be written at all. A path -// deeper than a document may nest cannot be generated either: the line would -// draw E016 on the way back in. +// binding line; a newline inside a selector has no one-line spelling, since the +// value emitter never escapes one. A path deeper than a document may nest +// cannot be generated either: the line would draw E016 on the way back in. A +// newline in a NAME is writable: names are stored escape-resolved and the name +// escaper spells one `\n`. static int g_unwritable(const ShclVCons *c) { if (c->segs.len > SHCL_MAX_DEPTH) return 1; - for (size_t si = 0; si < c->segs.len; si++) if (c->segs.data[si].sel.tag == SEL_INDEX || c->segs.data[si].star) return 1; + for (size_t si = 0; si < c->segs.len; si++) { + const ShclSegment *sg = &c->segs.data[si]; + if (sg->sel.tag == SEL_INDEX || sg->star) return 1; + if (sg->sel.tag == SEL_VALUE && memchr(sg->sel.value.p, '\n', sg->sel.value.n)) return 1; + } + return 0; +} +// A repeat lower bound of 2 or more is the one documented shortfall - the line +// is emitted once and the count reported - so it is not the fault below. +static int g_cannot_satisfy(const ShclVCons *c) { return c->required || (c->has_repeat && c->rep_lo == 1); } +static int g_path_has_nl(const ShclVCons *c) { for (size_t k = 0; k < c->path.n; k++) if (c->path.p[k] == '\n') return 1; return 0; } @@ -6102,6 +6114,20 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { // A filled wildcard emits a valued line of its own, so it belongs here too. if ((!g_has_wild(c) || fill[i]) && !g_unwritable(c) && g_must_exist(c) && c->has_default) { pv.data[pv.len].segs = &c->segs; pv.data[pv.len].value = c->default_text; pv.len++; } } + /* A path that cannot be written at all belongs in the trailing note, but one + that must exist can never be satisfied from there: the self-check would + then report the document as missing a path, which points at the config + rather than at the schema line that cannot be generated. */ + for (size_t i = 0; i < cons.len; i++) { + const ShclVCons *c = &cons.data[i]; + if (!g_cannot_satisfy(c) || !g_unwritable(c) || g_has_wild(c)) continue; + ShclSB m = {0, 0, 0}; + sb_puts(a, &m, "required path cannot be generated: "); + sb_putS(a, &m, g_escape_nl(a, c->path)); + push_diag(schema, 0, SHCL_SEV_ERROR, "V097", s_dup(&schema->arena, sb_S(&m))); + if (ok) *ok = 0; + ShclStr e = s_empty(); r.p = e.p; r.n = e.n; arena_free(&tmp); return r; + } ShclSB out = {0, 0, 0}; ShclVecS wild_path = {0, 0, 0}, wild_type = {0, 0, 0}; /* Dropping a trailing `[*]` can render the same line a concrete sibling @@ -6126,7 +6152,10 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { int under_valued_parent = 0; for (size_t k = 1; k < c->segs.len && !under_valued_parent; k++) under_valued_parent = c->segs.data[k - 1].sel.tag == SEL_NONE && parent_value_for(&pv, &c->segs, k) != NULL; - ShclStr path = (fill[i] || under_valued_parent) ? gen_path_text(a, &c->segs, &pv) : c->path; + // A name carrying a newline has no verbatim spelling on a binding line; + // the segment renderer escapes it, so such a path goes through there + // whether or not it was filled. + ShclStr path = (fill[i] || under_valued_parent || g_path_has_nl(c)) ? gen_path_text(a, &c->segs, &pv) : c->path; uint64_t ph = fnv_str(1469598103934665603ull, path); int dup = 0; for (size_t k = 0; k < emitted.len && !dup; k++) dup = emitted_hash[k] == ph && s_eq(emitted.data[k], path); diff --git a/source/go/shcl.go b/source/go/shcl.go index 68313d1..1570ab8 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -6069,11 +6069,13 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { } return false } - // `[#N]` needs a pre-existing instance and its `#` would start a comment - // on a binding line; a path with a literal newline cannot be written at - // all. Both go to the trailing note instead of emitting a broken line. - // A path deeper than a document may nest cannot be generated either: the - // line would draw E016 on the way back in. + // `[#N]` needs a pre-existing instance and its `#` would start a comment on + // a binding line; a newline inside a selector has no one-line spelling, + // since the value emitter never escapes one. Both go to the trailing note + // instead of emitting a broken line. A path deeper than a document may nest + // cannot be generated either: the line would draw E016 on the way back in. + // A newline in a NAME is writable: names are stored escape-resolved and the + // name escaper spells one `\n`. unwritable := func(c *constraint) bool { if len(c.segs) > MaxDepth { return true @@ -6082,8 +6084,11 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { if (s.sel != nil && s.sel.kind == selByIndex) || s.star { return true } + if s.sel != nil && s.sel.kind == selByValue && strings.Contains(s.sel.value, "\n") { + return true + } } - return strings.Contains(c.path, "\n") + return false } // Live concrete paths materialize instances; decide which must-exist // wildcards get filled (their first-wildcard parent chain is a prefix of @@ -6164,6 +6169,20 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { parentValues[namesKey(namesOf(c.segs))] = *c.defaultText } } + // A path that cannot be written at all belongs in the trailing note, but one + // that must exist can never be satisfied from there: the self-check would + // then report the document as missing a path, which points at the config + // rather than at the schema line that cannot be generated. A repeat lower + // bound of 2 or more is the one documented shortfall - the line is emitted + // once and the count reported - so it is not this fault. + for i := range cons { + c := &cons[i] + cannotSatisfy := c.required || (c.repeat != nil && c.repeat[0] == 1) + if cannotSatisfy && unwritable(c) && !hasWild(c) { + msg := "required path cannot be generated: " + strings.ReplaceAll(c.path, "\n", "\\n") + return "", []Diagnostic{{Line: 0, Severity: SeverityError, Message: msg, Code: "V097"}} + } + } var b strings.Builder var wild [][2]string // Dropping a trailing `[*]` can render the same line a concrete sibling @@ -6192,8 +6211,11 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { break } } + // A name carrying a newline has no verbatim spelling on a binding line; + // the segment renderer escapes it, so such a path goes through there + // whether or not it was filled. path := c.path - if fill[i] || underValuedParent { + if fill[i] || underValuedParent || strings.Contains(c.path, "\n") { path = genPathText(c.segs, parentValues) } if emitted[path] { diff --git a/source/python/shcl.py b/source/python/shcl.py index 50a6e13..e945ad6 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -4927,13 +4927,20 @@ def must_exist(c): def has_wild(c): return any(s.selector is not None and s.selector[0] == "wild" for s in c.segs) - # `[#N]` needs a pre-existing instance and its `#` would start a comment - # on a binding line; a path with a literal newline cannot be written at - # all. Both go to the trailing note instead of emitting a broken line. - # A path deeper than a document may nest cannot be generated either: the - # line would draw E016 on the way back in. + # `[#N]` needs a pre-existing instance and its `#` would start a comment on a + # binding line; a newline inside a selector has no one-line spelling, since + # the value emitter never escapes one. Both go to the trailing note instead + # of emitting a broken line. A path deeper than a document may nest cannot be + # generated either: the line would draw E016 on the way back in. A newline in + # a NAME is writable: names are stored escape-resolved and the name escaper + # spells one `\n`. def unwritable(c): - return len(c.segs) > MAX_DEPTH or any((s.selector is not None and s.selector[0] == "idx") or s.star for s in c.segs) or "\n" in c.path + return len(c.segs) > MAX_DEPTH or any( + (s.selector is not None and s.selector[0] == "idx") + or s.star + or (s.selector is not None and s.selector[0] == "val" and "\n" in s.selector[1]) + for s in c.segs + ) # Live concrete paths materialize instances; decide which must-exist # wildcards get filled (their first-wildcard parent chain is a prefix of @@ -4969,6 +4976,18 @@ def names_of(segs): for i, c in enumerate(cons) if (not has_wild(c) or fill[i]) and not unwritable(c) and must_exist(c) and c.default_text is not None } + # A path that cannot be written at all belongs in the trailing note, but one + # that must exist can never be satisfied from there: the self-check would + # then report the document as missing a path, which points at the config + # rather than at the schema line that cannot be generated. A repeat lower + # bound of 2 or more is the one documented shortfall - the line is emitted + # once and the count reported - so it is not this fault. + for c in cons: + cannot_satisfy = c.required or (c.repeat is not None and c.repeat[0] == 1) + if cannot_satisfy and unwritable(c) and not has_wild(c): + faults = [] + _vdiag(faults, 0, "V097", "required path cannot be generated: " + c.path.replace("\n", "\\n")) + return "", faults out = [] wild = [] # Dropping a trailing `[*]` can render the same line a concrete sibling @@ -4989,7 +5008,10 @@ def names_of(segs): c.segs[k - 1].selector is None and tuple(names_of(c.segs[:k])) in parent_values for k in range(1, len(c.segs)) ) - path = _gen_path_text(c.segs, parent_values) if fill[i] or under_valued_parent else c.path + # A name carrying a newline has no verbatim spelling on a binding line; + # the segment renderer escapes it, so such a path goes through there + # whether or not it was filled. + path = _gen_path_text(c.segs, parent_values) if fill[i] or under_valued_parent or "\n" in c.path else c.path if path in emitted: continue emitted.add(path) diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index e269854..6732c65 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -5786,17 +5786,19 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result MAX_DEPTH - || c.segs - .iter() - .any(|s| matches!(s.selector, Some(Selector::ByIndex(_))) || s.star) - || c.path.contains('\n') + || c.segs.iter().any(|s| { + matches!(s.selector, Some(Selector::ByIndex(_))) + || s.star || matches!(&s.selector, Some(Selector::ByValue { text, .. }) if text.contains('\n')) + }) }; // Live concrete paths materialize instances; decide which must-exist // wildcards get filled (their first-wildcard parent chain is a prefix of @@ -5850,6 +5852,28 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result = Vec::new(); // Dropping a trailing `[*]` can render the same line a concrete sibling @@ -5870,7 +5894,10 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result Date: Wed, 2 Sep 2026 19:35:47 -0700 Subject: [PATCH 17/27] field ceiling means at most Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/cli-regress.bash | 13 ++++++++++++- project/backlog.md | 5 ++++- source/c/shcl.h | 4 ++-- source/go/shcl.go | 4 ++-- source/python/shcl.py | 4 ++-- source/rust/src/lib.rs | 4 ++-- 6 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cicd/utility/cli-regress.bash b/cicd/utility/cli-regress.bash index 1362dff..8ddcd5e 100755 --- a/cicd/utility/cli-regress.bash +++ b/cicd/utility/cli-regress.bash @@ -55,6 +55,10 @@ printf 'field: server.port\n\ttype: int\n\trequired: yes\n\tmin: 1\n\tmax: 10\n\ ## documented shortfall and generates. printf 'field: "*"\n\ttype: int\n\trepeat: 1\n' > "${tmpDir}/star1.shcl" printf 'field: "*"\n\ttype: int\n\trepeat: 2\n' > "${tmpDir}/star2.shcl" +## The generation field ceiling, either side of it: the cap used to fire AT the +## limit while its message said past it. +awk 'BEGIN{ for (i = 0; i < 10000; i++) printf "field: f%d\n", i }' > "${tmpDir}/cap10000.shcl" +awk 'BEGIN{ for (i = 0; i < 10001; i++) printf "field: f%d\n", i }' > "${tmpDir}/cap10001.shcl" ## A must-exist path with nothing to generate from: an index selector needs an ## instance that is not there, and a path past the nesting cap would draw E016 ## on the way back in. Either way the fault names the path rather than reporting @@ -77,7 +81,8 @@ printf 'a: 1\nb: 2\n' > "${tmpDir}/two.shcl" ## argv placeholders: %F% the good file, %B% the two-error file, %D% a directory, ## %P% the deepest legal document, %S% the self-contradicting schema, %S1%/%S2% ## a nameless must-exist path at repeat 1 and 2, %S3% a schema that does not -## build, %S4% a required path with an index selector, %X% an +## build, %S4% a required path with an index selector, %S5%/%S6% a schema at and +## one past the generation field ceiling, %X% an ## instance whose discriminator holds an '=', %T% a document with a name that ## needs quoting in a path, %F2% a two-key file for the edit options, %M% a ## path with no file at it. @@ -118,6 +123,10 @@ rows=( ## self-check's "required path missing", which points at the config rather ## than at the schema line nothing can generate. 'init-index-required|init --schema=%S4%|-|6||V097 required path cannot be generated: srv\[#1\].port' + ## 20260902 item 20: V096 fired at exactly the ceiling, on a schema with no + ## fragments, saying the schema expands past it. + 'init-cap-at-limit|init --no-banner --schema=%S5%|-|0|-|^$' + 'init-cap-over|init --no-banner --schema=%S6%|-|6||V096 schema expands past 10000 fields' 'init-build-fault|init --schema=%S3%|-|6||V091 unknown schema type' 'init-build-fault-only|init --schema=%S3%|-|6||!V002' ## 20260830 item 35: -h and --help after FILE were an unknown option, though @@ -197,6 +206,8 @@ for row in "${rows[@]}"; do argv="${argv//%S2%/${tmpDir}/star2.shcl}" argv="${argv//%S3%/${tmpDir}/nobuild.shcl}" argv="${argv//%S4%/${tmpDir}/idxreq.shcl}" + argv="${argv//%S5%/${tmpDir}/cap10000.shcl}" + argv="${argv//%S6%/${tmpDir}/cap10001.shcl}" argv="${argv//%X%/${tmpDir}/sel.shcl}" argv="${argv//%T%/${tmpDir}/tree.shcl}" argv="${argv//%F2%/${tmpDir}/two.shcl}" diff --git a/project/backlog.md b/project/backlog.md index c182af3..03e986c 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -208,10 +208,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171800 - Closed: 20260903-011500 - - 🔘 Item 20: V096 fires at exactly 10000 fields with a message that says the schema expands past 10000. + - ✅ Item 20: V096 fires at exactly 10000 fields with a message that says the schema expands past 10000. - Reproduced in all four: 9999 plain `field:` lines generate, 10000 give `V096 schema expands past 10000 fields; fragments mounted at more than one path multiply`, on a schema with no fragments. - Cause: `cons.len() >= GEN_MAX_FIELDS` in `generate` and inside each `expand_mounts`. + - Fixed: the ceiling means at most that many, in all four. The expander stops one past it so `generate` can tell the two apart. + - Pinned by two `cli-regress` rows, a schema of exactly 10000 fields and one of 10001. The first exited 6 before. - Opened: 20260902-171900 + - Closed: 20260903-013000 - 🔘 Item 21: `shcl_generate` keeps the output of a V097-failing call in the schema's arena, and a succeeding call's output can never be released. - Reproduced: 1000 calls on a schema whose default fails its own constraint grow the heap by 21.9 KB per call while returning nothing, and leave 1001 copies of the same diagnostic on the schema; 1000 succeeding calls grow it by one output each, and `shcl_reads_release` cannot reclaim them because the output goes to `schema->arena`, not `schema->reads`. The header's own comment says everything but the returned bytes dies inside the call. diff --git a/source/c/shcl.h b/source/c/shcl.h index 0811862..dd217a1 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -6002,7 +6002,7 @@ static void g_expand_go(ShclArena *a, const ShclVecVCons *list, const ShclVSchem cc.segs = segs; } ShclStr path = cc.path; ShclVecSeg segs = cc.segs; - if (out->len >= GEN_MAX_FIELDS) return; + if (out->len > GEN_MAX_FIELDS) return; ShclVecVCons_push(a, out, cc); if (c->inherits.n) { int cycling = 0; @@ -6054,7 +6054,7 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { ShclVecVCons cons = {0, 0, 0}; ShclVecS cut_path = {0, 0, 0}, cut_frag = {0, 0, 0}; g_expand_mounts(a, &def, &cons, &cut_path, &cut_frag); - if (cons.len >= GEN_MAX_FIELDS) { + if (cons.len > GEN_MAX_FIELDS) { // Generation-only fault: recorded on the schema document (this // signature has no fault list of its own to return). ShclSB m = {0, 0, 0}; diff --git a/source/go/shcl.go b/source/go/shcl.go index 1570ab8..f5147f5 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -6054,7 +6054,7 @@ func Generate(schema *Document, noBanner bool) (string, []Diagnostic) { return "", faults } cons, cuts := expandMounts(&def) - if len(cons) >= genMaxFields { + if len(cons) > genMaxFields { msg := fmt.Sprintf("schema expands past %d fields; fragments mounted at more than one path multiply", genMaxFields) return "", []Diagnostic{{Line: 0, Severity: SeverityError, Message: msg, Code: "V096"}} } @@ -6425,7 +6425,7 @@ func expandMounts(def *schemaDef) ([]constraint, [][2]string) { } path := cc.path segs := cc.segs - if len(out) >= genMaxFields { + if len(out) > genMaxFields { return } out = append(out, cc) diff --git a/source/python/shcl.py b/source/python/shcl.py index e945ad6..a18e957 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -4916,7 +4916,7 @@ def generate(schema: Document, no_banner: bool = False) -> tuple[str, list[Diagn if faults: return "", faults cons, cuts = _expand_mounts(sdef) - if len(cons) >= _GEN_MAX_FIELDS: + if len(cons) > _GEN_MAX_FIELDS: faults = [] _vdiag(faults, 0, "V096", f"schema expands past {_GEN_MAX_FIELDS} fields; fragments mounted at more than one path multiply") return "", faults @@ -5159,7 +5159,7 @@ def _expand_mounts(sdef): cc.segs = list(s) + list(c.segs) path = cc.path segs = cc.segs - if len(out) >= _GEN_MAX_FIELDS: + if len(out) > _GEN_MAX_FIELDS: break out.append(cc) if c.inherits is not None: diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 6732c65..3d3f922 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -5769,7 +5769,7 @@ pub fn generate(schema: &Document, no_banner: bool) -> Result= GEN_MAX_FIELDS { + if cons.len() > GEN_MAX_FIELDS { return Err(vec![Diagnostic { line: 0, severity: Severity::Error, @@ -6090,7 +6090,7 @@ fn expand_mounts(def: &SchemaDef) -> (Vec, Vec<(String, String)>) { } let path = cc.path.clone(); let segs = cc.segs.clone(); - if out.len() >= GEN_MAX_FIELDS { + if out.len() > GEN_MAX_FIELDS { return; } out.push(cc); From fa0c9e943f913dc9508837b49634835140bea2c3 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:47:07 -0700 Subject: [PATCH 18/27] generate keeps nothing when it refuses Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- changelog.md | 2 ++ project/backlog.md | 7 +++++-- source/c/shcl.h | 27 ++++++++++++++++++++++++--- source/c/shcl.hpp | 1 + source/c/tests/mem_bounds.c | 19 +++++++++++++++++++ 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/changelog.md b/changelog.md index 1392292..0ec9285 100755 --- a/changelog.md +++ b/changelog.md @@ -36,6 +36,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Changed +- `shcl_generate` keeps nothing when it refuses, and what it returns can be given back. The output was copied into the schema's own arena before the self-check, so a call that failed kept text it never returned, and a call that succeeded left a copy no `shcl_reads_release` could reclaim - 21.9 KB per call in a loop. The bytes live in the read arena now, and generation faults from an earlier call are dropped rather than stacked up. The C++ veneer's `generate()` releases first, like every other copying wrapper. + - `init` names the path it cannot generate. A required path with a `[#N]` selector, or one past the nesting cap, went to the trailing comment block and then failed the self-check with "required path missing", which points at the generated config rather than at the schema line nothing can satisfy. It is a `V097` fault naming the path now. A name carrying a newline is generated rather than refused: names have been stored escape-resolved since 2.0 and the name escaper spells one. - A blank line before the first thing canonical output prints is dropped at load. Canonical output never starts with a blank, so a document that kept the flag did not survive its own canonical form: merging a layer gave a different result from merging its `fmt`, and the fold placed a blank line the author never wrote. Three shapes did it - a file starting with a blank line, a blank after a leading line the load dropped, and a blank on a later instance that merged into the first. diff --git a/project/backlog.md b/project/backlog.md index 03e986c..26ae03b 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -216,11 +216,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-171900 - Closed: 20260903-013000 - - 🔘 Item 21: `shcl_generate` keeps the output of a V097-failing call in the schema's arena, and a succeeding call's output can never be released. + - ✅ Item 21: `shcl_generate` keeps the output of a V097-failing call in the schema's arena, and a succeeding call's output can never be released. - Reproduced: 1000 calls on a schema whose default fails its own constraint grow the heap by 21.9 KB per call while returning nothing, and leave 1001 copies of the same diagnostic on the schema; 1000 succeeding calls grow it by one output each, and `shcl_reads_release` cannot reclaim them because the output goes to `schema->arena`, not `schema->reads`. The header's own comment says everything but the returned bytes dies inside the call. - Cause: the output is copied into the schema arena before the self-check, and the failure path returns empty without reclaiming it; faults are appended to the schema on every call. - - Note: self-check the private-arena text and copy into the arena only on success; place the output in `reads` and list it under `shcl_reads_release`; replace rather than append earlier generation faults, or document that a schema should not be regenerated from after it has reported. + - Fixed: the self-check runs on the private-arena text and the bytes are copied into the schema's read arena only on success, so a refusal keeps nothing and a caller can reclaim what it got. V096 and V097 come from nowhere else, so any left on the schema are dropped before generating - the list describes this call. The header says both, `shcl_reads_release` lists generation, and the veneer's `generate()` releases first like every other copying wrapper. + - Measured: 200 refused calls grow the schema by 19.5 KB of diagnostic text (it was 49 KB plus 200 stacked diagnostics), and 200 succeeding calls with a release between leave the document exactly as it started (it grew 6.4 KB). + - Pinned by `mem_bounds.c`, which fails both ways on the old header. - Opened: 20260902-172000 + - Closed: 20260903-015000 - 🔘 Item 22: `shcl.hpp` tells the veneer user to recover generation faults by validating an empty document, which cannot reproduce them. - Reproduced: `validate(empty, schema)` after a failed `generate()` gives `V002`, never the `V097` the generator recorded. V096 and V097 are generation-only codes. The C CLI made the same mistake and was fixed in the 20260830b round; the veneer comment kept the old recipe. diff --git a/source/c/shcl.h b/source/c/shcl.h index dd217a1..485e3da 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -239,7 +239,10 @@ int shcl_write_file_atomic(const char *path, const char *data, size_t n); // the schema document's diagnostics. A footer naming the format and pointing at the spec is // written last unless no_banner; the flag is negative so passing 0 writes the // footer. *ok is set to 1 on success, 0 if the schema has faults (V09x) - then -// the returned string is empty. Bytes live in the schema's arena. +// the returned string is empty and nothing was kept. Bytes live in the schema's +// read arena; valid until shcl_free, or until shcl_reads_release. Generation +// faults from an earlier call on the same schema are dropped first, so the list +// describes this call. shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok); // Canonical form (block layout, tabs, insertion order, minimal quoting). The @@ -317,7 +320,7 @@ shcl_read_str_arr shcl_read_string_array(shcl_doc *d, const char *path, size_t // Give back everything the read calls have handed out. Every result from a read // - shcl_read_*, shcl_children, shcl_paths, shcl_instances, shcl_lines, -// shcl_quote_segment, shcl_to_canonical - is invalid after this; the document itself is untouched +// shcl_quote_segment, shcl_to_canonical, shcl_generate - is invalid after this; the document itself is untouched // and stays readable, so the next read works normally. Optional: leave it alone // and results live until shcl_free, which is the documented contract and what a // read-once consumer wants. A process polling the same document in a loop calls @@ -6037,6 +6040,19 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { ShclArena *a = &tmp; ShclVSchemaDef def; memset(&def, 0, sizeof def); ShclVecDiag faults = {0, 0, 0}; + /* V096 and V097 can only come from generation, so any on the schema are + this call's predecessors. Drop them, or a caller generating in a loop + collects one copy per attempt and the diagnostic count stops meaning + anything. */ + { + size_t w = 0; + for (size_t i = 0; i < schema->diags.len; i++) { + const char *c = schema->diags.data[i].code; + if (c && (!strcmp(c, "V096") || !strcmp(c, "V097"))) continue; + schema->diags.data[w++] = schema->diags.data[i]; + } + schema->diags.len = w; + } // Generation lays the whole schema out, so unlike validation it has no // safe partial mode: any fault fails it. v_build_schema(a, schema, &def, &faults); @@ -6200,7 +6216,7 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { if (out.len) sb_putc(a, &out, '\n'); sb_puts(a, &out, GEN_BANNER); } - ShclStr s = s_dup(&schema->arena, sb_S(&out)); r.p = s.p; r.n = s.n; + ShclStr s = sb_S(&out); /* The output promises to validate clean against the schema that produced it, so check that here rather than trusting each branch above. A `default` outside its own field's constraints is the schema's fault, and @@ -6232,6 +6248,11 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { return r; } } + /* Only now does the text leave the private arena, and it goes to the read + arena rather than the document's own: a refused call then costs the + schema nothing, and a caller generating in a loop can give the copies + back with shcl_reads_release. */ + ShclStr kept = s_dup(&schema->reads, s); r.p = kept.p; r.n = kept.n; arena_free(&tmp); return r; } diff --git a/source/c/shcl.hpp b/source/c/shcl.hpp index 095e8d5..e936a23 100644 --- a/source/c/shcl.hpp +++ b/source/c/shcl.hpp @@ -221,6 +221,7 @@ class Document { // written last unless no_banner. Not const: a schema that expands past the // generator's field cap records the fault as a diagnostic on this document. std::pair generate(bool no_banner = false) { + shcl_reads_release(d_.get()); int ok = 0; std::string s = to_str(shcl_generate(d_.get(), no_banner ? 1 : 0, &ok)); return {std::move(s), ok != 0}; diff --git a/source/c/tests/mem_bounds.c b/source/c/tests/mem_bounds.c index d4ba6c0..f69240b 100644 --- a/source/c/tests/mem_bounds.c +++ b/source/c/tests/mem_bounds.c @@ -140,6 +140,25 @@ int main(void) { shcl_free(d); free(blob); + // Generation used to copy its output into the schema's own arena before + // checking it, so a refused call kept the text it never returned and a + // succeeding one could never be released. + const char *badschema = "field: server.port\n\ttype: int\n\trequired: yes\n\tmin: 1\n\tmax: 10\n\tdefault: 99\n"; + d = shcl_parse(badschema, strlen(badschema)); + held = arena_bytes(&d->arena); + int gok = 1; + for (int i = 0; i < 200; i++) { shcl_str t = shcl_generate(d, 1, &gok); if (gok || t.n) fail("refused generation returned something"); } + printf("mem_bounds: refused generation: arena %zu -> %zu, %zu diagnostic(s)\n", held, arena_bytes(&d->arena), shcl_diag_count(d)); + if (arena_bytes(&d->arena) > held + 200 * 256) fail("a refused generation kept its output"); + if (shcl_diag_count(d) != 1) fail("generation faults accumulated across calls"); + shcl_free(d); + d = shcl_parse("field: a\n\trequired: yes\n", 25); + held = arena_bytes(&d->arena); + for (int i = 0; i < 200; i++) { shcl_str t = shcl_generate(d, 1, &gok); if (!gok || !t.n) fail("generation failed"); shcl_reads_release(d); } + printf("mem_bounds: released generation: arena %zu -> %zu\n", held, arena_bytes(&d->arena)); + if (arena_bytes(&d->arena) > held + 4096) fail("a released generation still grew the document"); + shcl_free(d); + // A write lands in a bump arena and the value it replaced stays behind, so // a loop rewriting one field grows the document until shcl_free. Compaction // is the way out: the rebuilt document holds what it now contains and no From e9cd53c380115f4abc02354431d72c728e1d5564 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:48:09 -0700 Subject: [PATCH 19/27] veneer points at the right fault list Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- cicd/utility/check-docs.bash | 8 ++++++++ project/backlog.md | 6 ++++-- source/c/shcl.hpp | 12 +++++++----- source/c/tests/veneer_smoke.cpp | 10 ++++++++++ 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/cicd/utility/check-docs.bash b/cicd/utility/check-docs.bash index 706fe05..487f767 100755 --- a/cicd/utility/check-docs.bash +++ b/cicd/utility/check-docs.bash @@ -155,6 +155,14 @@ while IFS= read -r hit; do fBad "backlog.md: says how it was found rather than what changed: ${hit}" done < <(grep -nE '^[[:space:]]*- Found (by|while) ' "${backlog}" || true) +## V096 and V097 come only from generation, so validating anything against the +## schema cannot reproduce them - the veneer header used to send a reader that +## way for the fault list, which returns the validated document's own V002 and +## V007 instead. The C CLI made the same mistake and was fixed in 20260830b. +while IFS= read -r hit; do + fBad "shcl.hpp: sends a reader to validate() for generation faults: ${hit}" +done < <(grep -nE 'for the fault list, validate\(\)' "${repoDir}/source/c/shcl.hpp" || true) + if ((nBad)); then echo "check-docs: ${nBad} check(s) failed" >&2 exit 1 diff --git a/project/backlog.md b/project/backlog.md index 26ae03b..67902e0 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -225,10 +225,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-172000 - Closed: 20260903-015000 - - 🔘 Item 22: `shcl.hpp` tells the veneer user to recover generation faults by validating an empty document, which cannot reproduce them. + - ✅ Item 22: `shcl.hpp` tells the veneer user to recover generation faults by validating an empty document, which cannot reproduce them. - Reproduced: `validate(empty, schema)` after a failed `generate()` gives `V002`, never the `V097` the generator recorded. V096 and V097 are generation-only codes. The C CLI made the same mistake and was fixed in the 20260830b round; the veneer comment kept the old recipe. - - Note: "read `diagnostics()` on this document after the call". + - Fixed: the comment says to read `diagnostics()` on the schema after the call, and says why validating cannot answer it. + - Pinned by `check-docs.bash`, which refuses the old sentence, and by `veneer_smoke.cpp`, which requires the V097 to be on the schema and absent from validating an empty document against it. - Opened: 20260902-172100 + - Closed: 20260903-020000 - 🔘 Item 23: the datetime whitelist admits spellings the spec does not list. - Reproduced in all four, all Good: `9:30` (one-digit hour with no meridiem), `2026-7-1` and `2026/7/4` (one-digit month and day), `14:30z` and `2026-07-12t14:30` (lower case), `14:30 Z` and `14:30 +05:00` (space before the zone), `2026-07-12 14:30` (two spaces as the separator). The spec says the formats are a closed whitelist and anything else is BadType. Correctly refused in the same run: `24:00`, `14:30:60`, `2026-02-29`, `12-Jul/2026`, `2026-07-12 T 14:30`, `Jul 12,2026`. diff --git a/source/c/shcl.hpp b/source/c/shcl.hpp index e936a23..a4f9c46 100644 --- a/source/c/shcl.hpp +++ b/source/c/shcl.hpp @@ -215,11 +215,13 @@ class Document { // Schema-driven generation (`shcl init`): a commented, typed starter config // from this document read as a schema, and whether it succeeded - false on - // schema faults, with the text then empty; for the fault list, validate() - // an empty document against this schema - it reproduces the same V09x - // diagnostics. A footer naming the format and pointing at the spec is - // written last unless no_banner. Not const: a schema that expands past the - // generator's field cap records the fault as a diagnostic on this document. + // schema faults, with the text then empty; for the fault list, read + // diagnostics() on this document after the call. Validating an empty + // document against the schema does not reproduce them: V096 and V097 are + // generation-only, and what comes back is the empty document's own V002 and + // V007. A footer naming the format and pointing at the spec is written last + // unless no_banner. Not const: the faults are recorded as diagnostics on + // this document, and an earlier call's are dropped first. std::pair generate(bool no_banner = false) { shcl_reads_release(d_.get()); int ok = 0; diff --git a/source/c/tests/veneer_smoke.cpp b/source/c/tests/veneer_smoke.cpp index df1b936..6f79bc9 100644 --- a/source/c/tests/veneer_smoke.cpp +++ b/source/c/tests/veneer_smoke.cpp @@ -171,6 +171,16 @@ int main() { CHECK(starter.find("This config file format is SHCL.", bare.size()) != std::string::npos); auto faulty = shcl::Document::parse("field: port\n\tfrobnicate: 1\n").generate(); CHECK(!faulty.second && faulty.first.empty()); + // The fault list is on the schema itself. The header used to send a reader + // to validate() against an empty document, which cannot reproduce a + // generation-only code - it reports that document's own V002/V007 instead. + auto genfault = shcl::Document::parse("field: p\n\ttype: int\n\trequired: yes\n\tmin: 1\n\tmax: 10\n\tdefault: 99\n"); + CHECK(!genfault.generate(true).second); + bool sawV097 = false; + for (const auto &g : genfault.diagnostics()) if (g.code == "V097") sawV097 = true; + CHECK(sawV097); + shcl::Document empty; + for (const auto &g : empty.validate(genfault)) CHECK(g.code != "V097"); // A default-constructed Document is an empty one, not a null handle. shcl::Document blank; CHECK(blank.to_canonical().empty() && blank.count("x") == 0 && blank.diagnostics().empty()); From b11030aeecd008c063587a20d0598e15912277e1 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:49:16 -0700 Subject: [PATCH 20/27] spell out the datetime tolerances Claude-Session: https://claude.ai/code/session_01HQ4LwWJfXyrgeyaRCX5mNP --- project/backlog.md | 7 +++++-- project/conformance/007-dates/expected.shcl | 8 ++++++++ project/conformance/007-dates/input.shcl | 8 ++++++++ project/conformance/007-dates/reads.tsv | 8 ++++++++ project/spec.md | 2 ++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/project/backlog.md b/project/backlog.md index 67902e0..3c1f600 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -232,11 +232,14 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260902-172100 - Closed: 20260903-020000 - - 🔘 Item 23: the datetime whitelist admits spellings the spec does not list. + - ✅ Item 23: the datetime whitelist admits spellings the spec does not list. - Reproduced in all four, all Good: `9:30` (one-digit hour with no meridiem), `2026-7-1` and `2026/7/4` (one-digit month and day), `14:30z` and `2026-07-12t14:30` (lower case), `14:30 Z` and `14:30 +05:00` (space before the zone), `2026-07-12 14:30` (two spaces as the separator). The spec says the formats are a closed whitelist and anything else is BadType. Correctly refused in the same run: `24:00`, `14:30:60`, `2026-02-29`, `12-Jul/2026`, `2026-07-12 T 14:30`, `Jul 12,2026`. - Cause: `parse_num2` takes one or two digits, the zone and meridiem arms trim before matching, and the combined-separator scan trims around it; same functions in all four. - - Decided: needs a call: tighten (two-digit fields in the 24-hour and year-first forms, upper-case `Z` and `T`, no whitespace inside the value) with a corpus row per rejection, or list the tolerances in the spec. Status difference at exit 0, not data loss, since `fmt` never rewrites value text. + - Decided: list them. Every one is a spelling a person writes by hand and none can be misread, so tightening would turn working configs into `BadType` for nothing - against the forgiving-parser stance the rest of the language takes. The whitelist claim is what was wrong, not the code. + - Fixed: spec text only. The datetime section names the tolerances (a one-digit hour, month or day; a lower-case `z` or `t`; whitespace before the meridiem or zone; a run of spaces as the combined separator) and says what is still refused inside a value. + - Pinned by corpus `007`: six tolerance rows read Good and the two separator shapes read BadType, so the set cannot drift either way. - Opened: 20260902-172200 + - Closed: 20260903-021500 - 🔘 Item 24: at Loose, `$ 1200` reads as 1200 but `$ 3.14` is BadType. - Reproduced in all four: whitespace after the currency symbol is tolerated on the integer path only, because the float path tests the shape on the untrimmed remainder and then falls into the integer path, which trims. diff --git a/project/conformance/007-dates/expected.shcl b/project/conformance/007-dates/expected.shcl index d65d11d..a9acf14 100644 --- a/project/conformance/007-dates/expected.shcl +++ b/project/conformance/007-dates/expected.shcl @@ -20,3 +20,11 @@ bad5: "12:30+€ab" bad6: "Jul +12 2026" bad7: "Jul 0012 2026" bad8: "+12 Jul 2026" +tol1: "9:30" +tol2: 2026-7-1 +tol3: "14:30z" +tol4: "2026-07-12t14:30" +tol5: "14:30 Z" +tol6: "2026-07-12 14:30" +bad9: "2026-07-12 T 14:30" +bad10: "Jul 12,2026" diff --git a/project/conformance/007-dates/input.shcl b/project/conformance/007-dates/input.shcl index 361008c..404d402 100644 --- a/project/conformance/007-dates/input.shcl +++ b/project/conformance/007-dates/input.shcl @@ -20,3 +20,11 @@ bad5: "12:30+€ab" bad6: "Jul +12 2026" bad7: "Jul 0012 2026" bad8: "+12 Jul 2026" +tol1: "9:30" +tol2: 2026-7-1 +tol3: "14:30z" +tol4: "2026-07-12t14:30" +tol5: "14:30 Z" +tol6: "2026-07-12 14:30" +bad9: "2026-07-12 T 14:30" +bad10: "Jul 12,2026" diff --git a/project/conformance/007-dates/reads.tsv b/project/conformance/007-dates/reads.tsv index 231462e..2c19914 100644 --- a/project/conformance/007-dates/reads.tsv +++ b/project/conformance/007-dates/reads.tsv @@ -22,4 +22,12 @@ bad5 datetime - BadType bad6 datetime - BadType bad7 datetime - BadType bad8 datetime - BadType +tol1 datetime 09:30 Good +tol2 datetime 2026-07-01 Good +tol3 datetime 14:30Z Good +tol4 datetime 2026-07-12T14:30 Good +tol5 datetime 14:30Z Good +tol6 datetime 2026-07-12T14:30 Good +bad9 datetime - BadType +bad10 datetime - BadType - lost 0 - diff --git a/project/spec.md b/project/spec.md index 37cc3ad..d854d44 100644 --- a/project/spec.md +++ b/project/spec.md @@ -225,6 +225,8 @@ Month names are the fixed English set only - 3-letter abbreviation or full name, **Combined:** `