From 81ce6312027ebd5ef7ebfba0b4c4262b3535319b Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:28:40 -0700 Subject: [PATCH 01/19] skipped lines hold their level --- .../075-e012-holds-level/expected-diags.txt | 10 ++++ .../075-e012-holds-level/expected.shcl | 7 +++ .../075-e012-holds-level/input.shcl | 16 +++++++ .../075-e012-holds-level/reads.tsv | 16 +++++++ project/conformance/README.md | 2 + project/spec.md | 4 +- source/c/shcl.h | 33 +++++++++++-- source/go/shcl.go | 45 +++++++++++++++-- source/python/shcl.py | 37 +++++++++++--- source/rust/src/lib.rs | 48 ++++++++++++++++--- 10 files changed, 194 insertions(+), 24 deletions(-) create mode 100644 project/conformance/075-e012-holds-level/expected-diags.txt create mode 100644 project/conformance/075-e012-holds-level/expected.shcl create mode 100644 project/conformance/075-e012-holds-level/input.shcl create mode 100644 project/conformance/075-e012-holds-level/reads.tsv diff --git a/project/conformance/075-e012-holds-level/expected-diags.txt b/project/conformance/075-e012-holds-level/expected-diags.txt new file mode 100644 index 0000000..7d486ab --- /dev/null +++ b/project/conformance/075-e012-holds-level/expected-diags.txt @@ -0,0 +1,10 @@ +line 3: Error: E012 +line 6: Error: E012 +line 7: Error: E018 +line 8: Error: E012 +line 9: Error: E018 +line 11: Error: E013 +line 12: Error: E018 +line 14: Error: E013 +line 15: Error: E018 +failed: 9 diagnostic(s), 9 error(s) diff --git a/project/conformance/075-e012-holds-level/expected.shcl b/project/conformance/075-e012-holds-level/expected.shcl new file mode 100644 index 0000000..16e11f3 --- /dev/null +++ b/project/conformance/075-e012-holds-level/expected.shcl @@ -0,0 +1,7 @@ +a: + b: 1 +p: + *f + q: 1 + *i +z: 9 diff --git a/project/conformance/075-e012-holds-level/input.shcl b/project/conformance/075-e012-holds-level/input.shcl new file mode 100644 index 0000000..71130a1 --- /dev/null +++ b/project/conformance/075-e012-holds-level/input.shcl @@ -0,0 +1,16 @@ +a: + b: 1 + ``` +x: 2 + ``` + c: 2 + d: 3 + e: 4 + h: 6 +p: + *f + g: 5 + q: 1 + *i + j: 7 +z: 9 diff --git a/project/conformance/075-e012-holds-level/reads.tsv b/project/conformance/075-e012-holds-level/reads.tsv new file mode 100644 index 0000000..69a0a9f --- /dev/null +++ b/project/conformance/075-e012-holds-level/reads.tsv @@ -0,0 +1,16 @@ +query type expected status +- load ok - +- load fail - strict +a.b int 1 Good +x count 0 Good +a.c count 0 Good +a.d count 0 Good +a.b.d count 0 Good +a.e count 0 Good +a.h count 0 Good +p.q int 1 Good +p.g count 0 Good +p.j count 0 Good +a children b - +p children q - +- paths a|a.b|p|p.q|z - diff --git a/project/conformance/README.md b/project/conformance/README.md index 369b34a..c7150f3 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -129,6 +129,8 @@ Case `060` pins the stacked-list errors: an element with no parent field (`E007` Case `061` pins `E012`: a dedent to a column that matches no open level is skipped, and the next line at a real level binds where it belongs. +Case `075` pins that a skipped line holds its indent level in the other two skip shapes too: a line refused with `E012`, and a `*` line with no space (`E013`). What is written under either is skipped with it (`E018`), a fence line at a bad indent takes its whole body with it, and a second line at the same bad indent is refused the same way rather than binding one level up. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/project/spec.md b/project/spec.md index 6e41022..e900581 100644 --- a/project/spec.md +++ b/project/spec.md @@ -126,7 +126,7 @@ Hierarchy is expressed two interchangeable ways; both produce identical trees. - A line indented deeper than the previous line is its child. Indentation is **relative and stack-based**: any increase opens a level; a decrease must return to the exact column of an ancestor. -- A dedent to a column that matches no open level is a (recoverable) error - the line is diagnosed and skipped, the rest of the file continues. +- A dedent to a column that matches no open level is a (recoverable) error - the line is diagnosed and skipped, the rest of the file continues. Like any skipped line it holds its column: what is written deeper is skipped with it (`E018`), a fence line there takes its whole body with it, and another line at the same bad column is refused the same way rather than binding one level up. - Indentation is tabs *or* spaces, consistent within a subtree. (Detection resets at each top-level ancestor, so distinct top-level trees could technically differ, but authors should just keep it uniform per file.) @@ -429,7 +429,7 @@ Materialization is idempotent and order-stable, so two traversals of the same do | `E010` | bare comma in a stacked `*` list element (one element per line) | `E011` | stacked `*` element for a field that already has a value (element ignored) | `E012` | indentation matches no open level -| `E013` | malformed `*` line (`*` not followed by a space); line skipped +| `E013` | malformed `*` line (`*` not followed by a space); line skipped, and what is written under it with it | `E014` | malformed line skipped (with the reason named in the message) | `E015` | missing colon (repaired as an empty value) | `E016` | nesting deeper than the 512-level cap (line skipped) diff --git a/source/c/shcl.h b/source/c/shcl.h index 7524875..1203714 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -825,6 +825,9 @@ struct shcl_doc { level, so the lines written under it are skipped with it instead of re-parenting one level up. */ #define DEAD ((size_t)-1) +/* Stack entry for a line whose indent matched no open level (E012): never a + level a sibling can bind at, but deeper lines are still under it. */ +#define UNOPENED ((size_t)-2) /* The node vector lives in malloc storage, not the bump arena: the arena cannot reclaim the abandoned copy at each doubling, which held about one @@ -2103,7 +2106,7 @@ static void hang_deeper_pending(ShclParser *P, ShclStr new_indent) { size_t target = (size_t)-1; int at_own_level = 0; for (size_t ii = P->stack.len; ii-- > 0;) { ShclStr ind = P->stack.data[ii].indent; size_t n = P->stack.data[ii].node; - if (n != ROOT && n != DEAD && ind.n >= new_indent.n && p.indent.n >= ind.n && memcmp(p.indent.p, ind.p, ind.n) == 0) { target = n; at_own_level = ind.n == p.indent.n; break; } + if (n != ROOT && n != DEAD && n != UNOPENED && ind.n >= new_indent.n && p.indent.n >= ind.n && memcmp(p.indent.p, ind.p, ind.n) == 0) { target = n; at_own_level = ind.n == p.indent.n; break; } } if (target != (size_t)-1) { ShclLead lead = lead_make(p.text, p.blank_before); @@ -2124,14 +2127,24 @@ static void hang_deeper_pending(ShclParser *P, ShclStr new_indent) { static int resolve_parent(ShclParser *P, ShclStr indent, size_t *out) { size_t top = P->stack.len - 1; ShclStr ti = P->stack.data[top].indent; size_t tn = P->stack.data[top].node; - if (indent.n > ti.n && (ti.n == 0 || memcmp(indent.p, ti.p, ti.n) == 0)) { *out = tn; return 1; } + if (indent.n > ti.n && (ti.n == 0 || memcmp(indent.p, ti.p, ti.n) == 0)) { *out = tn == UNOPENED ? DEAD : tn; return 1; } for (size_t ii = P->stack.len; ii-- > 0;) { - if (s_eq(P->stack.data[ii].indent, indent)) { - *out = (ii == 0) ? ROOT : P->stack.data[ii - 1].node; + if (s_eq(P->stack.data[ii].indent, indent) && P->stack.data[ii].node != UNOPENED) { + size_t parent = (ii == 0) ? ROOT : P->stack.data[ii - 1].node; + *out = parent == UNOPENED ? DEAD : parent; P->stack.len = ii ? ii : 1; return 1; } } + /* Skipped, but it still owns its indent: whatever is written deeper is + skipped with it, and a sibling at the same bad indent is refused the + same way instead of binding one level up. */ + while (P->stack.len > 1) { + ShclStr top_indent = P->stack.data[P->stack.len - 1].indent; + if (indent.n > top_indent.n && (top_indent.n == 0 || memcmp(indent.p, top_indent.p, top_indent.n) == 0)) break; + P->stack.len--; + } + { ShclStackEnt se; se.indent = indent; se.node = UNOPENED; ShclVecStack_push(P->tmp, &P->stack, se); } return 0; } @@ -2532,8 +2545,11 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t ShclFence f = fence_open(rest); if (f.ok) { size_t parent; - if (!resolve_parent(&P, indent, &parent)) { p_err(&P, lineno, "E012", s_lit("indentation matches no open level")); d->lost++; i++; continue; } + int resolved = resolve_parent(&P, indent, &parent); size_t next; ShclValue val = consume_raw(&P, lines.data, lines.len, i + 1, lineno, indent, f, &next); + /* The body goes with its fence: parsed live, it would read as root + bindings and the closing fence would open a second block. */ + if (!resolved) { p_err(&P, lineno, "E012", s_lit("indentation matches no open level")); d->lost++; i = next; continue; } if (parent == DEAD) skip_under_dead(&P, lineno, indent); else { size_t bnode = bind_block(&P, parent, val, lineno); @@ -2552,6 +2568,11 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t if (parent != ROOT) attach_trivia(&P, parent, ecomment); add_star_element(&P, parent, body, lineno); i++; continue; } + { + size_t parent; + if (!resolve_parent(&P, indent, &parent)) { p_err(&P, lineno, "E012", s_lit("indentation matches no open level")); d->lost++; i++; continue; } + if (parent == DEAD) { skip_under_dead(&P, lineno, indent); i++; continue; } + } p_err(&P, lineno, "E013", s_lit("malformed line: '*' must be followed by a space")); /* Content-malformed at any position, so it is safe to retain verbatim as trivia: re-emitted, it re-diagnoses identically and @@ -2562,6 +2583,7 @@ static void parse_body(shcl_doc *d, ShclParseOwn *own, const char *text, size_t { ShclPend pd; pd.text = trim_end(rest); pd.indent = indent; pd.blank_before = had_blank; pd.ceiling = indent.n; ShclVecPend_push(P.tmp, &P.pending, pd); + ShclStackEnt se; se.indent = indent; se.node = DEAD; ShclVecStack_push(P.tmp, &P.stack, se); } i++; continue; } @@ -5986,6 +6008,7 @@ shcl_str shcl_generate(shcl_doc *schema, int no_banner, int *ok) { #undef NODE #undef NIL #undef DEAD +#undef UNOPENED #undef GEN_MAX_FIELDS #undef GEN_BANNER diff --git a/source/go/shcl.go b/source/go/shcl.go index 107a462..b6e37be 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -638,6 +638,11 @@ const root = 0 // of re-parenting one level up. const dead = -1 +// unopened is the stack entry for a line whose indent matched no open level +// (E012): never a level a sibling can bind at, but deeper lines are still +// under it. +const unopened = -2 + // foldNodeInto merges a later instance into an earlier one under the in-file // merge rule: children and trivia move over, first trailing wins (a second // demotes to a leading line), first spelling stays. The caller drops the loser @@ -1779,7 +1784,7 @@ func (p *parser) hangDeeperPending(newIndent string) { atOwnLevel := false for j := len(p.stack) - 1; j >= 0; j-- { ent := p.stack[j] - if ent.node != root && ent.node != dead && len(ent.indent) >= len(newIndent) && + if ent.node != root && ent.node != dead && ent.node != unopened && len(ent.indent) >= len(newIndent) && strings.HasPrefix(pn.indent, ent.indent) { target = ent.node atOwnLevel = len(ent.indent) == len(pn.indent) @@ -1813,10 +1818,13 @@ func (p *parser) hangDeeperPending(newIndent string) { func (p *parser) resolveParent(indent string) (int, bool) { top := p.stack[len(p.stack)-1] if len(indent) > len(top.indent) && strings.HasPrefix(indent, top.indent) { + if top.node == unopened { + return dead, true + } return top.node, true } for i := len(p.stack) - 1; i >= 0; i-- { - if p.stack[i].indent == indent { + if p.stack[i].indent == indent && p.stack[i].node != unopened { // Sibling of stack[i]: its parent is the entry below it. Keep the // sentinel; a top-level line resolves to root. parent := root @@ -1828,9 +1836,23 @@ func (p *parser) resolveParent(indent string) (int, bool) { keep = 1 } p.stack = p.stack[:keep] + if parent == unopened { + return dead, true + } return parent, true } } + // Skipped, but it still owns its indent: whatever is written deeper is + // skipped with it, and a sibling at the same bad indent is refused the + // same way instead of binding one level up. + for len(p.stack) > 1 { + topIndent := p.stack[len(p.stack)-1].indent + if len(indent) > len(topIndent) && strings.HasPrefix(indent, topIndent) { + break + } + p.stack = p.stack[:len(p.stack)-1] + } + p.stack = append(p.stack, stackEnt{indent: indent, node: unopened}) return 0, false } @@ -2210,13 +2232,15 @@ func (p *parser) parse(text string, strictness Strictness) *Document { // Child-indent fence: a value line for its parent field. if ch, length, info, ok := fenceOpen(rest); ok { parent, okp := p.resolveParent(indent) + v, next := p.consumeRaw(lines, i+1, lineno, indent, ch, length, info) if !okp { + // The body goes with its fence: parsed live, it would read as + // root bindings and the closing fence would open a second block. p.err(lineno, "E012", "indentation matches no open level") p.lost++ - i++ + i = next continue } - v, next := p.consumeRaw(lines, i+1, lineno, indent, ch, length, info) if parent == dead { p.skipUnderDead(lineno, indent) } else if node := p.bindBlock(parent, v, lineno); node >= 0 { @@ -2250,6 +2274,18 @@ func (p *parser) parse(text string, strictness Strictness) *Document { i++ continue } + parent, okp := p.resolveParent(indent) + if !okp { + p.err(lineno, "E012", "indentation matches no open level") + p.lost++ + i++ + continue + } + if parent == dead { + p.skipUnderDead(lineno, indent) + i++ + continue + } p.err(lineno, "E013", "malformed line: '*' must be followed by a space") // Content-malformed at any position, so it is safe to retain // verbatim as trivia: re-emitted, it re-diagnoses identically @@ -2258,6 +2294,7 @@ func (p *parser) parse(text string, strictness Strictness) *Document { // sibling site below carries cannot apply here: this line starts // with the '*' that brought us in. p.pending = append(p.pending, pend{text: trimEndWS(rest), indent: indent, blankBefore: hadBlank, ceiling: len(indent)}) + p.stack = append(p.stack, stackEnt{indent: indent, node: dead}) i++ continue } diff --git a/source/python/shcl.py b/source/python/shcl.py index 9fc6d60..88263b2 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -592,6 +592,9 @@ def _triv(self): # re-parenting one level up. sys.maxsize so an arena index through it fails # loudly rather than reading a real node. DEAD = sys.maxsize +# Stack entry for a line whose indent matched no open level (E012): never a +# level a sibling can bind at, but deeper lines are still under it. +UNOPENED = sys.maxsize - 1 # Ends a name-index chain (see _NameIndex). NIL = sys.maxsize @@ -1344,7 +1347,7 @@ def _hang_deeper_pending(self, new_indent): target = None at_own_level = False for ind, node in reversed(self.stack): - if node != ROOT and node != DEAD and len(ind) >= len(new_indent) and p.indent.startswith(ind): + if node != ROOT and node != DEAD and node != UNOPENED and len(ind) >= len(new_indent) and p.indent.startswith(ind): target = node at_own_level = len(ind) == len(p.indent) break @@ -1368,14 +1371,23 @@ def _resolve_parent(self, indent): an open level exactly (dedent), else it is a recoverable error.""" top_indent, top_node = self.stack[-1] if len(indent) > len(top_indent) and indent.startswith(top_indent): - return top_node + return DEAD if top_node == UNOPENED else top_node for i in range(len(self.stack) - 1, -1, -1): - if self.stack[i][0] == indent: + if self.stack[i][0] == indent and self.stack[i][1] != UNOPENED: # Sibling of stack[i]: its parent is the entry below it. parent = ROOT if i == 0 else self.stack[i - 1][1] # Keep the sentinel; a top-level line resolves to ROOT. self.stack = self.stack[:max(i, 1)] - return parent + return DEAD if parent == UNOPENED else parent + # Skipped, but it still owns its indent: whatever is written deeper is + # skipped with it, and a sibling at the same bad indent is refused the + # same way instead of binding one level up. + while len(self.stack) > 1: + top = self.stack[-1][0] + if len(indent) > len(top) and indent.startswith(top): + break + self.stack.pop() + self.stack.append((indent, UNOPENED)) return None def _skip_under_dead(self, line, indent): @@ -1684,12 +1696,14 @@ def parse(self, text, strictness): fence = _fence_open(rest) if fence is not None: parent = self._resolve_parent(indent) + value, nxt = self._consume_raw(lines, i + 1, lineno, indent, fence) if parent is None: + # The body goes with its fence: parsed live, it would read as + # root bindings and the closing fence would open a second block. self._err(lineno, "E012", "indentation matches no open level") self.lost += 1 - i += 1 + i = nxt continue - value, nxt = self._consume_raw(lines, i + 1, lineno, indent, fence) if parent == DEAD: self._skip_under_dead(lineno, indent) else: @@ -1719,6 +1733,16 @@ def parse(self, text, strictness): self._add_star_element(parent, body, lineno) i += 1 continue + parent = self._resolve_parent(indent) + if parent is None: + self._err(lineno, "E012", "indentation matches no open level") + self.lost += 1 + i += 1 + continue + if parent == DEAD: + self._skip_under_dead(lineno, indent) + i += 1 + continue self._err(lineno, "E013", "malformed line: '*' must be followed by a space") # Content-malformed at any position, so it is safe to retain # verbatim as trivia: re-emitted, it re-diagnoses identically @@ -1727,6 +1751,7 @@ def parse(self, text, strictness): # sibling site below carries cannot apply here: this line # starts with the '*' that brought us in. self.pending.append(_Pend(_trim_end(rest), indent, had_blank)) + self.stack.append((indent, DEAD)) i += 1 continue # Field line. diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 1f61151..6650c9b 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -563,6 +563,9 @@ const ROOT: usize = 0; // level, so the lines written under it are skipped with it instead of // re-parenting one level up. const DEAD: usize = usize::MAX; +// Stack entry for a line whose indent matched no open level (E012): never a +// level a sibling can bind at, but deeper lines are still under it. +const UNOPENED: usize = usize::MAX - 1; /// Merge a later instance into an earlier one under the in-file merge rule: /// children and trivia move over, first trailing wins (a second demotes to a @@ -1594,7 +1597,8 @@ impl Parser { .rev() .find(|(ind, node)| { *node != ROOT - && *node != DEAD && ind.len() >= new_indent.len() + && *node != DEAD && *node != UNOPENED + && ind.len() >= new_indent.len() && p.indent.starts_with(ind.as_str()) }) .map(|(ind, n)| (*n, ind.len() == p.indent.len())); @@ -1628,17 +1632,32 @@ impl Parser { return None; // sentinel invariant; degrade, never abort }; if indent.len() > top_indent.len() && indent.starts_with(top_indent.as_str()) { - return Some(*top_node); + return Some(if *top_node == UNOPENED { + DEAD + } else { + *top_node + }); } for i in (0..self.stack.len()).rev() { - if self.stack[i].0 == indent { + if self.stack[i].0 == indent && self.stack[i].1 != UNOPENED { // Sibling of stack[i]: its parent is the entry below it. let parent = if i == 0 { ROOT } else { self.stack[i - 1].1 }; // Keep the sentinel; a top-level line resolves to ROOT. self.stack.truncate(i.max(1)); - return Some(parent); + return Some(if parent == UNOPENED { DEAD } else { parent }); + } + } + // Skipped, but it still owns its indent: whatever is written deeper is + // skipped with it, and a sibling at the same bad indent is refused the + // same way instead of binding one level up. + while self.stack.len() > 1 { + let top = &self.stack[self.stack.len() - 1].0; + if indent.len() > top.len() && indent.starts_with(top.as_str()) { + break; } + self.stack.pop(); } + self.stack.push((indent.to_string(), UNOPENED)); None } @@ -2080,13 +2099,16 @@ impl Parser { self.hang_deeper_pending(indent); // Child-indent fence: a value line for its parent field. if let Some(fence) = fence_open(rest) { - let Some(parent) = self.resolve_parent(indent) else { + let parent = self.resolve_parent(indent); + let (value, next) = self.consume_raw(&lines, i + 1, lineno, indent, fence); + let Some(parent) = parent else { + // The body goes with its fence: parsed live, it would read as + // root bindings and the closing fence would open a second block. self.err(lineno, "E012", "indentation matches no open level"); self.lost += 1; - i += 1; + i = next; continue; }; - let (value, next) = self.consume_raw(&lines, i + 1, lineno, indent, fence); if parent == DEAD { self.skip_under_dead(lineno, indent); } else if let Some(node) = self.bind_block(parent, value, lineno) { @@ -2118,6 +2140,17 @@ impl Parser { i += 1; continue; } + let Some(parent) = self.resolve_parent(indent) else { + self.err(lineno, "E012", "indentation matches no open level"); + self.lost += 1; + i += 1; + continue; + }; + if parent == DEAD { + self.skip_under_dead(lineno, indent); + i += 1; + continue; + } self.err( lineno, "E013", @@ -2135,6 +2168,7 @@ impl Parser { blank_before: had_blank, ceiling: indent.len(), }); + self.stack.push((indent.to_string(), DEAD)); i += 1; continue; } From 2eab543174a7cb3a90f1c272dd8e1f13961fcf4a Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:30:20 -0700 Subject: [PATCH 02/19] index selector value is E002 --- project/backlog.md | 15 ++++++++++++--- .../076-index-selector-value/expected-diags.txt | 3 +++ .../076-index-selector-value/expected.shcl | 3 +++ .../076-index-selector-value/input.shcl | 7 +++++++ .../076-index-selector-value/reads.tsv | 7 +++++++ project/conformance/README.md | 2 ++ source/c/shcl.h | 7 +++++++ source/go/shcl.go | 6 ++++++ source/python/shcl.py | 5 +++++ source/rust/src/lib.rs | 10 ++++++++++ 10 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 project/conformance/076-index-selector-value/expected-diags.txt create mode 100644 project/conformance/076-index-selector-value/expected.shcl create mode 100644 project/conformance/076-index-selector-value/input.shcl create mode 100644 project/conformance/076-index-selector-value/reads.tsv diff --git a/project/backlog.md b/project/backlog.md index 27bda8a..7e94b19 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -51,18 +51,27 @@ Every item carries the date it was opened and, once settled, the date it closed. - The areas the 20260901 round recorded as not reached: the C parser and emitter read line by line, Go's validation walk, Python's validator, `v_suggest`, a full run under mingw and wine, the installers and packaging, `--layer` and merge semantics, and the three tooling scripts nobody had opened. Twenty-three defects here, the rest under Features and enhancements. Everything below was reproduced, not read off the code. - Nine of the defects are shapes all four bindings share, so the four-way check cannot see them. Seven are C or C++ only, which it also cannot see. Four were found only by running the windows builds; two are the release tooling. - - 🔘 Item 1: a line refused with `E012` does not hold its indent level, so what was written under it re-parents, and a refused fence line's body is parsed as live bindings. + - ✅ Item 1: a line refused with `E012` does not hold its indent level, so what was written under it re-parents, and a refused fence line's body is parsed as live bindings. - Reproduced in all four bindings. `d: 3` written under a refused `c: 2` becomes a child of the level above it, where the spec's `E018` row says it is skipped with the line it sits under. The 20260829 fix that added `E018` covered the `E014` and `E021` arms and missed all three `E012` arms. - The fence arm is the damaging one. A fence at a bad indent is skipped but its body is not consumed, so raw content becomes root bindings and the closing fence opens a second, unterminated block. + - Fixed: a line whose indent matches no open level now holds that indent as a level of its own, in all three `E012` arms. Deeper lines are skipped under it (`E018`), a refused fence line consumes its body, and a second line at the same bad indent is refused the same way rather than binding one level up. Spec rule reworded. + - Pinned by corpus `075`, which every binding failed before the change (a fence body read as root bindings, and a bad-indent line's children re-parenting up) and passes now. Existing `061` (a single `E012` line) is unchanged. - Opened: 20260901-190000 + - Closed: 20260902-090000 - - 🔘 Item 2: a line refused with `E013` (a `*` with no space) does not hold its level either, so its next sibling is lost. + - ✅ Item 2: a line refused with `E013` (a `*` with no space) does not hold its level either, so its next sibling is lost. - Reproduced in all four. Content under the bad line re-parents up, and the line's own next sibling then reports `E012` and is dropped. The `E014` arm beside it, same class of defect, does this right. + - Fixed: the `*`-with-no-space arm resolves its level first, like the `E014` arm beside it, and pushes the level as skipped. Its sibling binds where it should and what is written under it is skipped with it. + - Pinned by corpus `075` (the `p` block), failing before and passing now in all four. - Opened: 20260901-190100 + - Closed: 20260902-090000 - - 🔘 Item 3: a value after a last-segment index selector is dropped with no diagnostic and no lost count, so a save deletes it. + - ✅ Item 3: a value after a last-segment index selector is dropped with no diagnostic and no lost count, so a save deletes it. - Reproduced in all four. `a[0]: 2` loads clean and formats to nothing; the same with a raw block loads clean and the block vanishes. `fmt --write` exits 0 and bakes it in. The spec says the trailing value is reported as an error and counted as lost; the value-selector arm does exactly that, the index arm has no check at all. + - Fixed: the index arm carries the same last-segment check as the value arm: the trailing value is reported (`E002`) and counted as lost, so an in-place write refuses instead of deleting it. + - Pinned by corpus `076` (plain value and same-line fence after `a[0]`, plus a value after a non-last index that still binds). Failed in all four before, passes now. - Opened: 20260901-190200 + - Closed: 20260902-091500 - 🔘 Item 4: a fragment mounted at one node by two top-level schema paths is checked twice, and its diagnostics repeat. - Reproduced in all four. `field: a` and `field: "a[*]"` both inheriting one fragment report each of its faults twice. The spec says once per node. The dedupe set is created per top-level constraint, so it only covers mounts reached inside one constraint's own recursion. diff --git a/project/conformance/076-index-selector-value/expected-diags.txt b/project/conformance/076-index-selector-value/expected-diags.txt new file mode 100644 index 0000000..a9fd719 --- /dev/null +++ b/project/conformance/076-index-selector-value/expected-diags.txt @@ -0,0 +1,3 @@ +line 2: Error: E002 +line 3: Error: E002 +failed: 2 diagnostic(s), 2 error(s) diff --git a/project/conformance/076-index-selector-value/expected.shcl b/project/conformance/076-index-selector-value/expected.shcl new file mode 100644 index 0000000..4415e68 --- /dev/null +++ b/project/conformance/076-index-selector-value/expected.shcl @@ -0,0 +1,3 @@ +a: 1 +c: x + d: 4 diff --git a/project/conformance/076-index-selector-value/input.shcl b/project/conformance/076-index-selector-value/input.shcl new file mode 100644 index 0000000..4bae380 --- /dev/null +++ b/project/conformance/076-index-selector-value/input.shcl @@ -0,0 +1,7 @@ +a: 1 +a[0]: 2 +a[0]: ``` +body +``` +c: x +c[0].d: 4 diff --git a/project/conformance/076-index-selector-value/reads.tsv b/project/conformance/076-index-selector-value/reads.tsv new file mode 100644 index 0000000..347ed48 --- /dev/null +++ b/project/conformance/076-index-selector-value/reads.tsv @@ -0,0 +1,7 @@ +query type expected status +- load ok - +- load fail - strict +a int 1 Good +a count 1 Good +c.d int 4 Good +- paths a|c|c.d - diff --git a/project/conformance/README.md b/project/conformance/README.md index c7150f3..d31fa18 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -131,6 +131,8 @@ Case `061` pins `E012`: a dedent to a column that matches no open level is skipp Case `075` pins that a skipped line holds its indent level in the other two skip shapes too: a line refused with `E012`, and a `*` line with no space (`E013`). What is written under either is skipped with it (`E018`), a fence line at a bad indent takes its whole body with it, and a second line at the same bad indent is refused the same way rather than binding one level up. +Case `076` pins a value written after an index selector on the last segment (`a[0]: 2`): the instance is selected and the value is reported (`E002`) and counted as lost, exactly as after a value selector, so a save cannot quietly delete it. A same-line fence there is the same case. A value after an index that is not last still binds the deeper leaf. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/source/c/shcl.h b/source/c/shcl.h index 1203714..7fe8e64 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -2232,6 +2232,13 @@ static int attach_path(ShclParser *P, size_t parent, ShclSegment *segs, size_t n ShclSB m = {0}; sb_puts(P->line, &m, "no instance "); sb_put_u64(P->line, &m, seg->sel.index); sb_puts(P->line, &m, " of '"); sb_putS(P->line, &m, seg->name); sb_putc(P->line, &m, '\''); p_err(P, line, "E003", sb_S(&m)); P->d->lost++; return 0; } + /* Same as the value selector: the instance is already chosen, so a + trailing value has nowhere to bind. */ + if (is_last && !v_is_empty(&value)) { + ShclSB m = {0}; sb_puts(P->line, &m, "value after selector on '"); sb_putS(P->line, &m, seg->name); sb_puts(P->line, &m, "' ignored"); + p_err(P, line, "E002", sb_S(&m)); + P->d->lost++; + } break; } case SEL_WILDCARD: diff --git a/source/go/shcl.go b/source/go/shcl.go index b6e37be..2cfbfd2 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -1932,6 +1932,12 @@ func (p *parser) attachPath(parent int, segs []segment, v value, line int) (int, p.lost++ return 0, false } + if isLast && !v.isEmpty() { + // Same as the value selector: the instance is already chosen, + // so a trailing value has nowhere to bind. + p.err(line, "E002", fmt.Sprintf("value after selector on '%s' ignored", seg.name)) + p.lost++ + } case seg.sel != nil: p.err(line, "E004", "wildcard selector is query-only") p.lost++ diff --git a/source/python/shcl.py b/source/python/shcl.py index 88263b2..99d5943 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -1461,6 +1461,11 @@ def _attach_path(self, parent, segs, value, line): self._err(line, "E003", f"no instance {k} of '{seg.name}'") self.lost += 1 return None + if is_last and not value.is_empty(): + # Same as the value selector: the instance is already chosen, + # so a trailing value has nowhere to bind. + self._err(line, "E002", f"value after selector on '{seg.name}' ignored") + self.lost += 1 elif sel is not None and sel[0] == "wild": self._err(line, "E004", "wildcard selector is query-only") self.lost += 1 diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 6650c9b..ef128e1 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -1757,6 +1757,16 @@ impl Parser { self.lost += 1; return None; } + if is_last && value.as_ref().is_some_and(|v| !v.is_empty()) { + // Same as the value selector: the instance is already + // chosen, so a trailing value has nowhere to bind. + self.err( + line, + "E002", + format!("value after selector on '{}' ignored", seg.name), + ); + self.lost += 1; + } } (Some(Selector::Wildcard), _) => { self.err(line, "E004", "wildcard selector is query-only"); From f0c7302cba2dd788a3398aa7a509fe96abb54e83 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:32:11 -0700 Subject: [PATCH 03/19] fragment mounts once per node --- project/backlog.md | 5 ++++- .../077-schema-double-mount/expected-diags.txt | 1 + .../077-schema-double-mount/expected-validate.txt | 3 +++ .../077-schema-double-mount/expected.shcl | 3 +++ .../conformance/077-schema-double-mount/input.shcl | 3 +++ .../conformance/077-schema-double-mount/reads.tsv | 3 +++ .../077-schema-double-mount/schema.shcl | 8 ++++++++ project/conformance/README.md | 2 ++ source/c/shcl.h | 14 ++++++-------- source/go/shcl.go | 11 +++++------ source/python/shcl.py | 9 +++++---- source/rust/src/lib.rs | 11 +++++------ 12 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 project/conformance/077-schema-double-mount/expected-diags.txt create mode 100644 project/conformance/077-schema-double-mount/expected-validate.txt create mode 100644 project/conformance/077-schema-double-mount/expected.shcl create mode 100644 project/conformance/077-schema-double-mount/input.shcl create mode 100644 project/conformance/077-schema-double-mount/reads.tsv create mode 100644 project/conformance/077-schema-double-mount/schema.shcl diff --git a/project/backlog.md b/project/backlog.md index 7e94b19..b0b6a47 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -73,9 +73,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190200 - Closed: 20260902-091500 - - 🔘 Item 4: a fragment mounted at one node by two top-level schema paths is checked twice, and its diagnostics repeat. + - ✅ Item 4: a fragment mounted at one node by two top-level schema paths is checked twice, and its diagnostics repeat. - Reproduced in all four. `field: a` and `field: "a[*]"` both inheriting one fragment report each of its faults twice. The spec says once per node. The dedupe set is created per top-level constraint, so it only covers mounts reached inside one constraint's own recursion. + - Fixed: the mounted set is created once per validation and shared by every top-level constraint, so a (fragment, node) pair runs once whichever paths reach it. The per-constraint wrapper is gone in all four. + - Pinned by corpus `077` (`srv` and `srv[*]` both inheriting one fragment; each fault once). Failed in all four before, passes now. - Opened: 20260901-190300 + - Closed: 20260902-093000 - 🔘 Item 5: `H001`/`H002` disavowal re-reads the schema through the raw path text and the raw `repeat` value, so an escaped quote in a segment defeats it and a faulted `repeat` still disavows. - Reproduced in all four. `field: "a.\"b.c\""` with `repeat: 2` still prints the hint, while the unescaped spellings work. `repeat: 0x2` and a three-element `repeat` are schema faults and still silence the hint. diff --git a/project/conformance/077-schema-double-mount/expected-diags.txt b/project/conformance/077-schema-double-mount/expected-diags.txt new file mode 100644 index 0000000..4e7ad2e --- /dev/null +++ b/project/conformance/077-schema-double-mount/expected-diags.txt @@ -0,0 +1 @@ +ok (0 diagnostic(s)) diff --git a/project/conformance/077-schema-double-mount/expected-validate.txt b/project/conformance/077-schema-double-mount/expected-validate.txt new file mode 100644 index 0000000..a5c16c9 --- /dev/null +++ b/project/conformance/077-schema-double-mount/expected-validate.txt @@ -0,0 +1,3 @@ +line 2: Error: V003 +line 3: Error: V002 +failed: 2 diagnostic(s), 2 error(s) diff --git a/project/conformance/077-schema-double-mount/expected.shcl b/project/conformance/077-schema-double-mount/expected.shcl new file mode 100644 index 0000000..22e6d5d --- /dev/null +++ b/project/conformance/077-schema-double-mount/expected.shcl @@ -0,0 +1,3 @@ +srv: a + port: x +srv: b diff --git a/project/conformance/077-schema-double-mount/input.shcl b/project/conformance/077-schema-double-mount/input.shcl new file mode 100644 index 0000000..22e6d5d --- /dev/null +++ b/project/conformance/077-schema-double-mount/input.shcl @@ -0,0 +1,3 @@ +srv: a + port: x +srv: b diff --git a/project/conformance/077-schema-double-mount/reads.tsv b/project/conformance/077-schema-double-mount/reads.tsv new file mode 100644 index 0000000..6145027 --- /dev/null +++ b/project/conformance/077-schema-double-mount/reads.tsv @@ -0,0 +1,3 @@ +query type expected status +- load ok - +srv count 2 Good diff --git a/project/conformance/077-schema-double-mount/schema.shcl b/project/conformance/077-schema-double-mount/schema.shcl new file mode 100644 index 0000000..4bb22c7 --- /dev/null +++ b/project/conformance/077-schema-double-mount/schema.shcl @@ -0,0 +1,8 @@ +fragment: unit + field: port + type: int + required: yes +field: srv + inherits: unit +field: "srv[*]" + inherits: unit diff --git a/project/conformance/README.md b/project/conformance/README.md index d31fa18..7387786 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -133,6 +133,8 @@ Case `075` pins that a skipped line holds its indent level in the other two skip Case `076` pins a value written after an index selector on the last segment (`a[0]: 2`): the instance is selected and the value is reported (`E002`) and counted as lost, exactly as after a value selector, so a save cannot quietly delete it. A same-line fence there is the same case. A value after an index that is not last still binds the deeper leaf. +Case `077` pins that a fragment mounted at one node by two schema paths (`srv` and `srv[*]` both inheriting `unit`) runs once per node: each fault under it is reported once, not once per path. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/source/c/shcl.h b/source/c/shcl.h index 7fe8e64..2ac1fd3 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -4841,13 +4841,6 @@ static void v_check_from(ShclArena *a, ShclArena *lv, shcl_doc *d, const ShclVCo } } -static void v_check(ShclArena *a, ShclArena *lvls, shcl_doc *d, const ShclVCons *c, const ShclVSchemaDef *def, ShclVecDiag *out) { - // (fragment, node) pairs already mounted during this constraint's walk; - // entries live in the validation arena, so the set needs no own teardown. - ShclVMounts mounted; memset(&mounted, 0, sizeof mounted); - v_check_from(a, lvls, d, c, def, ROOT, 0, out, &mounted); -} - // Append a segment to a chain key. Chain keys join segments length-prefixed // (`:`), not with a bare NUL: NUL is legal in a quoted name, so a // single field named "x\0y" would impersonate the two-segment path x.y. Same @@ -5064,7 +5057,12 @@ shcl_validation *shcl_validate(shcl_doc *d, shcl_doc *schema) { if (!lvls) arena_panic(&panic); levels = lvls; for (size_t i = 0; i <= SHCL_MAX_DEPTH; i++) arena_guard(&lvls[i], &panic); - for (size_t i = 0; i < def.cons.len; i++) v_check(a, lvls, d, &def.cons.data[i], &def, &v->diags); + // One mount set for the whole schema: two top-level paths can resolve to + // the same node and mount the same fragment there, and the spec says each + // fragment runs once per node. Entries live in the validation arena, so + // the set needs no own teardown. + ShclVMounts mounted; memset(&mounted, 0, sizeof mounted); + for (size_t i = 0; i < def.cons.len; i++) v_check_from(a, lvls, d, &def.cons.data[i], &def, ROOT, 0, &v->diags, &mounted); // Nothing returns between the alloc and here, so every slot is reached. for (size_t i = 0; i <= SHCL_MAX_DEPTH; i++) arena_free(&lvls[i]); free(lvls); diff --git a/source/go/shcl.go b/source/go/shcl.go index 2cfbfd2..f47a7ca 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -6452,8 +6452,12 @@ func min3(a, b, c int) int { func (d *Document) Validate(schema *Document) []Diagnostic { def, faults := buildSchema(schema) out := faults + // One mount set for the whole schema: two top-level paths can resolve to + // the same node and mount the same fragment there, and the spec says each + // fragment runs once per node. + mounted := make(map[fragMount]bool) for i := range def.cons { - d.vCheck(&def.cons[i], &def, &out) + d.vCheckFrom(&def.cons[i], &def, root, 0, &out, mounted) } if def.pathsComplete { d.vUnknown(&def, &out) @@ -6537,11 +6541,6 @@ type fragMount struct { n int } -func (d *Document) vCheck(c *constraint, def *schemaDef, out *[]Diagnostic) { - mounted := make(map[fragMount]bool) - d.vCheckFrom(c, def, root, 0, out, mounted) -} - // A mounted fragment's fields run per resolved node, right after that node's // own checks, in fragment order - depth-first, so diagnostic order stays // derivable. Termination is structural: every mount descends at least one diff --git a/source/python/shcl.py b/source/python/shcl.py index 99d5943..1a768c0 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -3350,8 +3350,12 @@ def validate(self, schema: Document) -> list[Diagnostic]: a key-level fault keeps its entry's chain.""" sdef, faults = _build_schema(schema) out = faults + # One mount set for the whole schema: two top-level paths can resolve + # to the same node and mount the same fragment there, and the spec + # says each fragment runs once per node. + mounted: set = set() for c in sdef.cons: - self._v_check(c, sdef, out) + self._v_check_from(c, sdef, ROOT, 0, out, mounted) if sdef.paths_complete: self._v_unknown(sdef, out) return out @@ -3399,9 +3403,6 @@ def _v_contexts(self, start, segs, anchor, out): if not done: out.append((anchor, cur)) - def _v_check(self, c, sdef, out): - self._v_check_from(c, sdef, ROOT, 0, out, set()) - # A mounted fragment's fields run per resolved node, right after that # node's own checks, in fragment order - depth-first, so diagnostic order # stays derivable. Termination is structural: every mount descends at diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index ef128e1..51bdf40 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -6062,8 +6062,12 @@ impl Document { pub fn validate(&self, schema: &Document) -> Vec { let (def, faults) = build_schema(schema); let mut out = faults; + // One mount set for the whole schema: two top-level paths can resolve + // to the same node and mount the same fragment there, and the spec + // says each fragment runs once per node. + let mut mounted = std::collections::HashSet::new(); for c in &def.cons { - self.v_check(c, &def, &mut out); + self.v_check_from(c, &def, ROOT, 0, &mut out, &mut mounted); } if def.paths_complete { self.v_unknown(&def, &mut out); @@ -6140,11 +6144,6 @@ impl Document { out.push((anchor, cur)); } - fn v_check(&self, c: &Constraint, def: &SchemaDef, out: &mut Vec) { - let mut mounted = std::collections::HashSet::new(); - self.v_check_from(c, def, ROOT, 0, out, &mut mounted); - } - // A mounted fragment's fields run per resolved node, right after that // node's own checks, in fragment order - depth-first, so diagnostic order // stays derivable. Termination is structural: every mount descends at From 1f718aa41f506ec0033218fadab985a24ed92e90 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:36:02 -0700 Subject: [PATCH 04/19] disavow from the built schema --- project/backlog.md | 5 +- .../078-schema-disavow/expected-diags.txt | 6 ++ .../078-schema-disavow/expected-validate.txt | 7 ++ .../078-schema-disavow/expected.shcl | 14 +++ .../conformance/078-schema-disavow/input.shcl | 16 +++ .../conformance/078-schema-disavow/reads.tsv | 3 + .../078-schema-disavow/schema.shcl | 15 +++ project/conformance/README.md | 2 + source/c/shcl.h | 87 ++++++---------- source/go/shcl.go | 99 +++++++------------ source/python/shcl.py | 81 ++++++--------- source/rust/src/lib.rs | 98 ++++++------------ 12 files changed, 191 insertions(+), 242 deletions(-) create mode 100644 project/conformance/078-schema-disavow/expected-diags.txt create mode 100644 project/conformance/078-schema-disavow/expected-validate.txt create mode 100644 project/conformance/078-schema-disavow/expected.shcl create mode 100644 project/conformance/078-schema-disavow/input.shcl create mode 100644 project/conformance/078-schema-disavow/reads.tsv create mode 100644 project/conformance/078-schema-disavow/schema.shcl diff --git a/project/backlog.md b/project/backlog.md index b0b6a47..b98e700 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -80,10 +80,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190300 - Closed: 20260902-093000 - - 🔘 Item 5: `H001`/`H002` disavowal re-reads the schema through the raw path text and the raw `repeat` value, so an escaped quote in a segment defeats it and a faulted `repeat` still disavows. + - ✅ Item 5: `H001`/`H002` disavowal re-reads the schema through the raw path text and the raw `repeat` value, so an escaped quote in a segment defeats it and a faulted `repeat` still disavows. - Reproduced in all four. `field: "a.\"b.c\""` with `repeat: 2` still prints the hint, while the unescaped spellings work. `repeat: 0x2` and a three-element `repeat` are schema faults and still silence the hint. - Cause: both suppressors take paths from `instances()` display text and read `repeat` with the loose array read, where the schema build reads both correctly. Deriving the disavowed names from the built constraints closes both. + - Fixed: both suppressors take their names from the built schema (a shared `disavowed_names` helper over the top-level constraints and every fragment's), so the leaf name is the resolved one validation uses and a `repeat` or `reopen` that faulted disavows nothing. The constraint carries `reopen` now; the display-text and loose-array reads are gone from all four. + - Pinned by corpus `078` (escaped-quote path with `repeat: 2`, `repeat: 0x2`, a three-element `repeat`, `reopen: yes`, `reopen: 0x1`). Failed in all four before, passes now. - Opened: 20260901-190400 + - Closed: 20260902-100000 - 🔘 Item 6: a merge appends unmatched `over` nodes grouped by name, where the spec says file order. - Reproduced in all four. `c, s, c` in the layer comes out `c, c, s`. So merging onto an empty base is not the identity, and a layered `fmt` reorders siblings the layer's author put in a deliberate order. Reads are unaffected, which is why nothing caught it. diff --git a/project/conformance/078-schema-disavow/expected-diags.txt b/project/conformance/078-schema-disavow/expected-diags.txt new file mode 100644 index 0000000..677ffb3 --- /dev/null +++ b/project/conformance/078-schema-disavow/expected-diags.txt @@ -0,0 +1,6 @@ +line 12: Hint: H002 +line 15: Hint: H002 +line 5: Hint: H001 +line 7: Hint: H001 +line 3: Hint: H001 +ok (5 diagnostic(s)) diff --git a/project/conformance/078-schema-disavow/expected-validate.txt b/project/conformance/078-schema-disavow/expected-validate.txt new file mode 100644 index 0000000..1827e5a --- /dev/null +++ b/project/conformance/078-schema-disavow/expected-validate.txt @@ -0,0 +1,7 @@ +line 15: Hint: H002 +line 5: Hint: H001 +line 7: Hint: H001 +line 4: Error: V092 +line 6: Error: V092 +line 12: Error: V092 +failed: 6 diagnostic(s), 3 error(s) diff --git a/project/conformance/078-schema-disavow/expected.shcl b/project/conformance/078-schema-disavow/expected.shcl new file mode 100644 index 0000000..3c76580 --- /dev/null +++ b/project/conformance/078-schema-disavow/expected.shcl @@ -0,0 +1,14 @@ +a: + "b.c": 1 + "b.c": 2 +x: 1 +x: 2 +y: 1 +y: 2 +z: + p: 1 + q: 2 +w: + p: 1 + q: 2 +k: 1 diff --git a/project/conformance/078-schema-disavow/input.shcl b/project/conformance/078-schema-disavow/input.shcl new file mode 100644 index 0000000..c1562a9 --- /dev/null +++ b/project/conformance/078-schema-disavow/input.shcl @@ -0,0 +1,16 @@ +a: + "b.c": 1 + "b.c": 2 +x: 1 +x: 2 +y: 1 +y: 2 +z: + p: 1 +w: + p: 1 +z: + q: 2 +k: 1 +w: + q: 2 diff --git a/project/conformance/078-schema-disavow/reads.tsv b/project/conformance/078-schema-disavow/reads.tsv new file mode 100644 index 0000000..5961ded --- /dev/null +++ b/project/conformance/078-schema-disavow/reads.tsv @@ -0,0 +1,3 @@ +query type expected status +- load ok - +x count 2 Good diff --git a/project/conformance/078-schema-disavow/schema.shcl b/project/conformance/078-schema-disavow/schema.shcl new file mode 100644 index 0000000..2070010 --- /dev/null +++ b/project/conformance/078-schema-disavow/schema.shcl @@ -0,0 +1,15 @@ +field: "a.\"b.c\"" + repeat: 2 +field: x + repeat: 0x2 +field: y + repeat: 1, 2, 3 +field: z + reopen: yes +field: z.p +field: z.q +field: w + reopen: 0x1 +field: w.p +field: w.q +field: k diff --git a/project/conformance/README.md b/project/conformance/README.md index 7387786..4f277d1 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -135,6 +135,8 @@ Case `076` pins a value written after an index selector on the last segment (`a[ Case `077` pins that a fragment mounted at one node by two schema paths (`srv` and `srv[*]` both inheriting `unit`) runs once per node: each fault under it is reported once, not once per path. +Case `078` pins what a schema disavows: a `repeat` above 1 drops the `H001` hint for a field whose path carries an escaped quote, a `reopen: true` drops the `H002` hint, and a `repeat` or `reopen` that faults (`V092`) disavows nothing, so the hint stays beside the fault. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/source/c/shcl.h b/source/c/shcl.h index 2ac1fd3..34d3a66 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -4267,6 +4267,7 @@ typedef struct { int has_min_i, has_max_i, has_min_f, has_max_f; int64_t min_i, max_i; double min_f, max_f; int has_repeat; uint64_t rep_lo, rep_hi; + int reopen; // H002 suppressor only; validation ignores it ShclStr inherits; // fragment mounted at this path (subtree shape); .n == 0 = none size_t inherits_line; // schema line of the `inherits` key, for V095 // Generator-only (`shcl init`): validation ignores both. has_* gates them. @@ -4380,12 +4381,12 @@ static int v_parse_field(ShclArena *a, shcl_doc *schema, size_t f, ShclVecDiag * if (ok && required < 0) required = b; else v_diag(a, faults, kid->line, "V092", v_msg_key(a, "required")); } else if (s_eq(kid->name, s_lit("reopen"))) { - /* Consumed by the H002 suppressor (which reads the schema document - directly); validation itself ignores it, but a bad value still - faults so a typo cannot silently disavow nothing. */ + /* Consumed by the H002 suppressor; validation itself ignores it, + but a bad value still faults so a typo cannot silently disavow + nothing. */ ShclStr t; int b = 0; int ok = v_single_text(a, &kid->value, &t) && parse_bool_text(a, t, SHCL_STANDARD, &b); - if (ok && !reopen_seen) reopen_seen = 1; + if (ok && !reopen_seen) { reopen_seen = 1; c.reopen = b; } else v_diag(a, faults, kid->line, "V092", v_msg_key(a, "reopen")); } else if (s_eq(kid->name, s_lit("allowed"))) { if (kid->value.kind == V_CELL && allowed_at == (size_t)-1) allowed_at = kids.data[ki]; @@ -5082,42 +5083,37 @@ shcl_str shcl_validation_message(const shcl_validation *v, size_t i) { const char *shcl_validation_code(const shcl_validation *v, size_t i) { return v->diags.data[i].code; } void shcl_validation_free(shcl_validation *v) { if (!v) return; arena_free(&v->arena); arena_free(&v->scratch); free(v); } +/* Leaf names of the schema entries pick accepts, top-level fields and every + fragment's fields alike. Read through the built schema, so the names are + the ones validation will use (escapes resolved) and an entry whose key + faulted disavows nothing. Everything is built in tmp. */ +static int v_pick_repeat(const ShclVCons *c) { return c->has_repeat && c->rep_hi > 1; } +static int v_pick_reopen(const ShclVCons *c) { return c->reopen; } +static ShclVecS v_disavowed_names(shcl_doc *schema, ShclArena *tmp, int (*pick)(const ShclVCons *)) { + ShclVSchemaDef def; memset(&def, 0, sizeof def); + ShclVecDiag faults = {0}; + v_build_schema(tmp, schema, &def, &faults); + ShclVecS names = {0}; + for (size_t g = 0; g <= def.frags.len; g++) { + const ShclVecVCons *list = g == 0 ? &def.cons : &def.frags.data[g - 1].fields; + for (size_t i = 0; i < list->len; i++) { + const ShclVCons *c = &list->data[i]; + if (!pick(c) || c->segs.len == 0) continue; + /* Name wildcard: no single leaf name to disavow. */ + const ShclSegment *last = &c->segs.data[c->segs.len - 1]; + if (!last->star && last->name.n) ShclVecS_push(tmp, &names, last->name); + } + } + return names; +} + void shcl_suppress_declared_repeats(shcl_doc *schema, shcl_doc *doc) { /* Everything this probe builds - instance/repeat query results as well as the collected names (which must survive the per-read scratch resets) - goes into its own arena, freed on exit: the function owns neither doc, so it must not leave allocations behind in either. */ ShclArena tmp; memset(&tmp, 0, sizeof tmp); - ShclVecS names = {0}; - /* Top-level fields plus every fragment's fields: a repeat declared inside - a mounted shape disavows the hint the same way. */ - size_t nfrag = shcl_count(schema, "fragment", 8); - for (size_t g = 0; g <= nfrag; g++) { - char base[48]; - int bn = g == 0 ? snprintf(base, sizeof base, "field") - : snprintf(base, sizeof base, "fragment[#%zu].field", g - 1); - shcl_str *paths; - ShclStr bp; bp.p = base; bp.n = (size_t)bn; - size_t np = instances_in(schema, &tmp, bp, &paths); - for (size_t i = 0; i < np; i++) { - char q[80]; - int qn = snprintf(q, sizeof q, "%s[#%zu].repeat", base, i); - /* repeat is a 1-2 element array (`repeat: lo[, hi]`); the bound - that matters here is the last one. */ - ShclStr qp; qp.p = q; qp.n = (size_t)qn; - shcl_read_i64_arr rep = read_int_array_in(schema, &tmp, qp); - if (rep.status != SHCL_GOOD || rep.n == 0 || rep.values[rep.n - 1] <= 1) continue; - ShclStr p; p.p = paths[i].p; p.n = paths[i].n; - /* Leaf name from the parsed path, not a re-split of its text: a - quoted last segment may contain dots (`a."b.c"`). The scanner - folds the name; the doc side stores names folded too. */ - ShclPathScan ps = scan_lookup(&tmp, p); - if (!ps.ok || ps.segs.len == 0) continue; - const ShclSegment *last = &ps.segs.data[ps.segs.len - 1]; - if (last->star) continue; /* name wildcard: no single leaf name to disavow */ - if (last->name.n) ShclVecS_push(&tmp, &names, last->name); - } - } + ShclVecS names = v_disavowed_names(schema, &tmp, v_pick_repeat); if (!names.len) { arena_free(&tmp); return; } ShclVecS heads = {0}; for (size_t k = 0; k < names.len; k++) ShclVecS_push(&tmp, &heads, h001_head(&tmp, names.data[k])); @@ -5141,28 +5137,7 @@ void shcl_suppress_declared_repeats(shcl_doc *schema, shcl_doc *doc) { void shcl_suppress_declared_reopens(shcl_doc *schema, shcl_doc *doc) { /* Same arena discipline as the H001 suppressor above. */ ShclArena tmp; memset(&tmp, 0, sizeof tmp); - ShclVecS names = {0}; - size_t nfrag = shcl_count(schema, "fragment", 8); - for (size_t g = 0; g <= nfrag; g++) { - char base[48]; - int bn = g == 0 ? snprintf(base, sizeof base, "field") - : snprintf(base, sizeof base, "fragment[#%zu].field", g - 1); - shcl_str *paths; - ShclStr bp; bp.p = base; bp.n = (size_t)bn; - size_t np = instances_in(schema, &tmp, bp, &paths); - for (size_t i = 0; i < np; i++) { - char q[80]; - int qn = snprintf(q, sizeof q, "%s[#%zu].reopen", base, i); - shcl_read_bool re = shcl_read_bool_(schema, q, (size_t)qn); - if (re.status != SHCL_GOOD || !re.value) continue; - ShclStr p; p.p = paths[i].p; p.n = paths[i].n; - ShclPathScan ps = scan_lookup(&tmp, p); - if (!ps.ok || ps.segs.len == 0) continue; - const ShclSegment *last = &ps.segs.data[ps.segs.len - 1]; - if (last->star) continue; /* name wildcard: no single leaf name to disavow */ - if (last->name.n) ShclVecS_push(&tmp, &names, last->name); - } - } + ShclVecS names = v_disavowed_names(schema, &tmp, v_pick_reopen); if (!names.len) { arena_free(&tmp); return; } ShclVecS heads = {0}; for (size_t k = 0; k < names.len; k++) ShclVecS_push(&tmp, &heads, h002_head(&tmp, names.data[k])); diff --git a/source/go/shcl.go b/source/go/shcl.go index f47a7ca..41c9b0f 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -2793,40 +2793,7 @@ func h001Head(name string) string { // allocation and never disturbs the input (the reference filters its list in // place behind &mut; a Go return reads as a copy, so it must behave as one). func SuppressDeclaredRepeats(schema *Document, diags []Diagnostic) []Diagnostic { - // Top-level fields plus every fragment's fields: a repeat declared inside - // a mounted shape disavows the hint the same way. - type group struct { - base string - paths []string - } - groups := []group{{"field", schema.Instances("field")}} - for k := 0; k < schema.Count("fragment"); k++ { - base := fmt.Sprintf("fragment[#%d].field", k) - groups = append(groups, group{base, schema.Instances(base)}) - } - var names []string - for _, g := range groups { - for i, p := range g.paths { - // repeat is a 1-2 element array (`repeat: lo[, hi]`); the bound - // that matters here is the last one. - rep := schema.ReadIntArray(fmt.Sprintf("%s[#%d].repeat", g.base, i)) - if rep.Status != Good || len(rep.Value) == 0 || rep.Value[len(rep.Value)-1] <= 1 { - continue - } - // Leaf name from the parsed path, not a re-split of its text: a - // quoted last segment may contain dots (`a."b.c"`). The scanner - // folds the name; the doc side stores names folded too. - scan, err := scanLookup(p) - if err != nil || len(scan.segments) == 0 { - continue - } - seg := scan.segments[len(scan.segments)-1] - if seg.star { - continue // name wildcard: no single leaf name to disavow - } - names = append(names, seg.name) - } - } + names := disavowedNames(schema, func(c *constraint) bool { return c.repeat != nil && c.repeat[1] > 1 }) if len(names) == 0 { return diags } @@ -3148,6 +3115,32 @@ func (d *Document) SaveFileLossy(path string) error { return WriteFileAtomic(path, d.ToCanonical()) } +// disavowedNames returns the leaf names of the schema entries pick accepts, +// top-level fields and every fragment's fields alike. Read through the built +// schema, so the names are the ones validation will use (escapes resolved) +// and an entry whose key faulted disavows nothing. +func disavowedNames(schema *Document, pick func(*constraint) bool) []string { + def, _ := buildSchema(schema) + var names []string + all := [][]constraint{def.cons} + for _, fcs := range def.frags { + all = append(all, fcs) + } + for _, list := range all { + for i := range list { + c := &list[i] + if !pick(c) || len(c.segs) == 0 { + continue + } + // Name wildcard: no single leaf name to disavow. + if seg := c.segs[len(c.segs)-1]; !seg.star { + names = append(names, seg.name) + } + } + } + return names +} + // h002Head is the single H002 wording site: the merge hint and the schema // suppressor both come here, same discipline as h001Head. func h002Head(name string) string { @@ -3161,33 +3154,7 @@ func h002Head(name string) string { // Used by `check --schema` and LoadAndValidate; call it wherever doc // diagnostics and a schema meet. Returns a fresh slice like the H001 one. func SuppressDeclaredReopens(schema *Document, diags []Diagnostic) []Diagnostic { - type group struct { - base string - paths []string - } - groups := []group{{"field", schema.Instances("field")}} - for k := 0; k < schema.Count("fragment"); k++ { - base := fmt.Sprintf("fragment[#%d].field", k) - groups = append(groups, group{base, schema.Instances(base)}) - } - var names []string - for _, g := range groups { - for i, p := range g.paths { - re := schema.ReadBool(fmt.Sprintf("%s[#%d].reopen", g.base, i)) - if re.Status != Good || !re.Value { - continue - } - scan, err := scanLookup(p) - if err != nil || len(scan.segments) == 0 { - continue - } - seg := scan.segments[len(scan.segments)-1] - if seg.star { - continue // name wildcard: no single leaf name to disavow - } - names = append(names, seg.name) - } - } + names := disavowedNames(schema, func(c *constraint) bool { return c.reopen }) if len(names) == 0 { return diags } @@ -5549,6 +5516,7 @@ type constraint struct { minF *float64 maxF *float64 repeat *[2]uint64 + reopen bool // H002 suppressor only; validation ignores it inherits string // fragment mounted at this path (subtree shape); "" = none inheritsLine int // schema line of the `inherits` key, for V095 // Generator-only (`shcl init`): validation ignores both. @@ -5729,16 +5697,17 @@ func parseField(schema *Document, f int, faults *[]Diagnostic) (constraint, bool } else { vdiag(faults, kid.line, "V092", "bad schema constraint 'required'") } - // Consumed by the H002 suppressor (which reads the schema document - // directly); validation itself ignores it, but a bad value still - // faults so a typo cannot silently disavow nothing. + // Consumed by the H002 suppressor; validation itself ignores it, but + // a bad value still faults so a typo cannot silently disavow nothing. case "reopen": t, ok := singleText(&kid.value) + var b bool if ok { - _, ok = parseBoolText(t, Standard) + b, ok = parseBoolText(t, Standard) } if ok && !reopenSeen { reopenSeen = true + c.reopen = b } else { vdiag(faults, kid.line, "V092", "bad schema constraint 'reopen'") } diff --git a/source/python/shcl.py b/source/python/shcl.py index 1a768c0..beda4f4 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -3650,35 +3650,7 @@ def suppress_declared_repeats(schema: Document, diags: list[Diagnostic]) -> None consumers were hand-rolling - which errs toward quiet, for a hint. Used by `check --schema` and load_and_validate; call it wherever doc diagnostics and a schema meet. Mutates diags in place.""" - # Top-level fields plus every fragment's fields: a repeat declared inside - # a mounted shape disavows the hint the same way. - groups = [("field", schema.instances("field"))] - for k in range(schema.count("fragment")): - base = f"fragment[#{k}].field" - groups.append((base, schema.instances(base))) - names = [] - for base, paths in groups: - for i, p in enumerate(paths): - # repeat is a 1-2 element array (`repeat: lo[, hi]`); the bound - # that matters here is the last one. - rep = schema.read_int_array(f"{base}[#{i}].repeat") - if rep.status != Status.Good: - continue - if not rep.value or rep.value[-1] <= 1: - continue - # Leaf name from the parsed path, not a re-split of its text: a - # quoted last segment may contain dots (`a."b.c"`). The scanner - # folds the name; the doc side stores names folded too. - try: - segments, _ = _scan_lookup(p) - except _PathError: - continue - if not segments: - continue - seg = segments[-1] - if seg.star: - continue # name wildcard: no single leaf name to disavow - names.append(seg.name) + names = _disavowed_names(schema, lambda c: c.repeat is not None and c.repeat[1] > 1) if not names: return heads = [_h001_head(n) for n in names] @@ -3940,6 +3912,25 @@ def _set_read_only(path, on): pass +def _disavowed_names(schema, pick): + """Leaf names of the schema entries `pick` accepts, top-level fields and + every fragment's fields alike. Read through the built schema, so the names + are the ones validation will use (escapes resolved) and an entry whose key + faulted disavows nothing.""" + sdef, _ = _build_schema(schema) + names = [] + lists = [sdef.cons] + list(sdef.frags.values()) + for cons in lists: + for c in cons: + if not pick(c) or not c.segs: + continue + # Name wildcard: no single leaf name to disavow. + seg = c.segs[-1] + if not seg.star: + names.append(seg.name) + return names + + def _h002_head(name): """The single H002 wording site: the merge hint and the schema suppressor both come here, same discipline as _h001_head.""" @@ -3953,26 +3944,7 @@ def suppress_declared_reopens(schema: Document, diags: list[Diagnostic]) -> None H001 suppressor, and it errs toward quiet, for a hint. Used by `check --schema` and load_and_validate; call it wherever doc diagnostics and a schema meet. Mutates diags in place.""" - groups = [("field", schema.instances("field"))] - for k in range(schema.count("fragment")): - base = f"fragment[#{k}].field" - groups.append((base, schema.instances(base))) - names = [] - for base, paths in groups: - for i, p in enumerate(paths): - re = schema.read_bool(f"{base}[#{i}].reopen") - if re.status != Status.Good or not re.value: - continue - try: - segments, _ = _scan_lookup(p) - except _PathError: - continue - if not segments: - continue - seg = segments[-1] - if seg.star: - continue # name wildcard: no single leaf name to disavow - names.append(seg.name) + names = _disavowed_names(schema, lambda c: c.reopen) if not names: return heads = [_h002_head(n) for n in names] @@ -4522,7 +4494,7 @@ def parse_datetime(text: str) -> ShclDateTime | None: class _Constraint: __slots__ = ( "path", "segs", "ty", "required", "allowed", - "min_i", "max_i", "min_f", "max_f", "repeat", + "min_i", "max_i", "min_f", "max_f", "repeat", "reopen", "inherits", "inherits_line", "desc", "default_text", ) @@ -4538,6 +4510,7 @@ def __init__(self, path, segs): self.min_f = None self.max_f = None self.repeat = None # (lo, hi) + self.reopen = False # H002 suppressor only; validation ignores it self.inherits = None # fragment mounted at this path (subtree shape) self.inherits_line = 0 # schema line of the `inherits` key, for V095 # Generator-only (`shcl init`): validation ignores both. @@ -4554,6 +4527,7 @@ def clone(self): cc.min_f = self.min_f cc.max_f = self.max_f cc.repeat = self.repeat + cc.reopen = self.reopen cc.inherits = self.inherits cc.inherits_line = self.inherits_line cc.desc = self.desc @@ -4699,13 +4673,14 @@ def _parse_field(schema, f, faults): else: _vdiag(faults, kid.line, "V092", "bad schema constraint 'required'") elif kid.name == "reopen": - # Consumed by the H002 suppressor (which reads the schema document - # directly); validation itself ignores it, but a bad value still - # faults so a typo cannot silently disavow nothing. + # Consumed by the H002 suppressor; validation itself ignores it, + # but a bad value still faults so a typo cannot silently disavow + # nothing. t = _single_text(kid.value) b = _parse_bool_text(t, Strictness.Standard) if t is not None else None if b is not None and not reopen_seen: reopen_seen = True + c.reopen = b else: _vdiag(faults, kid.line, "V092", "bad schema constraint 'reopen'") elif kid.name == "allowed": diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 51bdf40..457e517 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -3019,43 +3019,7 @@ fn h001_head(name: &str) -> String { /// `check --schema` and load_and_validate; call it wherever doc diagnostics /// and a schema meet. pub fn suppress_declared_repeats(schema: &Document, diags: &mut Vec) { - // Top-level fields plus every fragment's fields: a repeat declared inside - // a mounted shape disavows the hint the same way. - let mut groups: Vec<(String, Vec)> = - vec![("field".to_string(), schema.instances("field"))]; - for k in 0..schema.count("fragment") { - let base = format!("fragment[#{}].field", k); - let paths = schema.instances(&base); - groups.push((base, paths)); - } - let mut names: Vec = Vec::new(); - for (base, paths) in &groups { - for (i, p) in paths.iter().enumerate() { - // repeat is a 1-2 element array (`repeat: lo[, hi]`); the bound - // that matters here is the last one. - let rep = schema.read_int_array(&format!("{}[#{}].repeat", base, i)); - if rep.status != Status::Good { - continue; - } - match rep.value.last() { - Some(&u) if u > 1 => {} - _ => continue, - } - // Leaf name from the parsed path, not a re-split of its text: a - // quoted last segment may contain dots (`a."b.c"`). The scanner - // folds the name; the doc side stores names folded too. - let Ok(scan) = scan_lookup(p) else { - continue; - }; - let Some(seg) = scan.segments.last() else { - continue; - }; - if seg.star { - continue; // name wildcard: no single leaf name to disavow - } - names.push(seg.name.clone()); - } - } + let names = disavowed_names(schema, |c| c.repeat.is_some_and(|(_, hi)| hi > 1)); if names.is_empty() { return; } @@ -3063,6 +3027,26 @@ pub fn suppress_declared_repeats(schema: &Document, diags: &mut Vec) diags.retain(|d| d.code != "H001" || !heads.iter().any(|h| d.message.starts_with(h.as_str()))); } +/// Leaf names of the schema entries `pick` accepts, top-level fields and every +/// fragment's fields alike. Read through the built schema, so the names are +/// the ones validation will use (escapes resolved) and an entry whose key +/// faulted disavows nothing. +fn disavowed_names(schema: &Document, pick: impl Fn(&Constraint) -> bool) -> Vec { + let (def, _) = build_schema(schema); + let mut names: Vec = Vec::new(); + let frags = def.frags.values().flat_map(|v| v.iter()); + for c in def.cons.iter().chain(frags) { + if !pick(c) { + continue; + } + // Name wildcard: no single leaf name to disavow. + if let Some(seg) = c.segs.last().filter(|seg| !seg.star) { + names.push(seg.name.clone()); + } + } + names +} + /// The single H002 wording site: the merge hint and the schema suppressor /// both come here, same discipline as h001_head. fn h002_head(name: &str) -> String { @@ -3076,32 +3060,7 @@ fn h002_head(name: &str) -> String { /// `check --schema` and load_and_validate; call it wherever doc diagnostics /// and a schema meet. pub fn suppress_declared_reopens(schema: &Document, diags: &mut Vec) { - let mut groups: Vec<(String, Vec)> = - vec![("field".to_string(), schema.instances("field"))]; - for k in 0..schema.count("fragment") { - let base = format!("fragment[#{}].field", k); - let paths = schema.instances(&base); - groups.push((base, paths)); - } - let mut names: Vec = Vec::new(); - for (base, paths) in &groups { - for (i, p) in paths.iter().enumerate() { - let re = schema.read_bool(&format!("{}[#{}].reopen", base, i)); - if re.status != Status::Good || !re.value { - continue; - } - let Ok(scan) = scan_lookup(p) else { - continue; - }; - let Some(seg) = scan.segments.last() else { - continue; - }; - if seg.star { - continue; // name wildcard: no single leaf name to disavow - } - names.push(seg.name.clone()); - } - } + let names = disavowed_names(schema, |c| c.reopen); if names.is_empty() { return; } @@ -5185,6 +5144,7 @@ struct Constraint { min_f: Option, max_f: Option, repeat: Option<(u64, u64)>, + reopen: bool, // H002 suppressor only; validation ignores it inherits: Option, // fragment mounted at this path (subtree shape) inherits_line: usize, // schema line of the `inherits` key, for V095 // Generator-only (`shcl init`): validation ignores both. @@ -5358,6 +5318,7 @@ fn parse_field(schema: &Document, f: usize, faults: &mut Vec) -> Opt min_f: None, max_f: None, repeat: None, + reopen: false, inherits: None, inherits_line: 0, desc: None, @@ -5416,14 +5377,17 @@ fn parse_field(schema: &Document, f: usize, faults: &mut Vec) -> Opt ), } } - // Consumed by the H002 suppressor (which reads the schema document - // directly); validation itself ignores it, but a bad value still - // faults so a typo cannot silently disavow nothing. + // Consumed by the H002 suppressor; validation itself ignores it, + // but a bad value still faults so a typo cannot silently disavow + // nothing. "reopen" => { let v = single_text(&kid.value).and_then(|t| parse_bool_text(&t, Strictness::Standard)); match v { - Some(_) if !reopen_seen => reopen_seen = true, + Some(b) if !reopen_seen => { + reopen_seen = true; + c.reopen = b; + } _ => vdiag( faults, kid.line, From 61b5eea9b6c390303e2229a5ec46cd813705ce7c Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:40:12 -0700 Subject: [PATCH 05/19] merge keeps file order --- project/backlog.md | 10 ++++- .../079-merge-file-order/expected-diags.txt | 2 + .../079-merge-file-order/expected-merged.shcl | 17 +++++++++ .../079-merge-file-order/expected.shcl | 14 +++++++ .../079-merge-file-order/input.shcl | 14 +++++++ .../079-merge-file-order/layer1.shcl | 6 +++ .../079-merge-file-order/reads.tsv | 3 ++ project/conformance/README.md | 2 + source/c/shcl.h | 38 ++++++++++++------- source/go/shcl.go | 36 +++++++++++------- source/python/shcl.py | 26 +++++++------ source/rust/src/lib.rs | 32 +++++++++------- source/rust/tests/fuzz_smoke.rs | 10 +++++ 13 files changed, 156 insertions(+), 54 deletions(-) create mode 100644 project/conformance/079-merge-file-order/expected-diags.txt create mode 100644 project/conformance/079-merge-file-order/expected-merged.shcl create mode 100644 project/conformance/079-merge-file-order/expected.shcl create mode 100644 project/conformance/079-merge-file-order/input.shcl create mode 100644 project/conformance/079-merge-file-order/layer1.shcl create mode 100644 project/conformance/079-merge-file-order/reads.tsv diff --git a/project/backlog.md b/project/backlog.md index b98e700..ed44bdd 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -88,13 +88,19 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190400 - Closed: 20260902-100000 - - 🔘 Item 6: a merge appends unmatched `over` nodes grouped by name, where the spec says file order. + - ✅ Item 6: a merge appends unmatched `over` nodes grouped by name, where the spec says file order. - Reproduced in all four. `c, s, c` in the layer comes out `c, c, s`. So merging onto an empty base is not the identity, and a layered `fmt` reorders siblings the layer's author put in a deliberate order. Reads are unaffected, which is why nothing caught it. + - Fixed: each appended clone remembers its position among the over node's children and the rebuild emits them in that order. Replaced leaf groups still splice at the name's first base position, as the spec says. + - Pinned by corpus `079` (layer appends `c, a, b, a` in file order) and the merge fuzz property, which now also asserts that a merge onto an empty document is the identity over every seed and mutation. Both failed in all four before and pass now. - Opened: 20260901-190500 + - Closed: 20260902-104500 - - 🔘 Item 7: a layer's own repeated footer comments collapse to one. + - ✅ Item 7: a layer's own repeated footer comments collapse to one. - Reproduced in all four. The once-per-layer footer dedupe compares each line against a list that already holds the lines just added from the same layer, so a within-file repeat is dropped. Another way an empty-base merge fails to be the identity. + - Fixed: the footer dedupe compares against the lines the base held before the merge, so a layer's own repeated line is kept and a line the base already has is still carried once. + - Pinned by corpus `079` (`# more` twice in the top layer) and the same identity property. - Opened: 20260901-190600 + - Closed: 20260902-104500 - 🔘 Item 8: the did-you-mean suggestion is quadratic in name length, so a check against a schema with long field names takes seconds to minutes. - Measured: 300 unknown fields of 300 characters against 300 schema names of 300 characters, 188 KB in all, takes 23 s in the release reference, 27 s in Go, 10 s in C. Double both lengths and the reference did not finish in two minutes. diff --git a/project/conformance/079-merge-file-order/expected-diags.txt b/project/conformance/079-merge-file-order/expected-diags.txt new file mode 100644 index 0000000..1df818c --- /dev/null +++ b/project/conformance/079-merge-file-order/expected-diags.txt @@ -0,0 +1,2 @@ +line 4: Hint: H001 +ok (1 diagnostic(s)) diff --git a/project/conformance/079-merge-file-order/expected-merged.shcl b/project/conformance/079-merge-file-order/expected-merged.shcl new file mode 100644 index 0000000..95a5197 --- /dev/null +++ b/project/conformance/079-merge-file-order/expected-merged.shcl @@ -0,0 +1,17 @@ +# head +base: 1 +s: + p: 1 + x: 1 +c: 1 +c: 2 +a: q + k: 1 +b: 1 +a: r + k: 2 +new: + y: 1 +# foot +# more +# more diff --git a/project/conformance/079-merge-file-order/expected.shcl b/project/conformance/079-merge-file-order/expected.shcl new file mode 100644 index 0000000..0c043b2 --- /dev/null +++ b/project/conformance/079-merge-file-order/expected.shcl @@ -0,0 +1,14 @@ +c: 1 +s: + x: 1 +c: 2 +a: q + k: 1 +b: 1 +a: r + k: 2 +new: + y: 1 +# foot +# more +# more diff --git a/project/conformance/079-merge-file-order/input.shcl b/project/conformance/079-merge-file-order/input.shcl new file mode 100644 index 0000000..0c043b2 --- /dev/null +++ b/project/conformance/079-merge-file-order/input.shcl @@ -0,0 +1,14 @@ +c: 1 +s: + x: 1 +c: 2 +a: q + k: 1 +b: 1 +a: r + k: 2 +new: + y: 1 +# foot +# more +# more diff --git a/project/conformance/079-merge-file-order/layer1.shcl b/project/conformance/079-merge-file-order/layer1.shcl new file mode 100644 index 0000000..5eb6172 --- /dev/null +++ b/project/conformance/079-merge-file-order/layer1.shcl @@ -0,0 +1,6 @@ +# head +base: 1 +s: + p: 1 +c: 0 +# foot diff --git a/project/conformance/079-merge-file-order/reads.tsv b/project/conformance/079-merge-file-order/reads.tsv new file mode 100644 index 0000000..ccab32a --- /dev/null +++ b/project/conformance/079-merge-file-order/reads.tsv @@ -0,0 +1,3 @@ +query type expected status +- load ok - +c count 2 Good diff --git a/project/conformance/README.md b/project/conformance/README.md index 4f277d1..aa94fc4 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -137,6 +137,8 @@ Case `077` pins that a fragment mounted at one node by two schema paths (`srv` a Case `078` pins what a schema disavows: a `repeat` above 1 drops the `H001` hint for a field whose path carries an escaped quote, a `reopen: true` drops the `H002` hint, and a `repeat` or `reopen` that faults (`V092`) disavows nothing, so the hint stays beside the fault. +Case `079` pins the order a merge appends in: unmatched higher-layer nodes keep that file's order (`c, a, b, a`) rather than regrouping by name, and a footer line the layer repeats itself is kept twice, while one the base already carries is carried over once. Merging onto an empty base is the identity. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/source/c/shcl.h b/source/c/shcl.h index 34d3a66..b41253e 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -3593,8 +3593,9 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { ShclArena *a = &d->arena; ShclArena *t = &d->scratch; ShclVecSize okids = over->nodes.data[op].children; // const doc: stable - // Over side: name -> bucket, in first-appearance order. Map hits verify - // against what the entry's value names (hash-only entries store no key). + // Over side: name -> bucket of child positions, in first-appearance + // order. Map hits verify against what the entry's value names (hash-only + // entries store no key). ShclVecS order = {0}; ShclVecSize *buckets = NULL; size_t nb = 0, cb = 0; ShclCMap group_of; memset(&group_of, 0, sizeof group_of); for (size_t i = 0; i < okids.len; i++) { @@ -3610,7 +3611,7 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { cmap_put(t, &group_of, h, g); ShclVecS_push(t, &order, nm); } - ShclVecSize_push(t, &buckets[g], k); + ShclVecSize_push(t, &buckets[g], i); } // Base side, one pass: does the name exist / have a container instance // (entries name a representative base child), and which child carries @@ -3644,10 +3645,14 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { // not a leaf, so it falls through to the instance merge: a bare section // header in a higher layer never wipes the subtree below it. Replaced // groups splice in the rebuild; everything appended (unmatched instances, - // and replaced names base never had) keeps processing order. + // and replaced names base never had) keeps the over file's order, which + // the per-name pass here would otherwise regroup: app_at is indexed by + // the over child's position. ShclVecSize *rep = (ShclVecSize *)arena_alloc(t, (nb ? nb : 1) * sizeof(ShclVecSize)); int *is_rep = (int *)arena_alloc(t, (nb ? nb : 1) * sizeof(int)); - ShclVecSize appended = {0}; + size_t *app_at = (size_t *)arena_alloc(t, (okids.len ? okids.len : 1) * sizeof(size_t)); + for (size_t i = 0; i < okids.len; i++) app_at[i] = (size_t)-1; + size_t nappended = 0; int any_rep = 0; ShclValue ev; memset(&ev, 0, sizeof ev); ev.kind = V_EMPTY; for (size_t gi = 0; gi < nb; gi++) { @@ -3655,7 +3660,7 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { ShclVecSize grp = buckets[gi]; memset(&rep[gi], 0, sizeof rep[gi]); is_rep[gi] = 0; int over_leafy = 1; - for (size_t i = 0; i < grp.len; i++) if (over->nodes.data[grp.data[i]].children.len > 0) { over_leafy = 0; break; } + for (size_t i = 0; i < grp.len; i++) if (over->nodes.data[okids.data[grp.data[i]]].children.len > 0) { over_leafy = 0; break; } uint64_t hn = cmap_hash(name, s_empty()); int inb = 0, bc = 0; for (ShclCMapEnt *e = cmap_first(&in_base, hn); e; e = cmap_next(e, hn)) @@ -3664,13 +3669,16 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { if (s_eq(NODE(d, e->val).name, name)) { bc = 1; break; } if (over_leafy && !bc) { for (size_t i = 0; i < grp.len; i++) { - size_t c = w_clone_subtree(d, over, grp.data[i], bp); - ShclVecSize_push(t, inb ? &rep[gi] : &appended, c); + size_t pos = grp.data[i]; + size_t c = w_clone_subtree(d, over, okids.data[pos], bp); + if (inb) ShclVecSize_push(t, &rep[gi], c); + else { app_at[pos] = c; nappended++; } } if (inb) { is_rep[gi] = 1; any_rep = 1; } } else { for (size_t i = 0; i < grp.len; i++) { - size_t ok = grp.data[i]; + size_t pos = grp.data[i]; + size_t ok = okids.data[pos]; uint64_t hk = merge_hash(name, &over->nodes.data[ok].value); size_t b = (size_t)-1; for (ShclCMapEnt *e = cmap_first(&by_key, hk); e; e = cmap_next(e, hk)) @@ -3696,11 +3704,11 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { } } if (b != (size_t)-1) { adopt_trivia(d, b, over, ok); w_overlay(d, b, over, ok); } - else ShclVecSize_push(t, &appended, w_clone_subtree(d, over, ok, bp)); + else { app_at[pos] = w_clone_subtree(d, over, ok, bp); nappended++; } } } } - if (!any_rep && appended.len == 0) return; + if (!any_rep && nappended == 0) return; // Rebuild once: each replaced group lands at its name's first original // position (dropped nodes stay in the arena, unreferenced - reads and // emit walk children from the root), appends go at the end. One splice @@ -3723,7 +3731,7 @@ static void w_overlay(shcl_doc *d, size_t bp, const shcl_doc *over, size_t op) { ShclVecSize_push(a, &nw, b); } } - for (size_t k = 0; k < appended.len; k++) ShclVecSize_push(a, &nw, appended.data[k]); + for (size_t k = 0; k < okids.len; k++) if (app_at[k] != (size_t)-1) ShclVecSize_push(a, &nw, app_at[k]); NODE(d, bp).children = nw; } @@ -3734,11 +3742,13 @@ void shcl_merge(shcl_doc *d, const shcl_doc *over) { arena_reset(&d->scratch); // merge temporaries (compare keys, clone lists) die here w_overlay(d, ROOT, over, ROOT); // Layers commonly share a footer; keeping one copy of each keeps a stack - // of files from repeating it once per layer. + // of files from repeating it once per layer. Only the lines already here + // count: a layer's own repeats are its content. + size_t had = d->orphans.len; for (size_t i = 0; i < over->orphans.len; i++) { ShclStr ot = over->orphans.data[i].text; int dup = 0; - for (size_t k = 0; k < d->orphans.len; k++) if (s_eq(d->orphans.data[k].text, ot)) { dup = 1; break; } + for (size_t k = 0; k < had; k++) if (s_eq(d->orphans.data[k].text, ot)) { dup = 1; break; } if (!dup) ShclVecLead_push(a, &d->orphans, lead_make(s_dup(a, ot), over->orphans.data[i].blank_before)); } } diff --git a/source/go/shcl.go b/source/go/shcl.go index 41c9b0f..3b56e3e 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -4304,10 +4304,12 @@ func (d *Document) Merge(over *Document) { d.lost += over.lost d.overlay(root, over, root) // Layers commonly share a footer; keeping one copy of each keeps a - // stack of files from repeating it once per layer. + // stack of files from repeating it once per layer. Only the lines + // already here count: a layer's own repeats are its content. + had := len(d.orphans) for _, o := range over.orphans { seen := false - for _, e := range d.orphans { + for _, e := range d.orphans[:had] { if e.text == o.text { seen = true break @@ -4351,13 +4353,14 @@ func (d *Document) overlay(baseParent int, over *Document, overParent int) { overKids := over.arena[overParent].children // Over side: name -> node bucket, in first-appearance order. var order []string - groups := map[string][]int{} - for _, k := range overKids { + type overKid struct{ pos, node int } + groups := map[string][]overKid{} + for pos, k := range overKids { n := over.arena[k].name if _, seen := groups[n]; !seen { order = append(order, n) } - groups[n] = append(groups[n], k) + groups[n] = append(groups[n], overKid{pos, k}) } // Base side, one pass: does the name have a container instance, and // which child carries each (name, key) - every key computed once. The @@ -4379,15 +4382,16 @@ func (d *Document) overlay(baseParent int, over *Document, overParent int) { // mention, not a leaf, so it falls through to the instance merge: a // bare section header in a higher layer never wipes the subtree below. // Replaced groups splice in the rebuild; everything appended (unmatched - // instances, and replaced names base never had) keeps processing order. + // instances, and replaced names base never had) keeps the over file's + // order, which the per-name pass here would otherwise regroup. replace := map[string][]int{} - var appended []int + var appended []overKid emptyKey := (&value{kind: vEmpty}).key() for _, name := range order { group := groups[name] overLeafy := true for _, k := range group { - if len(over.arena[k].children) > 0 { + if len(over.arena[k.node].children) > 0 { overLeafy = false break } @@ -4396,15 +4400,18 @@ func (d *Document) overlay(baseParent int, over *Document, overParent int) { if overLeafy && !baseContainer { clones := make([]int, len(group)) for i, ok := range group { - clones[i] = d.cloneSubtree(over, ok, baseParent) + clones[i] = d.cloneSubtree(over, ok.node, baseParent) } if inBase { replace[name] = clones } else { - appended = append(appended, clones...) + for i, ok := range group { + appended = append(appended, overKid{ok.pos, clones[i]}) + } } } else { - for _, ok := range group { + for _, k := range group { + ok := k.node okey := over.arena[ok].value.key() target, found := byKey[[2]string{name, okey}] // A raw block in the higher layer fills a same-named empty @@ -4429,7 +4436,7 @@ func (d *Document) overlay(baseParent int, over *Document, overParent int) { d.overlay(target, over, ok) } else { c := d.cloneSubtree(over, ok, baseParent) - appended = append(appended, c) + appended = append(appended, overKid{k.pos, c}) } } } @@ -4453,7 +4460,10 @@ func (d *Document) overlay(baseParent int, over *Document, overParent int) { newKids = append(newKids, b) } } - newKids = append(newKids, appended...) + sort.Slice(appended, func(i, j int) bool { return appended[i].pos < appended[j].pos }) + for _, k := range appended { + newKids = append(newKids, k.node) + } d.arena[baseParent].children = newKids } diff --git a/source/python/shcl.py b/source/python/shcl.py index beda4f4..8e7b888 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -2895,9 +2895,11 @@ def merge(self, over: Document) -> None: self._lost += over._lost self._overlay(ROOT, over, ROOT) # Layers commonly share a footer; keeping one copy of each keeps a - # stack of files from repeating it once per layer. + # stack of files from repeating it once per layer. Only the lines + # already here count: a layer's own repeats are its content. + had = len(self.orphans) for o in over.orphans: - if not any(e.text == o.text for e in self.orphans): + if not any(e.text == o.text for e in self.orphans[:had]): self.orphans.append(_Lead(o.text, o.blank_before)) # One grouping pass over each side, then a single children rebuild: the @@ -2939,14 +2941,14 @@ def _overlay_level(self, base_parent, over, over_parent): # Over side: name -> node bucket, in first-appearance order. order = [] groups: dict = {} - for k in over_kids: + for pos, k in enumerate(over_kids): n = over.arena[k].name g = groups.get(n) if g is None: order.append(n) g = [] groups[n] = g - g.append(k) + g.append((pos, k)) # Base side, one pass: does the name have a container instance, and # which child carries each (name, key) - every key computed once. The # list is copied because the splices below rewrite it as they go. @@ -2963,24 +2965,25 @@ def _overlay_level(self, base_parent, over, over_parent): # mention, not a leaf, so it falls through to the instance merge: a # bare section header in a higher layer never wipes the subtree below. # Replaced groups splice in the rebuild; everything appended (unmatched - # instances, and replaced names base never had) keeps processing order. + # instances, and replaced names base never had) keeps the over file's + # order, which the per-name pass here would otherwise regroup. replace = {} appended = [] pending = [] empty_key = _Value("empty").key() for name in order: group = groups[name] - over_leafy = all(not over.arena[k].children for k in group) + over_leafy = all(not over.arena[k].children for _, k in group) in_base = name in has_container base_container = has_container.get(name, False) if over_leafy and not base_container: - clones = [self._clone_subtree(over, ok, base_parent) for ok in group] + clones = [(pos, self._clone_subtree(over, ok, base_parent)) for pos, ok in group] if in_base: - replace[name] = clones + replace[name] = [c for _, c in clones] else: appended.extend(clones) else: - for ok in group: + for pos, ok in group: okey = over.arena[ok].value.key() b = by_key.get((name, okey)) # A raw block in the higher layer fills a same-named empty @@ -3001,7 +3004,7 @@ def _overlay_level(self, base_parent, over, over_parent): # survives the rebuild below and can wait for it. pending.append((b, ok)) else: - appended.append(self._clone_subtree(over, ok, base_parent)) + appended.append((pos, self._clone_subtree(over, ok, base_parent))) if not replace and not appended: return pending # Rebuild once: each replaced group lands at its name's first original @@ -3017,7 +3020,8 @@ def _overlay_level(self, base_parent, over, over_parent): elif name not in spliced: spliced.add(name) new_kids.extend(clones) - new_kids.extend(appended) + appended.sort(key=lambda pc: pc[0]) + new_kids.extend(c for _, c in appended) self.arena[base_parent].children = new_kids return pending diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 457e517..914f865 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -4046,9 +4046,11 @@ impl Document { self.lost += over.lost; self.overlay(ROOT, over, ROOT); // Layers commonly share a footer; keeping one copy of each keeps a - // stack of files from repeating it once per layer. + // stack of files from repeating it once per layer. Only the lines + // already here count: a layer's own repeats are its content. + let had = self.orphans.len(); for o in &over.orphans { - if !self.orphans.iter().any(|e| e.text == o.text) { + if !self.orphans[..had].iter().any(|e| e.text == o.text) { self.orphans.push(o.clone()); } } @@ -4082,8 +4084,8 @@ impl Document { let over_kids = &over.arena[over_parent].children; // Over side: name -> node bucket, in first-appearance order. let mut order: Vec = Vec::new(); - let mut groups: HashMap> = HashMap::new(); - for &k in over_kids { + let mut groups: HashMap> = HashMap::new(); + for (pos, &k) in over_kids.iter().enumerate() { let n = &over.arena[k].name; groups .entry(n.clone()) @@ -4091,7 +4093,7 @@ impl Document { order.push(n.clone()); Vec::new() }) - .push(k); + .push((pos, k)); } // Base side, one pass: does the name have a container instance, and // which child carries each (name, key) - every key computed once. The @@ -4111,27 +4113,28 @@ impl Document { // mention, not a leaf, so it falls through to the instance merge: a // bare section header in a higher layer never wipes the subtree below. // Replaced groups splice in the rebuild; everything appended (unmatched - // instances, and replaced names base never had) keeps processing order. + // instances, and replaced names base never had) keeps the over file's + // order, which the per-name pass here would otherwise regroup. let mut replace: HashMap> = HashMap::new(); - let mut appended: Vec = Vec::new(); + let mut appended: Vec<(usize, usize)> = Vec::new(); let empty_key = Value::Empty.key(); for name in &order { let group = &groups[name]; - let over_leafy = group.iter().all(|&k| over.arena[k].children.is_empty()); + let over_leafy = group.iter().all(|&(_, k)| over.arena[k].children.is_empty()); let in_base = has_container.contains_key(name); let base_container = has_container.get(name).copied().unwrap_or(false); if over_leafy && !base_container { - let clones: Vec = group + let clones: Vec<(usize, usize)> = group .iter() - .map(|&ok| self.clone_subtree(over, ok, base_parent)) + .map(|&(pos, ok)| (pos, self.clone_subtree(over, ok, base_parent))) .collect(); if in_base { - replace.insert(name.clone(), clones); + replace.insert(name.clone(), clones.into_iter().map(|(_, c)| c).collect()); } else { appended.extend(clones); } } else { - for &ok in group { + for &(pos, ok) in group { let okey = over.arena[ok].value.key(); // A raw block in the higher layer fills a same-named empty // binding below, exactly as a fence line fills one inside a @@ -4156,7 +4159,7 @@ impl Document { } None => { let c = self.clone_subtree(over, ok, base_parent); - appended.push(c); + appended.push((pos, c)); } } } @@ -4181,7 +4184,8 @@ impl Document { None => newkids.push(b), } } - newkids.extend(appended.iter().copied()); + appended.sort_by_key(|&(pos, _)| pos); + newkids.extend(appended.iter().map(|&(_, c)| c)); self.arena[base_parent].children = newkids; } diff --git a/source/rust/tests/fuzz_smoke.rs b/source/rust/tests/fuzz_smoke.rs index 52d22fb..0783094 100644 --- a/source/rust/tests/fuzz_smoke.rs +++ b/source/rust/tests/fuzz_smoke.rs @@ -267,6 +267,16 @@ fn merge_never_panics_and_stays_fixpoint() { "merged output not idempotent at iteration {} for:\nA:\n{}\nB:\n{}", i, a, b ); + // Onto an empty base a merge is the identity: nothing to match, so + // every node and every footer line comes across in file order. + let mut empty = Document::new(); + empty.merge(&Document::parse(&b)); + assert_eq!( + empty.to_canonical(), + Document::parse(&b).to_canonical(), + "merge onto empty base is not the identity at iteration {} for:\n{}", + i, b + ); } } From 7b13183265568dad9f4899e6bbeb1ea883b2a35e Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:47:39 -0700 Subject: [PATCH 06/19] band the did-you-mean distance --- cicd/utility/perf-gate.bash | 35 ++++++++++++++++------ project/backlog.md | 5 +++- source/c/shcl.h | 28 +++++++++++++----- source/go/shcl.go | 59 ++++++++++++++++++++++++++++++++----- source/python/shcl.py | 34 ++++++++++++++++----- source/rust/src/lib.rs | 37 ++++++++++++++++++----- 6 files changed, 157 insertions(+), 41 deletions(-) diff --git a/cicd/utility/perf-gate.bash b/cicd/utility/perf-gate.bash index c02522d..0d382af 100755 --- a/cicd/utility/perf-gate.bash +++ b/cicd/utility/perf-gate.bash @@ -5,13 +5,15 @@ ## Two rounds running, a fix that was correct made bulk writes 4.5x slower ## and absent-path defaults 140x slower, and both reached dev because ## nothing had a number to fail on. A third round found a plain text file -## parsing in quadratic time. Each workload is timed against the same +## parsing in quadratic time, and a fourth the did-you-mean suggestion +## quadratic in name length. Each workload is timed against the same ## binding's parse-only baseline on the same machine, so the gate carries ## no wall-clock constant and does not care how fast the runner is: -## applying the ops, or refusing every line, must stay small beside -## reading a well-formed document of the same size. A per-op index -## rebuild, a scan of every sibling, or a rewalk of the retained lines -## breaks that ratio by more than an order of magnitude. +## applying the ops, refusing every line, or suggesting a name for every +## unknown field, must stay small beside reading a well-formed document +## of the same size. A per-op index rebuild, a scan of every sibling, a +## rewalk of the retained lines, or a full edit-distance table per name +## pair breaks that ratio by more than an order of magnitude. ## Syntax: ## perf-gate.bash [--keys N] [--factor F] NAME|CLI [NAME|CLI ...] ## --keys N flat keys in the generated document (default 40000) @@ -59,16 +61,27 @@ awk -v n="${keys}" 'BEGIN{ for (i = 0; i < n; i++) printf "int-default\tk%d\t0\n ## retained as trivia, and the retained list must not be rewalked per line. badDoc="${tmpDir}/bad.shcl" awk -v n="${keys}" 'BEGIN{ for (i = 0; i < n; i++) print "no colon here" }' > "${badDoc}" +## Unknown fields with long names against a schema of long names, all one +## length so the length prefilter rejects nothing: every pair must cost time +## linear in the length, not the full table. 30 x 30 pairs at 800 characters +## is 2 s to 90 s across the bindings with a full table and well under their +## baselines with a banded one. +sugSchema="${tmpDir}/sug-schema.shcl"; sugDoc="${tmpDir}/sug.shcl" +awk 'BEGIN{ pad = sprintf("%794s", ""); gsub(/ /, "x", pad); for (i = 0; i < 30; i++) printf "field: s%05d%s\n", i, pad }' > "${sugSchema}" +awk 'BEGIN{ pad = sprintf("%794s", ""); gsub(/ /, "x", pad); for (i = 0; i < 30; i++) printf "u%05d%s: 1\n", i, pad }' > "${sugDoc}" -## Milliseconds for one run of $2 (an ops file, or a document when $3 is -## "check") through CLI $1, best of two so a scheduling hiccup does not fail -## the gate. +## Milliseconds for one run of $2 (an ops file, a document when $3 is +## "check", or a document validated against ${sugSchema} when $3 is +## "suggest") through CLI $1, best of two so a scheduling hiccup does not +## fail the gate. fTimeMs(){ local cli="$1" input="$2" mode="${3:-set}" best=0 ms start end for _ in 1 2; do start="$(date +%s%N)" if [[ "${mode}" == check ]]; then "${cli}" check "${input}" > /dev/null 2>&1 || true + elif [[ "${mode}" == suggest ]]; then + "${cli}" check --schema "${sugSchema}" "${input}" > /dev/null 2>&1 || true else "${cli}" set "${doc}" < "${input}" > /dev/null 2>&1 || true fi @@ -88,9 +101,11 @@ for b in "${bindings[@]}"; do budget=$(( baseMs * factor )) floor=$(( baseMs + 250 )) if ((budget < floor)); then budget="${floor}"; fi - for w in writes defaults reads badlines; do + for w in writes defaults reads badlines suggest; do if [[ "${w}" == badlines ]]; then ms="$(fTimeMs "${cli}" "${badDoc}" check)" + elif [[ "${w}" == suggest ]]; then + ms="$(fTimeMs "${cli}" "${sugDoc}" suggest)" else ms="$(fTimeMs "${cli}" "${tmpDir}/${w}.ops")" fi @@ -114,3 +129,5 @@ echo "perf-gate: OK: ${keys} keys, ${#bindings[@]} binding(s) within ${factor}x ## in consecutive rounds with no numeric gate to catch them. ## 2026-09-01 badlines workload: a document of refused lines, which went ## quadratic through the retained-trivia list. +## 2026-09-02 suggest workload: long unknown names against long schema names, +## which cost a full edit-distance table per pair. diff --git a/project/backlog.md b/project/backlog.md index ed44bdd..135bc24 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -102,10 +102,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190600 - Closed: 20260902-104500 - - 🔘 Item 8: the did-you-mean suggestion is quadratic in name length, so a check against a schema with long field names takes seconds to minutes. + - ✅ Item 8: the did-you-mean suggestion is quadratic in name length, so a check against a schema with long field names takes seconds to minutes. - Measured: 300 unknown fields of 300 characters against 300 schema names of 300 characters, 188 KB in all, takes 23 s in the release reference, 27 s in Go, 10 s in C. Double both lengths and the reference did not finish in two minutes. - Cause: a full Levenshtein table per pair with no prefilter, while the result is discarded past distance 2. A length difference over 2 can be rejected outright, and a banded table bounded by the threshold makes the rest linear. + - Fixed: the edit distance takes the cap (2): a length gap past it returns at once, only the band of cells within the cap is computed, and a row whose minimum passes the cap ends the pair. The 188 KB repro went from 23 s to 0.06 s in the reference; the 2000-character shape that did not finish in two minutes takes 0.04 s. The banded result equals the full table for every distance at or under the cap (checked over 200k random pairs at four caps), so no suggestion changes. + - Pinned by `perf-gate.bash`'s new `suggest` workload: 30 unknown 800-character names against 30 schema names of the same length, timed against the binding's parse baseline. Over budget on the old code in every binding (17 s, 1.9 s, 86 s, 0.66 s for rust, go, python, c) and 10 to 370 ms now. - Opened: 20260901-190700 + - Closed: 20260902-113000 - 🔘 Item 9: C formats an exact power of two with 17 digits where the other three print 16. - Reproduced, C only: 46 of the 2098 powers of two, none of 11,206 random doubles. `get --float` and a float write op both show it, so it reaches a saved file. diff --git a/source/c/shcl.h b/source/c/shcl.h index b41253e..1dfc9a3 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -4573,23 +4573,37 @@ static void v_build_schema(ShclArena *a, shcl_doc *schema, ShclVSchemaDef *def, } } -// Two-row Levenshtein over codepoints; powers the "did you mean" prose (never -// the code). -static size_t v_edit_distance(ShclArena *a, ShclStr sa, ShclStr sb) { +// Levenshtein distance over codepoints capped at cap, for the "did you mean" +// prose (never the code): anything past the cap comes back as cap + 1. Only +// the band |i - j| <= cap of the table is computed, so a pair costs linear +// time in the names' length, and a length gap past the cap needs no table at +// all. +static size_t v_edit_distance(ShclArena *a, ShclStr sa, ShclStr sb, size_t cap) { ShclCPs ca = decode_cps(a, sa); ShclCPs cb = decode_cps(a, sb); + size_t inf = cap + 1; + if ((ca.n > cb.n ? ca.n - cb.n : cb.n - ca.n) > cap) return inf; size_t *prev = (size_t *)arena_alloc(a, (cb.n + 1) * sizeof(size_t)); size_t *cur = (size_t *)arena_alloc(a, (cb.n + 1) * sizeof(size_t)); - for (size_t j = 0; j <= cb.n; j++) prev[j] = j; + for (size_t j = 0; j <= cb.n; j++) { prev[j] = j < inf ? j : inf; cur[j] = inf; } for (size_t i = 1; i <= ca.n; i++) { - cur[0] = i; - for (size_t j = 1; j <= cb.n; j++) { + cur[0] = i < inf ? i : inf; + size_t lo = i > cap ? i - cap : 1; + size_t hi = i + cap < cb.n ? i + cap : cb.n; + if (lo > 1) cur[lo - 1] = inf; + size_t row_min = cur[0]; + for (size_t j = lo; j <= hi; j++) { size_t cost = ca.cp[i - 1] == cb.cp[j - 1] ? 0 : 1; size_t m = prev[j] + 1; if (cur[j - 1] + 1 < m) m = cur[j - 1] + 1; if (prev[j - 1] + cost < m) m = prev[j - 1] + cost; + if (m > inf) m = inf; cur[j] = m; + if (m < row_min) row_min = m; } + if (hi < cb.n) cur[hi + 1] = inf; + // No cell in a later row can come back under this row's minimum. + if (row_min > cap) return inf; size_t *t = prev; prev = cur; cur = t; } return prev[cb.n]; @@ -4606,7 +4620,7 @@ static void v_suggest(ShclArena *a, ShclArena *tmp, const ShclVecS *names, ShclS if (!names) return; int have = 0; size_t best_dist = 0; ShclStr best_name = s_empty(); for (size_t i = 0; i < names->len; i++) { - size_t dist = v_edit_distance(tmp, name, names->data[i]); + size_t dist = v_edit_distance(tmp, name, names->data[i], 2); if (dist <= 2 && (!have || dist < best_dist)) { have = 1; best_dist = dist; best_name = names->data[i]; } } if (have) { diff --git a/source/go/shcl.go b/source/go/shcl.go index 3b56e3e..b2d861c 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -6381,30 +6381,73 @@ func expandMounts(def *schemaDef) ([]constraint, [][2]string) { return out, cuts } -// editDistance is two-row Levenshtein; powers the "did you mean" prose (never -// the code). -func editDistance(a, b string) int { +// editDistance is the Levenshtein distance capped at cap, for the "did you +// mean" prose (never the code): anything past the cap comes back as cap + 1. +// Only the band |i - j| <= cap of the table is computed, so a pair costs +// linear time in the names' length, and a length gap past the cap needs no +// table at all. +func editDistance(a, b string, cap int) int { ar := []rune(a) br := []rune(b) + inf := cap + 1 + if absDiff(len(ar), len(br)) > cap { + return inf + } prev := make([]int, len(br)+1) cur := make([]int, len(br)+1) for j := range prev { - prev[j] = j + prev[j] = minInt(j, inf) + cur[j] = inf } for i := 1; i <= len(ar); i++ { - cur[0] = i - for j := 1; j <= len(br); j++ { + cur[0] = minInt(i, inf) + lo := maxInt(i-cap, 1) + hi := minInt(i+cap, len(br)) + if lo > 1 { + cur[lo-1] = inf + } + rowMin := cur[0] + for j := lo; j <= hi; j++ { cost := 1 if ar[i-1] == br[j-1] { cost = 0 } - cur[j] = min3(prev[j]+1, cur[j-1]+1, prev[j-1]+cost) + cur[j] = minInt(min3(prev[j]+1, cur[j-1]+1, prev[j-1]+cost), inf) + rowMin = minInt(rowMin, cur[j]) + } + if hi < len(br) { + cur[hi+1] = inf + } + // No cell in a later row can come back under this row's minimum. + if rowMin > cap { + return inf } prev, cur = cur, prev } return prev[len(br)] } +func absDiff(a, b int) int { + if a > b { + return a - b + } + return b - a +} + +func minInt(a, b int) int { + if a < b { + return a + } + return b +} + +func maxInt(a, b int) int { + if a > b { + return a + } + return b +} + func min3(a, b, c int) int { if b < a { a = b @@ -6941,7 +6984,7 @@ func vSuggest(siblings map[string][]string, parentChain, name string) string { bestDist := -1 bestName := "" for _, s := range siblings[parentChain] { - dist := editDistance(name, s) + dist := editDistance(name, s, 2) if dist <= 2 && (bestDist < 0 || dist < bestDist) { bestDist = dist bestName = s diff --git a/source/python/shcl.py b/source/python/shcl.py index 8e7b888..e18515e 100644 --- a/source/python/shcl.py +++ b/source/python/shcl.py @@ -5108,15 +5108,33 @@ def _expand_mounts(sdef): return out, cuts -def _edit_distance(a, b): - # Two-row Levenshtein; powers the "did you mean" prose (never the code). - prev = list(range(len(b) + 1)) - cur = [0] * (len(b) + 1) +def _edit_distance(a, b, cap): + # Levenshtein distance capped at `cap`, for the "did you mean" prose (never + # the code): anything past the cap comes back as cap + 1. Only the band + # |i - j| <= cap of the table is computed, so a pair costs linear time in + # the names' length, and a length gap past the cap needs no table at all. + inf = cap + 1 + if abs(len(a) - len(b)) > cap: + return inf + prev = [min(j, inf) for j in range(len(b) + 1)] + cur = [inf] * (len(b) + 1) for i in range(1, len(a) + 1): - cur[0] = i - for j in range(1, len(b) + 1): + cur[0] = min(i, inf) + lo = max(i - cap, 1) + hi = min(i + cap, len(b)) + if lo > 1: + cur[lo - 1] = inf + row_min = cur[0] + for j in range(lo, hi + 1): cost = 0 if a[i - 1] == b[j - 1] else 1 - cur[j] = min(prev[j] + 1, cur[j - 1] + 1, prev[j - 1] + cost) + cur[j] = min(prev[j] + 1, cur[j - 1] + 1, prev[j - 1] + cost, inf) + if cur[j] < row_min: + row_min = cur[j] + if hi < len(b): + cur[hi + 1] = inf + # No cell in a later row can come back under this row's minimum. + if row_min > cap: + return inf prev, cur = cur, prev return prev[len(b)] @@ -5195,7 +5213,7 @@ def _v_suggest(siblings, parent_chain, name): contract. The sibling lists are prebuilt once per validate.""" best = None for s in siblings.get(parent_chain, ()): - dist = _edit_distance(name, s) + dist = _edit_distance(name, s, 2) if dist <= 2 and (best is None or dist < best[0]): best = (dist, s) if best is None: diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 914f865..95c9542 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -5996,17 +5996,38 @@ fn expand_mounts(def: &SchemaDef) -> (Vec, Vec<(String, String)>) { (out, cuts) } -/// Two-row Levenshtein; powers the "did you mean" prose (never the code). -fn edit_distance(a: &str, b: &str) -> usize { +/// Levenshtein distance capped at `cap`, for the "did you mean" prose (never +/// the code): anything past the cap comes back as `cap + 1`. Only the band +/// `|i - j| <= cap` of the table is computed, so a pair costs linear time in +/// the names' length, and a length gap past the cap needs no table at all. +fn edit_distance(a: &str, b: &str, cap: usize) -> usize { let a: Vec = a.chars().collect(); let b: Vec = b.chars().collect(); - let mut prev: Vec = (0..=b.len()).collect(); - let mut cur = vec![0usize; b.len() + 1]; + let inf = cap + 1; + if a.len().abs_diff(b.len()) > cap { + return inf; + } + let mut prev: Vec = (0..=b.len()).map(|j| j.min(inf)).collect(); + let mut cur = vec![inf; b.len() + 1]; for i in 1..=a.len() { - cur[0] = i; - for j in 1..=b.len() { + cur[0] = i.min(inf); + let lo = i.saturating_sub(cap).max(1); + let hi = (i + cap).min(b.len()); + if lo > 1 { + cur[lo - 1] = inf; + } + let mut row_min = cur[0]; + for j in lo..=hi { let cost = if a[i - 1] == b[j - 1] { 0 } else { 1 }; - cur[j] = (prev[j] + 1).min(cur[j - 1] + 1).min(prev[j - 1] + cost); + cur[j] = (prev[j] + 1).min(cur[j - 1] + 1).min(prev[j - 1] + cost).min(inf); + row_min = row_min.min(cur[j]); + } + if hi < b.len() { + cur[hi + 1] = inf; + } + // No cell in a later row can come back under this row's minimum. + if row_min > cap { + return inf; } std::mem::swap(&mut prev, &mut cur); } @@ -6519,7 +6540,7 @@ fn v_suggest(siblings: &HashMap>, parent_chain: &str, name: let mut best: Option<(usize, &str)> = None; if let Some(names) = siblings.get(parent_chain) { for s in names { - let dist = edit_distance(name, s); + let dist = edit_distance(name, s, 2); if dist <= 2 && best.is_none_or(|(bd, _)| dist < bd) { best = Some((dist, s.as_str())); } From 2dfd6e40df9efcea1ece6a129becc557f4b625a4 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:52:55 -0700 Subject: [PATCH 07/19] float spelling agrees --- cicd/utility/crosscheck.bash | 15 +++ project/backlog.md | 10 +- .../080-float-shortest/expected-diags.txt | 1 + .../080-float-shortest/expected-write.shcl | 111 ++++++++++++++++++ .../080-float-shortest/expected.shcl | 1 + .../conformance/080-float-shortest/input.shcl | 1 + .../conformance/080-float-shortest/reads.tsv | 3 + .../conformance/080-float-shortest/write.ops | 55 +++++++++ project/conformance/README.md | 2 + source/c/shcl.h | 32 ++++- source/rust/src/lib.rs | 42 ++++++- 11 files changed, 268 insertions(+), 5 deletions(-) create mode 100644 project/conformance/080-float-shortest/expected-diags.txt create mode 100644 project/conformance/080-float-shortest/expected-write.shcl create mode 100644 project/conformance/080-float-shortest/expected.shcl create mode 100644 project/conformance/080-float-shortest/input.shcl create mode 100644 project/conformance/080-float-shortest/reads.tsv create mode 100644 project/conformance/080-float-shortest/write.ops diff --git a/cicd/utility/crosscheck.bash b/cicd/utility/crosscheck.bash index b6d58e9..90b4c07 100755 --- a/cicd/utility/crosscheck.bash +++ b/cicd/utility/crosscheck.bash @@ -377,6 +377,21 @@ fCompare "fmt --write rejects --set" fmt --write --set a=1 missing.shcl fCompareWrite "set --write creates a missing file" fFixAbsent set --write --set a=1 fCompareWrite "fmt --write still refuses a missing file" fFixAbsent fmt --write +# Float spelling: shortest-round-trip formatters may lawfully differ at a +# power of two (a lopsided rounding interval) and on an exact tie between two +# spellings of the shortest length. Every power of two, plus a fixed set of +# 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); + 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; + } +}' > "${tmpDir}/floats.ops" +fCompareStdin "float spelling" "${tmpDir}/floats.ops" set - + # `set -` follows stdin, so the same spelling means two things and both are # pinned: the piped document when an option holds the edits, an empty base when # stdin is the ops script instead. diff --git a/project/backlog.md b/project/backlog.md index 135bc24..fe40aff 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -110,15 +110,21 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190700 - Closed: 20260902-113000 - - 🔘 Item 9: C formats an exact power of two with 17 digits where the other three print 16. + - ✅ Item 9: C formats an exact power of two with 17 digits where the other three print 16. - Reproduced, C only: 46 of the 2098 powers of two, none of 11,206 random doubles. `get --float` and a float write op both show it, so it reaches a saved file. - Cause: the shortest-round-trip loop tries only the correctly rounded string at each precision. At a power of two the rounding interval is lopsided and the neighbor one digit up is the one that round-trips. + - Fixed: at each precision the C formatter also tries the spelling one last digit up and one down before adding a digit, which is the neighbor a shortest-digits algorithm picks when the closest spelling falls outside a lopsided interval. All 2098 powers of two and 208k doubles now spell the same in all four. + - Pinned by corpus `080` (the 46 powers of two plus the ties) and a `float spelling` dimension in `crosscheck.bash` over every power of two and 3000 fixed random doubles. Both diverged on the old C and agree now. - Opened: 20260901-190800 + - Closed: 20260902-121500 - - 🔘 Item 10: on a value exactly halfway at 17 digits, the reference rounds up and the other three round half-even. + - ✅ Item 10: on a value exactly halfway at 17 digits, the reference rounds up and the other three round half-even. - Reproduced: `2.9802322387695312e-08` written back is `...313` from Rust and `...312` from Go, Python and C. Two hits in 11,206 random doubles; both spellings parse to the same double. - Needs a decision on which side to match. Matching the reference means each port detects the tie and bumps the digit; matching the three means the reference post-processes its own formatter. + - Fixed: decided for round-half-even: it is IEEE's own tie rule, what three of the four did already, and what Python's `repr` and Go's `strconv` print, so a value read from another tool's output spells the same here. The reference now takes the correctly rounded spelling of the shortest length whenever it reads back (core rounds that to even), and keeps its shortest spelling only when it does not (the lopsided power-of-two case, where every binding has one choice). Ties turned out to occur at any length, not only 17 digits: `811212085039910.25` is one at 16. + - Pinned by corpus `080` and the crosscheck float dimension; the old reference diverged on nine of the corpus values. - Opened: 20260901-190900 + - Closed: 20260902-121500 - 🔘 Item 11: the distributed CLI aborts on Windows when the program reading its output closes early. - Reproduced under wine: `fmt` of a 40k-key file piped through `head` panics with "failed printing to stdout: Pipe not connected", and the release build turns the panic into an abort. Go and C exit 0 silently there, and all three are silent on Linux. diff --git a/project/conformance/080-float-shortest/expected-diags.txt b/project/conformance/080-float-shortest/expected-diags.txt new file mode 100644 index 0000000..4e7ad2e --- /dev/null +++ b/project/conformance/080-float-shortest/expected-diags.txt @@ -0,0 +1 @@ +ok (0 diagnostic(s)) diff --git a/project/conformance/080-float-shortest/expected-write.shcl b/project/conformance/080-float-shortest/expected-write.shcl new file mode 100644 index 0000000..cde4113 --- /dev/null +++ b/project/conformance/080-float-shortest/expected-write.shcl @@ -0,0 +1,111 @@ +x: 1 + +f0: -1929377421321310.2 + +f1: -0.000000029802322387695312 + +f2: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007120236347223045 + +f3: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007291122019556398 + +f4: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008209073602596753 + +f5: 0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005641232424577593 + +f6: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005858190679279809 + +f7: 0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007678447687145631 + +f8: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006142758149716505 + +f9: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006290184345309701 + +f10: 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005940911144672375 + +f11: 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006083493012144512 + +f12: 0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005225680706521042 + +f13: 0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005351097043477547 + +f14: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005426657103235053 + +f15: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005966672584960166 + +f16: 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004887898181599368 + +f17: 0.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006256509672447191 + +f18: 0.00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000005075883674631299 + +f19: 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006653062250012736 + +f20: 0.000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007854549544476363 + +f21: 0.0000000000000000000000000000000000000000007174648137343064 + +f22: 0.000000000000000000000000000006310887241768095 + +f23: 0.000000000000000000000006617444900424222 + +f24: 0.00000000000005684341886080802 + +f25: 0.000000029802322387695312 + +f26: 0.000000029802322387695312 + +f27: 0.00000005960464477539063 + +f28: 811212085039910.2 + +f29: 1125899906842624.2 + +f30: 1650374396507224.2 + +f31: 1860728232985249.2 + +f32: 2044818889941875.2 + +f33: 618970019642690200000000000 + +f34: 5316911983139664000000000000000000000 + +f35: 5444517870735016000000000000000000000000 + +f36: 5986310706507379000000000000000000000000000000000000 + +f37: 6129982163463556000000000000000000000000000000000000000 + +f38: 60708402882054040000000000000000000000000000000000000000000000000000000000000000000 + +f39: 65185151242703560000000000000000000000000000000000000000000000000000000000000000000000000000 + +f40: 71671831749689740000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f41: 615656346818663800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f42: 645562469521727200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f43: 82631996098781080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f44: 6243497100631985000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f45: 56236422431789960000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f46: 57586096570152920000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f47: 58968162887836590000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f48: 61832600368276140000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f49: 64836180763765520000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f50: 5386379163185535000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f51: 53344115463038840000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f52: 61501577861568110000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f53: 528294531135665300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 + +f54: 638668899051110400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 diff --git a/project/conformance/080-float-shortest/expected.shcl b/project/conformance/080-float-shortest/expected.shcl new file mode 100644 index 0000000..d508cf7 --- /dev/null +++ b/project/conformance/080-float-shortest/expected.shcl @@ -0,0 +1 @@ +x: 1 diff --git a/project/conformance/080-float-shortest/input.shcl b/project/conformance/080-float-shortest/input.shcl new file mode 100644 index 0000000..d508cf7 --- /dev/null +++ b/project/conformance/080-float-shortest/input.shcl @@ -0,0 +1 @@ +x: 1 diff --git a/project/conformance/080-float-shortest/reads.tsv b/project/conformance/080-float-shortest/reads.tsv new file mode 100644 index 0000000..136bbd9 --- /dev/null +++ b/project/conformance/080-float-shortest/reads.tsv @@ -0,0 +1,3 @@ +query type expected status +- load ok - +x int 1 Good diff --git a/project/conformance/080-float-shortest/write.ops b/project/conformance/080-float-shortest/write.ops new file mode 100644 index 0000000..6129f9b --- /dev/null +++ b/project/conformance/080-float-shortest/write.ops @@ -0,0 +1,55 @@ +float f0 -1929377421321310.2 +float f1 -2.9802322387695312e-08 +float f2 7.120236347223045e-307 +float f3 7.291122019556398e-304 +float f4 8.209073602596753e-289 +float f5 5.641232424577593e-278 +float f6 5.858190679279809e-244 +float f7 7.678447687145631e-239 +float f8 6.142758149716505e-238 +float f9 6.290184345309701e-235 +float f10 5.940911144672375e-213 +float f11 6.083493012144512e-210 +float f12 5.225680706521042e-200 +float f13 5.351097043477547e-197 +float f14 5.426657103235053e-166 +float f15 5.966672584960166e-154 +float f16 4.887898181599368e-150 +float f17 6.256509672447191e-148 +float f18 5.075883674631299e-116 +float f19 6.653062250012736e-111 +float f20 7.854549544476363e-90 +float f21 7.174648137343064e-43 +float f22 6.310887241768095e-30 +float f23 6.617444900424222e-24 +float f24 5.684341886080802e-14 +float f25 2.9802322387695312e-08 +float f26 2.9802322387695312e-08 +float f27 5.960464477539063e-08 +float f28 811212085039910.2 +float f29 1125899906842624.2 +float f30 1650374396507224.2 +float f31 1860728232985249.2 +float f32 2044818889941875.2 +float f33 6.189700196426902e+26 +float f34 5.316911983139664e+36 +float f35 5.444517870735016e+39 +float f36 5.986310706507379e+51 +float f37 6.129982163463556e+54 +float f38 6.070840288205404e+82 +float f39 6.518515124270356e+91 +float f40 7.167183174968974e+103 +float f41 6.156563468186638e+113 +float f42 6.455624695217272e+119 +float f43 8.263199609878108e+121 +float f44 6.243497100631985e+144 +float f45 5.623642243178996e+160 +float f46 5.758609657015292e+163 +float f47 5.896816288783659e+166 +float f48 6.183260036827614e+172 +float f49 6.483618076376552e+178 +float f50 5.386379163185535e+213 +float f51 5.334411546303884e+241 +float f52 6.150157786156811e+259 +float f53 5.282945311356653e+269 +float f54 6.386688990511104e+293 diff --git a/project/conformance/README.md b/project/conformance/README.md index aa94fc4..495122c 100644 --- a/project/conformance/README.md +++ b/project/conformance/README.md @@ -139,6 +139,8 @@ Case `078` pins what a schema disavows: a `repeat` above 1 drops the `H001` hint Case `079` pins the order a merge appends in: unmatched higher-layer nodes keep that file's order (`c, a, b, a`) rather than regrouping by name, and a footer line the layer repeats itself is kept twice, while one the base already carries is carried over once. Merging onto an empty base is the identity. +Case `080` pins float spelling on the values where shortest-round-trip formatters are allowed to differ: powers of two, whose rounding interval is lopsided so the closest short spelling does not read back and the neighbor does, and exact ties between two spellings of the shortest length, which round to even. Every binding writes the same digits. + Case `045` pins comment depth under childless headers: a header whose children are all commented keeps them indented under it (top-level, nested, and at end of file), while a commented line trailing a live child keeps the existing trails-the-binding placement. Case `044` pins the value-syntax setter: an array, a single element, a quoted element keeping its internal comma, trimming, an unquoted `#` ending the value, an empty value, and the only-if-absent form both skipping an existing path and creating a new one. Its `write-bad.ops` pins the two rejections - a value opening a quote it never closes (the same text the parser reports `E017` for) and a wildcard path. diff --git a/source/c/shcl.h b/source/c/shcl.h index 1dfc9a3..756492a 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -4117,12 +4117,40 @@ shcl_str shcl_to_canonical(shcl_doc *d) { // --- format helpers + remaining public API ---------------------------------- +// The correctly rounded string at a precision is the closest one, but at a +// power of two the rounding interval is lopsided, and the neighbor one digit +// up or down can read back while the closest does not. The shortest-digits +// algorithms the other bindings use find it; stepping the last digit of the +// "%.*e" text in tmp by delta (with carry) and reading it back does the same. +// A carry past the leading digit is a shorter spelling, already tried. +static int f64_neighbor(const char *tmp, double v, int delta, char *out) { + strcpy(out, tmp); + char *e = strchr(out, 'e'); + if (!e || e == out) return 0; + char *p = e - 1; + for (;;) { + if (*p >= '0' && *p <= '9') { + int d = *p - '0' + delta; + if (d >= 0 && d <= 9) { *p = (char)('0' + d); break; } + *p = (char)(d < 0 ? '9' : '0'); + } + if (p == out) return 0; + p--; + } + if (*(out[0] == '-' ? out + 1 : out) == '0') return 0; + return strtod(out, NULL) == v; +} + size_t shcl_format_f64(double v, char *out) { if (isnan(v)) { memcpy(out, "NaN", 3); return 3; } if (isinf(v)) { if (v < 0) { memcpy(out, "-inf", 4); return 4; } memcpy(out, "inf", 3); return 3; } if (v == 0.0) { if (signbit(v)) { memcpy(out, "-0", 2); return 2; } out[0] = '0'; return 1; } - char tmp[64]; int prec; - for (prec = 1; prec <= 17; prec++) { snprintf(tmp, sizeof tmp, "%.*e", prec - 1, v); if (strtod(tmp, NULL) == v) break; } + char tmp[64], alt[64]; int prec; + for (prec = 1; prec <= 17; prec++) { + snprintf(tmp, sizeof tmp, "%.*e", prec - 1, v); + if (strtod(tmp, NULL) == v) break; + if (f64_neighbor(tmp, v, 1, alt) || f64_neighbor(tmp, v, -1, alt)) { memcpy(tmp, alt, sizeof tmp); break; } + } // The round-trip above needed tmp in the host locale; the scan below wants '.'. { const char *dp = dec_point(); size_t dn = strlen(dp); diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index 95c9542..f2ec47e 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -2747,7 +2747,47 @@ fn emit_name(name: &str) -> String { /// text by hand should not have to know which of the four they are reading. #[must_use] pub fn format_f64(v: f64) -> String { - format!("{v}") + let s = format!("{v}"); + if !v.is_finite() || v == 0.0 { + return s; + } + // The shortest spelling that reads back is the same in every binding + // except on an exact tie between two spellings of that length, where core + // rounds away from zero and Go, Python and C round to even. The correctly + // rounded spelling of the same length is the to-even one, so use it when + // it reads back; when it does not (a lopsided interval at a power of + // two), the shortest one is the only choice and all four agree already. + let sig = s + .trim_start_matches('-') + .replace('.', "") + .trim_start_matches('0') + .trim_end_matches('0') + .len() + .max(1); + let e = format!("{v:.*e}", sig - 1); + if e.parse::() != Ok(v) { + return s; + } + let (mant, exp) = e.split_once('e').unwrap_or((&e, "0")); + let digits: String = mant.chars().filter(char::is_ascii_digit).collect(); + let point = exp.parse::().unwrap_or(0) + 1; + let mut out = String::new(); + if v.is_sign_negative() { + out.push('-'); + } + if point <= 0 { + out.push_str("0."); + out.extend(std::iter::repeat_n('0', (-point) as usize)); + out.push_str(&digits); + } else if point as usize >= digits.len() { + out.push_str(&digits); + out.extend(std::iter::repeat_n('0', point as usize - digits.len())); + } else { + out.push_str(&digits[..point as usize]); + out.push('.'); + out.push_str(&digits[point as usize..]); + } + out } /// Quote one path segment so it can be spliced into a lookup path: a bare name From b44e39cbcbde9e1f4f012ffd948174d8dc63e8c4 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:54:57 -0700 Subject: [PATCH 08/19] quiet broken pipe on windows --- project/backlog.md | 5 ++- source/rust/src/lib.rs | 9 ++++- source/rust/src/main.rs | 66 +++++++++++++++++++++++---------- source/rust/tests/cli_pipe.rs | 55 +++++++++++++++++++++++++++ source/rust/tests/fuzz_smoke.rs | 3 +- 5 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 source/rust/tests/cli_pipe.rs diff --git a/project/backlog.md b/project/backlog.md index fe40aff..3fbd0d5 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -126,10 +126,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-190900 - Closed: 20260902-121500 - - 🔘 Item 11: the distributed CLI aborts on Windows when the program reading its output closes early. + - ✅ Item 11: the distributed CLI aborts on Windows when the program reading its output closes early. - Reproduced under wine: `fmt` of a 40k-key file piped through `head` panics with "failed printing to stdout: Pipe not connected", and the release build turns the panic into an abort. Go and C exit 0 silently there, and all three are silent on Linux. - Cause: the broken-pipe handling restores the default signal action and is compiled only on unix. Windows has no SIGPIPE; the write returns an error and the next print panics. Piping into `more` or `Select-Object -First` is ordinary use. + - Fixed: every stdout write in the CLI goes through one pair of macros that exit 0 quietly when the write fails, which is what Go and C do on windows. unix is unchanged: the SIGPIPE default is still restored and the signal ends the process before the error branch is reached. + - Pinned by `tests/cli_pipe.rs`, which runs the built CLI with a reader that closes after one byte and requires an empty stderr and a clean exit (SIGPIPE or 0 on unix, 0 elsewhere). It runs on the hosted windows job; on the windows build it fails on the old code with the panic text on stderr and passes now. - Opened: 20260901-191000 + - Closed: 20260902-124500 - 🔘 Item 12: the C CLI on Windows takes its arguments in the active code page and checks them as UTF-8, so a non-ASCII path is refused, and a name the code page best-fits maps to a different file, including for `--write`. - Reproduced under wine with code page 1252. `cafÃĐ.shcl` is refused as bad encoding. With `ā.shcl` and `a.shcl` both present, `set --write` on the first exits 0 and rewrites the second. diff --git a/source/rust/src/lib.rs b/source/rust/src/lib.rs index f2ec47e..d751047 100644 --- a/source/rust/src/lib.rs +++ b/source/rust/src/lib.rs @@ -4160,7 +4160,9 @@ impl Document { let empty_key = Value::Empty.key(); for name in &order { let group = &groups[name]; - let over_leafy = group.iter().all(|&(_, k)| over.arena[k].children.is_empty()); + let over_leafy = group + .iter() + .all(|&(_, k)| over.arena[k].children.is_empty()); let in_base = has_container.contains_key(name); let base_container = has_container.get(name).copied().unwrap_or(false); if over_leafy && !base_container { @@ -6059,7 +6061,10 @@ fn edit_distance(a: &str, b: &str, cap: usize) -> usize { let mut row_min = cur[0]; for j in lo..=hi { let cost = if a[i - 1] == b[j - 1] { 0 } else { 1 }; - cur[j] = (prev[j] + 1).min(cur[j - 1] + 1).min(prev[j - 1] + cost).min(inf); + cur[j] = (prev[j] + 1) + .min(cur[j - 1] + 1) + .min(prev[j - 1] + cost) + .min(inf); row_min = row_min.min(cur[j]); } if hi < b.len() { diff --git a/source/rust/src/main.rs b/source/rust/src/main.rs index 6b0f3d7..c4c1c28 100644 --- a/source/rust/src/main.rs +++ b/source/rust/src/main.rs @@ -10,6 +10,34 @@ use shcl::{ }; use std::process::ExitCode; +// Every stdout write goes through these. On windows a reader that closed +// early is not a signal but a write error, and the std print macros turn that +// into a panic, which the release build aborts on; a `fmt` piped into `more` +// must not do that. unix never reaches the error branch: main restores the +// SIGPIPE default and the write kills the process the conventional way. +macro_rules! out { + ($($arg:tt)*) => {{ + use std::io::Write; + if write!(std::io::stdout(), $($arg)*).is_err() { + broken_pipe(); + } + }}; +} +macro_rules! outln { + ($($arg:tt)*) => {{ + use std::io::Write; + if writeln!(std::io::stdout(), $($arg)*).is_err() { + broken_pipe(); + } + }}; +} + +/// 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) +} + const HELP: &str = "\ shcl - Simple Hierarchical Config Language (reference CLI) @@ -843,9 +871,9 @@ fn do_get(o: &Opts) -> u8 { let emit = |lines: &[String]| { for (i, l) in lines.iter().enumerate() { if o.slots { - println!("{:?}\t{}", slot_at(i), l); + outln!("{:?}\t{}", slot_at(i), l); } else { - println!("{}", l); + outln!("{}", l); } } }; @@ -904,9 +932,9 @@ fn do_get(o: &Opts) -> u8 { } else { let dv = o.default.clone().unwrap_or_default(); if o.slots { - println!("{:?}\t{}", status, dv); + outln!("{:?}\t{}", status, dv); } else { - println!("{}", dv); + outln!("{}", dv); } } 0 @@ -963,7 +991,7 @@ fn do_fmt(o: &Opts) -> u8 { if o.write { return write_back(&doc, file, o); } - print!("{}", doc.to_canonical()); + out!("{}", doc.to_canonical()); 0 } @@ -1202,7 +1230,7 @@ fn do_set(o: &Opts) -> u8 { if o.write { return write_back(&doc, file, o); } - print!("{}", doc.to_canonical()); + out!("{}", doc.to_canonical()); 0 } @@ -1265,7 +1293,7 @@ fn do_check(o: &Opts) -> u8 { // A V090-V093 line number is a SCHEMA line (the code table says so); the // prose names the file so the two number spaces cannot be confused. for d in &diags { - println!("line {}: {:?}: {}", d.line, d.severity, d.code); + outln!("line {}: {:?}: {}", d.line, d.severity, d.code); } say_diagnostics(&diags); let errors = diags @@ -1273,14 +1301,14 @@ fn do_check(o: &Opts) -> u8 { .filter(|d| d.severity == Severity::Error) .count(); if strict_failed { - println!("strict load failed: {} diagnostic(s)", diags.len()); + outln!("strict load failed: {} diagnostic(s)", diags.len()); 6 } else if errors > 0 { // Loaded, but lines were dropped: nonzero so a CI gate on check catches it. - println!("failed: {} diagnostic(s), {} error(s)", diags.len(), errors); + outln!("failed: {} diagnostic(s), {} error(s)", diags.len(), errors); 6 } else { - println!("ok ({} diagnostic(s))", diags.len()); + outln!("ok ({} diagnostic(s))", diags.len()); 0 } } @@ -1321,7 +1349,7 @@ fn do_init(o: &Opts) -> u8 { } match generate(&sdoc, o.no_banner) { Ok(text) => { - print!("{}", text); + out!("{}", text); 0 } Err(faults) => { @@ -1352,10 +1380,10 @@ fn do_enum(o: &Opts, want_count: bool) -> u8 { // One report per invocation, so a read in a loop is one line per call. say_diagnostics(&diags); if want_count { - println!("{}", doc.count(path)); + outln!("{}", doc.count(path)); } else { for v in doc.instances(path) { - println!("{}", v); + outln!("{}", v); } } 0 @@ -1380,7 +1408,7 @@ fn do_children(o: &Opts) -> u8 { }; say_diagnostics(&diags); for name in doc.children(path) { - println!("{}", shcl::quote_segment(&name)); + outln!("{}", shcl::quote_segment(&name)); } 0 } @@ -1398,7 +1426,7 @@ fn do_paths(o: &Opts) -> u8 { }; say_diagnostics(&diags); for p in doc.paths() { - println!("{}", p); + outln!("{}", p); } 0 } @@ -1510,19 +1538,19 @@ fn main() -> ExitCode { // 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 asked == Some("help") || first == Some("help") || argv.is_empty() { - print!("\n{}\n", HELP); + out!("\n{}\n", HELP); return ExitCode::from(0); } if asked == Some("version") || first == Some("version") { - println!("shcl {}", env!("CARGO_PKG_VERSION")); + outln!("shcl {}", env!("CARGO_PKG_VERSION")); return ExitCode::from(0); } if asked == Some("about") || first == Some("about") { - print!("\n{}\n", ABOUT); + out!("\n{}\n", ABOUT); return ExitCode::from(0); } if asked == Some("donate") || first == Some("donate") { - print!("\n{}\n", DONATE); + out!("\n{}\n", DONATE); return ExitCode::from(0); } let cmd = argv[0].clone(); diff --git a/source/rust/tests/cli_pipe.rs b/source/rust/tests/cli_pipe.rs new file mode 100644 index 0000000..6514d68 --- /dev/null +++ b/source/rust/tests/cli_pipe.rs @@ -0,0 +1,55 @@ +//! A reader that closes stdout early must not make the CLI print anything on +//! stderr or fail loudly: on unix it dies of SIGPIPE like every other tool, +//! and on windows, which has no such signal, it exits quietly. The release +//! build used to abort there ("failed printing to stdout"), so piping `fmt` +//! into `more` or `Select-Object -First` on windows was an abort. + +use std::io::{Read, Write}; +use std::process::{Command, Stdio}; + +#[test] +fn early_closed_stdout_is_quiet() { + let dir = std::env::temp_dir().join(format!("shcl-pipe-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("big.shcl"); + { + let mut f = std::fs::File::create(&file).unwrap(); + for i in 0..40_000 { + writeln!(f, "k{i}: {i}").unwrap(); + } + } + let mut child = Command::new(env!("CARGO_BIN_EXE_shcl")) + .arg("fmt") + .arg(&file) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // Take one byte, then close our end while the CLI still has most of the + // document to write. + let mut out = child.stdout.take().unwrap(); + let mut one = [0u8; 1]; + out.read_exact(&mut one).unwrap(); + drop(out); + let mut err = String::new(); + child + .stderr + .take() + .unwrap() + .read_to_string(&mut err) + .unwrap(); + let status = child.wait().unwrap(); + let _ = std::fs::remove_dir_all(&dir); + assert_eq!(err, "", "stderr must stay empty on a broken pipe"); + #[cfg(unix)] + { + use std::os::unix::process::ExitStatusExt; + assert!( + status.signal() == Some(13) || status.success(), + "expected SIGPIPE or a clean exit, got {status:?}" + ); + } + #[cfg(not(unix))] + assert!(status.success(), "expected a clean exit, got {status:?}"); +} diff --git a/source/rust/tests/fuzz_smoke.rs b/source/rust/tests/fuzz_smoke.rs index 0783094..01cc0db 100644 --- a/source/rust/tests/fuzz_smoke.rs +++ b/source/rust/tests/fuzz_smoke.rs @@ -275,7 +275,8 @@ fn merge_never_panics_and_stays_fixpoint() { empty.to_canonical(), Document::parse(&b).to_canonical(), "merge onto empty base is not the identity at iteration {} for:\n{}", - i, b + i, + b ); } } From fdd84fe13a314f056db89660c78dd84b812b721f Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:08:51 -0700 Subject: [PATCH 09/19] c cli wide argv, publish errno --- cicd/utility/win-runners.bash | 20 ++++++++++++++++ project/backlog.md | 16 ++++++++++--- source/c/cmd/shcl/main.c | 45 ++++++++++++++++++++++++++++++++--- source/c/shcl.h | 29 +++++++++++++++++++--- source/c/tests/conformance.c | 33 ++++++++++++++++++++++++- 5 files changed, 133 insertions(+), 10 deletions(-) diff --git a/cicd/utility/win-runners.bash b/cicd/utility/win-runners.bash index 0afadef..06ff7d9 100755 --- a/cicd/utility/win-runners.bash +++ b/cicd/utility/win-runners.bash @@ -78,6 +78,25 @@ fRunOom() { source/c/tests/oom_hook.c -o "${work}/oom_hook${exe}" -lm \ && "${work}/oom_hook${exe}" } +## The C CLI's argv: the narrow one arrives in the active code page, best-fit +## mapped, so a name the page cannot spell reached a different file. The two +## names here are the shapes that went wrong: one outside the page, one the +## page maps onto a plain letter. +fRunCcli() { + "${cc}" -std=c11 -O2 -Wall -Wextra -Werror -Isource/c \ + source/c/cmd/shcl/main.c -o "${work}/shcl-c${exe}" -lm || return 1 + local dir="${work}/argv" + mkdir -p "${dir}" + printf 'a: 1\n' > "${dir}/a.shcl" + printf 'b: 1\n' > "${dir}/ā.shcl" + printf 'c: 1\n' > "${dir}/cafÃĐ.shcl" + local got + got="$("${work}/shcl-c${exe}" get "${dir}/cafÃĐ.shcl" c 2>&1)" || { echo "win-runners: c cli: cafÃĐ.shcl: ${got}" >&2; return 1; } + [[ "${got}" == "1" ]] || { echo "win-runners: c cli: cafÃĐ.shcl read ${got@Q}" >&2; return 1; } + "${work}/shcl-c${exe}" set --write --set x=1 "${dir}/ā.shcl" > /dev/null || return 1 + [[ "$(cat "${dir}/a.shcl")" == "a: 1" ]] || { echo "win-runners: c cli: a.shcl was rewritten in place of ā.shcl" >&2; return 1; } + grep -q '^x: 1$' "${dir}/ā.shcl" || { echo "win-runners: c cli: ā.shcl was not written" >&2; return 1; } +} ## Fuzz iterations stay at the in-test default: the long soak is the Linux gate's ## job, and nothing about it is platform-dependent. @@ -88,6 +107,7 @@ fRun "python" "${py}" source/python/tests/conformance.py fRun "c" fRunC fRun "c++ veneer" fRunCxx fRun "c oom hook" fRunOom +fRun "c cli argv" fRunCcli ## The installers' PATH handling needs a real registry, which only exists here: ## it edits and restores the runner's own Environment keys, so it stays off ## every other host. diff --git a/project/backlog.md b/project/backlog.md index 3fbd0d5..82d94cf 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -134,19 +134,29 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191000 - Closed: 20260902-124500 - - 🔘 Item 12: the C CLI on Windows takes its arguments in the active code page and checks them as UTF-8, so a non-ASCII path is refused, and a name the code page best-fits maps to a different file, including for `--write`. + - ✅ Item 12: the C CLI on Windows takes its arguments in the active code page and checks them as UTF-8, so a non-ASCII path is refused, and a name the code page best-fits maps to a different file, including for `--write`. - Reproduced under wine with code page 1252. `cafÃĐ.shcl` is refused as bad encoding. With `ā.shcl` and `a.shcl` both present, `set --write` on the first exits 0 and rewrites the second. - The library's own file tier was made wide in the 20260828 fixes; the CLI's narrow `main` and the header's silence on argument encoding are what is left. The C CLI is not distributed, but a C consumer's own `main` inherits the same trap. + - Fixed: on windows the C CLI takes its arguments from the wide command line and converts them to UTF-8, and its own file opens and the directory check go through the library's wide calls instead of the narrow `fopen`/`_access`. The header's file-tier comment now says paths are UTF-8 on every platform and that a consumer's `main` has to hand over UTF-8 too. + - Pinned by a `c cli argv` step in `win-runners.bash`, which builds the C CLI on the windows job, reads `cafÃĐ.shcl` and writes `ā.shcl` beside an `a.shcl` that must stay untouched. On the old build the first is refused and the second rewrites `a.shcl`; both pass now. - Opened: 20260901-191100 + - Closed: 20260902-140000 - - 🔘 Item 13: on Windows, `shcl_write_file_atomic` reports failure with `errno` untouched when the publish step fails, so the C CLI prints `Success` beside exit 8. + - ✅ Item 13: on Windows, `shcl_write_file_atomic` reports failure with `errno` untouched when the publish step fails, so the C CLI prints `Success` beside exit 8. - Reproduced under wine: a target held open by another process, or a device name as the target, both print `: Success`. Rust and Go name the real cause. - Cause: the wide publish calls set the Win32 last error and nothing maps it to `errno`, against what the header promises. + - Fixed: a failed publish maps `GetLastError` onto errno (`EACCES` for a sharing or lock violation, `ENOENT`, `EEXIST`, `ENOSPC`, `EINVAL`, `EIO` for the rest), and the temp-file unlink on the failure path no longer overwrites the errno the failure left, on either platform. + - Pinned by a windows-only fixture in the C runner: a write over a target held open without delete sharing, and one to a device name, both fail with a non-zero errno. Fails on the old header, passes now. + - Note: wine's `strtod` (and mingw's `__mingw_strtod`) read `7.67844768714563e-239` one ulp high where glibc, UCRT and Python agree, so under wine the C runner fails corpus `080` in the float formatter's read-back test. The C binding is only as exact as its C runtime's `strtod`; the hosted job decides what real windows does. - Opened: 20260901-191200 + - Closed: 20260902-140000 - - 🔘 Item 14: the C runner's Windows read-only fixture cannot see a leftover temp file, because it skips every dotfile and the temp name starts with a dot. + - ✅ Item 14: the C runner's Windows read-only fixture cannot see a leftover temp file, because it skips every dotfile and the temp name starts with a dot. - Reproduced: a planted `.ro.shcl.tmp999.0` passes the fixture. The other three runners count it. + - Fixed: the fixture skips only `.` and `..` when counting what the write left behind, so a leftover dot-named temp counts like it does in the other three runners. + - Pinned by planting `.ro.shcl.tmp999.0` in the fixture's directory: the old filter counted nothing, the new one fails the fixture. - Opened: 20260901-191300 + - Closed: 20260902-140000 - 🔘 Item 15: the `.deb` and `.rpm` declare no dependencies, so they install cleanly on a system where the binary cannot run. - Reproduced with the pipeline's own packages: lintian reports undeclared ELF prerequisites and the rpm lists no requires. The binary needs glibc 2.34 and libgcc; on Debian 11 or RHEL 8 the package installs and `shcl` dies with a loader error, which is exactly what the installer's glibc check exists to prevent. diff --git a/source/c/cmd/shcl/main.c b/source/c/cmd/shcl/main.c index a7c4e62..0e54952 100644 --- a/source/c/cmd/shcl/main.c +++ b/source/c/cmd/shcl/main.c @@ -25,6 +25,7 @@ #include #ifdef _WIN32 #include + #include #include #include #include @@ -246,11 +247,22 @@ static void *xdoc(void *p) { // Reads FILE (or stdin for "-") fully. Returns malloc'd buffer + len, or NULL on // error (message printed to stderr). Rejects invalid UTF-8 like the reference. +// A narrow fopen reads the path in the active code page. The argv here is +// UTF-8 (see utf8_argv), so the file opens go through the library's wide +// open, which is the only way a name outside the code page reaches its file. +static FILE *open_rb(const char *file) { +#ifdef _WIN32 + return shcl_fopen_rb(file); +#else + return fopen(file, "rb"); +#endif +} + // Nothing at the path at all, as opposed to something there that will not open. // fopen rather than stat/access so the check needs no platform header: a // directory opens here and a protected file sets EACCES, so both stay errors. static int path_absent(const char *file) { - FILE *p = fopen(file, "rb"); + FILE *p = open_rb(file); if (p) { fclose(p); return 0; } return errno == ENOENT; } @@ -259,7 +271,7 @@ static char *read_input(const char *file, size_t *len) { char *buf = NULL; size_t cap = 0, n = 0; int is_stdin = strcmp(file, "-") == 0; const char *who = is_stdin ? "stdin" : file; - FILE *f = is_stdin ? stdin : fopen(file, "rb"); + FILE *f = is_stdin ? stdin : open_rb(file); if (!f) { fprintf(stderr, "%s: %s\n", who, strerror(errno)); return NULL; } char chunk[65536]; size_t r; errno = 0; @@ -554,7 +566,10 @@ static int dir_writable(const char *file) { else if (slash == file) snprintf(dir, sizeof dir, "/"); else snprintf(dir, sizeof dir, "%.*s", (int)(slash - file), file); #ifdef _WIN32 - return _access(dir, 2) == 0; + wchar_t *w = shcl_widen(dir); + int ok = w && _waccess(w, 2) == 0; + free(w); + return ok; #else return access(dir, W_OK) == 0; #endif @@ -1205,6 +1220,29 @@ static int is_command(const char *cmd) { return 0; } +#ifdef _WIN32 +// The narrow argv arrives in the active code page, best-fit mapped: a name +// the page cannot spell becomes a different name, and `--write` then rewrites +// a different file. The wide command line is exact; hand it over as UTF-8, +// which is what the library's file tier expects. NULL when the conversion +// fails (nothing sensible is left to run). +static char **utf8_argv(int *argc) { + int n = 0; + wchar_t **wargv = CommandLineToArgvW(GetCommandLineW(), &n); + if (!wargv) return NULL; + char **out = (char **)calloc((size_t)n + 1, sizeof *out); + if (!out) return NULL; + for (int i = 0; i < n; i++) { + int len = WideCharToMultiByte(CP_UTF8, 0, wargv[i], -1, NULL, 0, NULL, NULL); + if (len <= 0 || !(out[i] = (char *)malloc((size_t)len))) return NULL; + WideCharToMultiByte(CP_UTF8, 0, wargv[i], -1, out[i], len, NULL, NULL); + } + LocalFree(wargv); + *argc = n; + return out; +} +#endif + int main(int argc, char **argv) { setlocale(LC_ALL, "C"); // strtod/printf must use '.' regardless of environment #ifdef _WIN32 @@ -1212,6 +1250,7 @@ int main(int argc, char **argv) { _setmode(_fileno(stdin), _O_BINARY); _setmode(_fileno(stdout), _O_BINARY); _setmode(_fileno(stderr), _O_BINARY); + if (!(argv = utf8_argv(&argc))) { fprintf(stderr, "cannot read the command line\n"); return 1; } #endif // Reject non-UTF-8 argv up front (exit 1), matching the reference; the parser // assumes valid UTF-8, and a garbled arg is a usage error, not a real miss. diff --git a/source/c/shcl.h b/source/c/shcl.h index 756492a..ee08562 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -188,7 +188,10 @@ void shcl_suppress_declared_reopens(shcl_doc *schema, shcl_doc *doc); shcl_doc *shcl_load_and_validate(const char *text, size_t len, const char *schema, size_t slen, shcl_strictness s); // File tier (optional companion; compile out with -DSHCL_NO_FILE_IO to keep -// the core free of file I/O). Load does not fail on the file's account: the +// the core free of file I/O). Paths are UTF-8 on every platform: on windows +// they are widened for the file calls rather than read in the active code +// page, so a program's own main() has to hand over UTF-8 too (the wide +// command line, not the narrow argv). Load does not fail on the file's account: the // document always comes back usable (empty when the file could not be read), // and the status out-param (may be NULL) separates the four cases a consumer's // own load path otherwise confuses. NULL means an allocation failed, as for a @@ -5291,6 +5294,23 @@ static wchar_t *shcl_widen(const char *s) { return w; } +// The Win32 calls report through GetLastError and leave errno alone, and +// errno is what the header promises a failed write describes. The common +// causes map; the rest is EIO, which at least is not "Success". +static int shcl_errno_from_win32(DWORD e) { + switch (e) { + case ERROR_FILE_NOT_FOUND: case ERROR_PATH_NOT_FOUND: case ERROR_INVALID_DRIVE: return ENOENT; + case ERROR_ACCESS_DENIED: case ERROR_SHARING_VIOLATION: case ERROR_LOCK_VIOLATION: case ERROR_USER_MAPPED_FILE: return EACCES; + case ERROR_ALREADY_EXISTS: case ERROR_FILE_EXISTS: return EEXIST; + case ERROR_NOT_ENOUGH_MEMORY: case ERROR_OUTOFMEMORY: return ENOMEM; + case ERROR_INVALID_NAME: case ERROR_BAD_PATHNAME: case ERROR_INVALID_PARAMETER: case ERROR_FILENAME_EXCED_RANGE: return EINVAL; + case ERROR_DISK_FULL: case ERROR_HANDLE_DISK_FULL: return ENOSPC; + case ERROR_BUSY: return EBUSY; + case ERROR_DIRECTORY: return ENOTDIR; + default: return EIO; + } +} + // ReplaceFile carries the destination's ACLs, attributes and named streams // onto the replacement; a move publishes a brand-new file and leaves all of it // behind. It needs the destination to exist, and it fails rather than skip a @@ -5298,9 +5318,11 @@ static wchar_t *shcl_widen(const char *s) { // to MoveFileEx - which is there regardless because C rename() will not // replace an existing file on Windows at all. static int shcl_publish_file(const wchar_t *tmp, const wchar_t *target) { - return (GetFileAttributesW(target) != INVALID_FILE_ATTRIBUTES + int ok = (GetFileAttributesW(target) != INVALID_FILE_ATTRIBUTES && ReplaceFileW(target, tmp, NULL, REPLACEFILE_WRITE_THROUGH, NULL, NULL)) || MoveFileExW(tmp, target, MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH); + if (!ok) errno = shcl_errno_from_win32(GetLastError()); + return ok; } #endif @@ -5489,7 +5511,8 @@ int shcl_write_file_atomic(const char *path, const char *data, size_t n) { ok = ok && rename(tmp, target) == 0; if (ok) shcl_sync_dir(target); #endif - if (!ok) SHCL_FILE_UNLINK(); + // The unlink must not overwrite the errno the failure left behind. + if (!ok) { int e = errno; SHCL_FILE_UNLINK(); errno = e; } free(tmp); SHCL_FILE_CLEANUP(); #undef SHCL_FILE_CLEANUP diff --git a/source/c/tests/conformance.c b/source/c/tests/conformance.c index 8a3ebb7..991cd06 100644 --- a/source/c/tests/conformance.c +++ b/source/c/tests/conformance.c @@ -1187,14 +1187,45 @@ int main(int argc, char **argv) { if (!rt || rn != 5 || memcmp(rt, "a: 2\n", 5) != 0) fail("readonly", "file not rewritten"); free(rt); if (!(GetFileAttributesA(rfile) & FILE_ATTRIBUTE_READONLY)) fail("readonly", "file did not come back read-only"); + // The temp name starts with a dot, so only the two directory entries + // are skipped, not every dotfile. DIR *rdd = opendir(rdir); int left = 0; const struct dirent *re; - while (rdd && (re = readdir(rdd))) if (re->d_name[0] != '.') left++; + while (rdd && (re = readdir(rdd))) if (strcmp(re->d_name, ".") != 0 && strcmp(re->d_name, "..") != 0) left++; if (rdd) closedir(rdd); if (left != 1) fail("readonly", "a temp file was left behind"); shcl_free(rd); SetFileAttributesA(rfile, FILE_ATTRIBUTE_NORMAL); remove(rfile); rmdir(rdir); } + // A publish that fails leaves errno describing it. The wide calls report + // through GetLastError and used to leave errno at 0, so the CLI printed + // "Success" beside exit 8. A target held open without delete sharing + // fails the replace; a device name fails the move. + { + char hdir[256], hfile[288]; + snprintf(hdir, sizeof hdir, "%s/shcl-held-%ld", tmp_root(), (long)getpid()); + snprintf(hfile, sizeof hfile, "%s/held.shcl", hdir); + if (_mkdir(hdir) != 0) fail("errno", "mkdir failed"); + FILE *hf = fopen(hfile, "wb"); + if (!hf || fputs("a: 1\n", hf) == EOF || fclose(hf) != 0) fail("errno", "seed write failed"); + wchar_t wh[320]; + if (MultiByteToWideChar(CP_UTF8, 0, hfile, -1, wh, 320) == 0) fail("errno", "widen failed"); + HANDLE hold = CreateFileW(wh, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (hold == INVALID_HANDLE_VALUE) fail("errno", "hold failed"); + errno = 0; + if (shcl_write_file_atomic(hfile, "a: 2\n", 5)) fail("errno", "write over a held file succeeded"); + if (errno == 0) fail("errno", "failed publish left errno at 0"); + CloseHandle(hold); + errno = 0; + if (shcl_write_file_atomic("nul", "a: 2\n", 5)) fail("errno", "write to a device name succeeded"); + if (errno == 0) fail("errno", "failed publish to a device left errno at 0"); + DIR *hdd = opendir(hdir); const struct dirent *he; + while (hdd && (he = readdir(hdd))) if (strcmp(he->d_name, ".") != 0 && strcmp(he->d_name, "..") != 0) { + char left[600]; snprintf(left, sizeof left, "%s/%s", hdir, he->d_name); remove(left); + } + if (hdd) closedir(hdd); + rmdir(hdir); + } #endif // Reads and saves must not retain: a read of a plain field hands back a // slice of the retained input (a million reads once grew a document by From bcccdc3aefc0d21f2b20f467f55469714c9e62c1 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:10:49 -0700 Subject: [PATCH 10/19] package deps --- cicd/packaging/nfpm.yaml | 20 +++++++++++++++++++ cicd/utility/package.bash | 41 ++++++++++++++++++++++++++++++++++++--- project/backlog.md | 5 ++++- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/cicd/packaging/nfpm.yaml b/cicd/packaging/nfpm.yaml index 84e8524..23c5520 100644 --- a/cicd/packaging/nfpm.yaml +++ b/cicd/packaging/nfpm.yaml @@ -15,6 +15,17 @@ description: | bindings (Rust, Go, Python, C/C++) and Bash/PowerShell wrappers. homepage: https://github.com/jim-collier/shcl license: MIT +# What the binary links against, read off the binary itself by package.bash: +# the glibc floor is its newest GLIBC_ version, and libgcc joins only when +# libgcc_s is in NEEDED. Without these a package installs on a system where +# the binary then dies in the loader. +overrides: + deb: + depends: + - libc6 (>= ${SHCL_GLIBC})${SHCL_DEB_LIBGCC} + rpm: + depends: + - glibc >= ${SHCL_GLIBC}${SHCL_RPM_LIBGCC} rpm: # rpm stamps the building machine's hostname into the package; a fixed name # keeps two builds of one commit byte-identical (the build date comes from @@ -35,6 +46,15 @@ contents: dst: /usr/share/man/man1/shcl.1.gz file_info: mode: 0644 + # Debian policy wants both under /usr/share/doc/. + - src: ${SHCL_PAYLOAD}/doc/copyright + dst: /usr/share/doc/shcl/copyright + file_info: + mode: 0644 + - src: ${SHCL_PAYLOAD}/doc/changelog.gz + dst: /usr/share/doc/shcl/changelog.gz + file_info: + mode: 0644 # Completions go to the shell's own directory, not under /usr/share/shcl: # bash-completion and zsh only autoload from theirs. - src: ${SHCL_PAYLOAD}/completions/shcl.bash diff --git a/cicd/utility/package.bash b/cicd/utility/package.bash index cf2102d..7e2ee4d 100755 --- a/cicd/utility/package.bash +++ b/cicd/utility/package.bash @@ -32,6 +32,7 @@ meDir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" fEcho(){ echo "[ $* ]"; } fWarn(){ fEcho "WARNING: $*"; } +fDie(){ echo "package: $*" >&2; exit 1; } ## NSIS wants four dot-separated integers where the project version can carry a ## prerelease tail. Same rule the Rust build script uses for the executable's @@ -68,6 +69,25 @@ find "${payload}" -exec touch -d "@${SOURCE_DATE_EPOCH}" {} + built=0 +## The deb's Depends and the rpm's Requires against the binary they carry: +## the glibc floor, libgcc when linked, and the Debian doc files. +fCheckDeps(){ + local stem="$1" glibc="$2" needGcc="$3" deps + deps="$(dpkg-deb -f "${stem}.deb" Depends)" + [[ "${deps}" == *"libc6 (>= ${glibc})"* ]] || fDie "$(basename "${stem}").deb: Depends ${deps@Q} lacks libc6 (>= ${glibc})" + if [[ -n "${needGcc}" && "${deps}" != *libgcc-s1* ]]; then fDie "$(basename "${stem}").deb: Depends ${deps@Q} lacks libgcc-s1"; fi + ## Into a variable first: grep -q quitting early would kill the tar behind + ## dpkg-deb with SIGPIPE and fail the pipeline for the wrong reason. + local listing; listing="$(dpkg-deb -c "${stem}.deb")" + grep -q ' ./usr/share/doc/shcl/copyright$' <<<"${listing}" || fDie "$(basename "${stem}").deb: no copyright file" + grep -q ' ./usr/share/doc/shcl/changelog.gz$' <<<"${listing}" || fDie "$(basename "${stem}").deb: no changelog" + if command -v rpm >/dev/null 2>&1; then + deps="$(rpm -qp --requires "${stem}.rpm" 2>/dev/null)" + [[ "${deps}" == *"glibc >= ${glibc}"* ]] || fDie "$(basename "${stem}").rpm: Requires ${deps@Q} lacks glibc >= ${glibc}" + if [[ -n "${needGcc}" && "${deps}" != *libgcc* ]]; then fDie "$(basename "${stem}").rpm: Requires ${deps@Q} lacks libgcc"; fi + fi +} + ## Linux: .deb + .rpm per arch with a binary present. nfpm arch names are ## GOARCH-style; the artifact names carry the uname-style spelling. The config ## template is sed-rendered per build (nfpm won't expand env vars in src paths). @@ -77,18 +97,28 @@ if command -v nfpm >/dev/null 2>&1; then ## formats want the man page compressed; -n keeps the timestamp and the ## original name out of the gzip header, so the same source gives the same ## bytes on every build. - mkdir -p "${payload}/man" "${payload}/completions" + mkdir -p "${payload}/man" "${payload}/completions" "${payload}/doc" gzip -9 -n -c "${root}/source/man/shcl.1" > "${payload}/man/shcl.1.gz" cp "${root}/source/completions/shcl.bash" "${root}/source/completions/_shcl" "${payload}/completions/" - chmod 644 "${payload}/man/shcl.1.gz" "${payload}"/completions/* - fPinMtime "${payload}/man" "${payload}/man/shcl.1.gz" "${payload}/completions" "${payload}"/completions/* + cp "${root}/license.md" "${payload}/doc/copyright" + gzip -9 -n -c "${root}/changelog.md" > "${payload}/doc/changelog.gz" + chmod 644 "${payload}/man/shcl.1.gz" "${payload}"/completions/* "${payload}"/doc/* + fPinMtime "${payload}/man" "${payload}/man/shcl.1.gz" "${payload}/completions" "${payload}"/completions/* "${payload}/doc" "${payload}"/doc/* for pair in "x86_64|amd64" "arm64|arm64"; do osarch="${pair%%|*}"; goarch="${pair#*|}" bin="${artDir}/shcl-${ver}-linux-${osarch}" [[ -f "${bin}" ]] || continue fPinMtime "${bin}" + ## The dependencies come off the binary: its newest GLIBC_ symbol + ## version is the glibc floor, and libgcc is needed only when the + ## dynamic section says so (the arm64 build links it statically). + glibc="$(objdump -T "${bin}" | grep -o 'GLIBC_[0-9.]*' | sed 's/GLIBC_//' | sort -uV | tail -1)" + [[ -n "${glibc}" ]] || fDie "no GLIBC_ symbol version in ${bin}" + debGcc=""; rpmGcc="" + if readelf -d "${bin}" | grep -q 'NEEDED.*libgcc_s'; then debGcc=$'\n - libgcc-s1'; rpmGcc=$'\n - libgcc'; fi sed -e "s|\${SHCL_VERSION}|${ver}|g" -e "s|\${SHCL_ARCH}|${goarch}|g" \ -e "s|\${SHCL_BIN}|${bin}|g" -e "s|\${SHCL_PAYLOAD}|${payload}|g" \ + -e "s|\${SHCL_GLIBC}|${glibc}|g" -e "s|\${SHCL_DEB_LIBGCC}|${debGcc//$'\n'/\\n}|g" -e "s|\${SHCL_RPM_LIBGCC}|${rpmGcc//$'\n'/\\n}|g" \ "${meDir}/../packaging/nfpm.yaml" > "${payload}/nfpm.yaml" for fmt in deb rpm; do out="${artDir}/shcl-${ver}-linux-${osarch}.${fmt}" @@ -96,6 +126,9 @@ if command -v nfpm >/dev/null 2>&1; then fEcho "OK: package: $(basename "${out}") ($(du -h --apparent-size "${out}" | cut -f1))" built=$((built + 1)) done + ## What went in has to match what the binary needs, or the package + ## installs where the binary cannot run. + fCheckDeps "${artDir}/shcl-${ver}-linux-${osarch}" "${glibc}" "${debGcc:+1}" done else fWarn "nfpm not installed; .deb/.rpm skipped" @@ -131,3 +164,5 @@ fi ## - 2026-07-22: Created: nfpm deb/rpm + NSIS setup over the release artifact dir. ## - 2026-08-29: Reproducible output: mtimes pinned to the commit time; man page ## and completions staged for the Linux packages only. +## - 2026-09-02: Dependencies read off the binary, Debian copyright and changelog +## files, and a check of both against every package built. diff --git a/project/backlog.md b/project/backlog.md index 82d94cf..580dd18 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: 20260901-191300 - Closed: 20260902-140000 - - 🔘 Item 15: the `.deb` and `.rpm` declare no dependencies, so they install cleanly on a system where the binary cannot run. + - ✅ Item 15: the `.deb` and `.rpm` declare no dependencies, so they install cleanly on a system where the binary cannot run. - Reproduced with the pipeline's own packages: lintian reports undeclared ELF prerequisites and the rpm lists no requires. The binary needs glibc 2.34 and libgcc; on Debian 11 or RHEL 8 the package installs and `shcl` dies with a loader error, which is exactly what the installer's glibc check exists to prevent. - Also from the same lintian run: no copyright file, no changelog, an unknown `License` field. Cheap to close together. + - Fixed: the packages declare what the binary links against, read off the binary at build time: its newest `GLIBC_` symbol version is the glibc floor (`libc6 (>= 2.34)` and `glibc >= 2.34` on x86_64, 2.30 on arm64), and `libgcc-s1` / `libgcc` join only when `libgcc_s` is in the dynamic section (x86_64 only; arm64 links it statically). The deb also carries `/usr/share/doc/shcl/copyright` (the license) and `changelog.gz`. lintian's two errors and the prerequisites warning are gone; the `License` field warning is nfpm's own and stays. + - Pinned by `package.bash` itself: after each build it reads the deb's Depends and the rpm's Requires back and fails unless they carry the floor it derived, libgcc when needed, and the two doc files. Against the 2.0.0 package it fails on the empty Depends. - Opened: 20260901-191400 + - Closed: 20260902-143000 - 🔘 Item 16: a system install under umask 077 still leaves the man directory root-only when the installer has to create it. - Reproduced with the system paths redirected into a sandbox. The 20260830b fix widens the install root only; `man1` under `/usr/local/share/man` is created by a plain `mkdir` under the caller's umask and is not widened, and a fresh Debian does not have it. From 4b2672149ac0e4fba3615c0b331c7b32207f2a82 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:13:19 -0700 Subject: [PATCH 11/19] installer modes and PATH note --- cicd/utility/package.bash | 2 +- cicd/utility/shell-regress.bash | 42 ++++++++++++++---- install-dev.bash | 11 ++++- install.bash | 77 +++++++++++++++++++++++---------- project/backlog.md | 10 ++++- 5 files changed, 106 insertions(+), 36 deletions(-) diff --git a/cicd/utility/package.bash b/cicd/utility/package.bash index 7e2ee4d..1100d54 100755 --- a/cicd/utility/package.bash +++ b/cicd/utility/package.bash @@ -112,7 +112,7 @@ if command -v nfpm >/dev/null 2>&1; then ## The dependencies come off the binary: its newest GLIBC_ symbol ## version is the glibc floor, and libgcc is needed only when the ## dynamic section says so (the arm64 build links it statically). - glibc="$(objdump -T "${bin}" | grep -o 'GLIBC_[0-9.]*' | sed 's/GLIBC_//' | sort -uV | tail -1)" + glibc="$(objdump -T "${bin}" | { grep -o 'GLIBC_[0-9.]*' || true; } | sed 's/GLIBC_//' | sort -uV | tail -1)" [[ -n "${glibc}" ]] || fDie "no GLIBC_ symbol version in ${bin}" debGcc=""; rpmGcc="" if readelf -d "${bin}" | grep -q 'NEEDED.*libgcc_s'; then debGcc=$'\n - libgcc-s1'; rpmGcc=$'\n - libgcc'; fi diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index d231362..b0a4107 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -102,22 +102,46 @@ fi ## 20260830b item 12: nothing set the modes on a system install, and sudo keeps ## the caller's umask, so under 077 the tree and the launcher came out 0700 and -## only root could run what had just been installed for everyone. Staged the -## way the installer stages it, under that umask. -eval "$(sed -n '/^fWidenModes()/,/^}/p' "${repoDir}/install.bash")" +## only root could run what had just been installed for everyone. 20260901b +## item 16: the widening covered the install root alone, and a man1 directory +## the installer had to create stayed root-only. The installer's own lay-down +## step runs here on a staged payload, under that umask, into a sandbox whose +## bin and man1 directories do not exist yet. +eval "$(sed -n '/^fWidenModes()/,/^}/p;/^fTopMissing()/,/^}/p;/^fLayDown()/,/^}/p' "${repoDir}/install.bash")" ( umask 077 - mkdir -p "${tmpDir}/inst/code" - printf 'bin\n' > "${tmpDir}/inst/shcl"; chmod 700 "${tmpDir}/inst/shcl" - printf 'data\n' > "${tmpDir}/inst/code/lib.rs" + mkdir -p "${tmpDir}/stage/code" "${tmpDir}/stage/scripts" "${tmpDir}/stage/man" "${tmpDir}/stage/completions" "${tmpDir}/sys/usr/local/share" + printf 'bin\n' > "${tmpDir}/stage/shcl"; chmod 700 "${tmpDir}/stage/shcl" + printf 'data\n' > "${tmpDir}/stage/code/lib.rs" + printf 'data\n' > "${tmpDir}/stage/scripts/shcl.bash" + printf 'man\n' > "${tmpDir}/stage/man/shcl.1" + printf 'comp\n' > "${tmpDir}/stage/completions/shcl.bash" + ## The installer's own globals, as the lifted function reads them. + # shellcheck disable=SC2034 + asroot="" tmp="${tmpDir}/stage" dest="${tmpDir}/sys/opt/shcl" link="${tmpDir}/sys/usr/local/bin/shcl" + # shellcheck disable=SC2034 + manlink="${tmpDir}/sys/usr/local/share/man/man1/shcl.1" target=system have_dropins=1 have_docs=1 + fLayDown ) -fWidenModes "" "${tmpDir}/inst" while IFS= read -r row; do case "${row}" in - "755 ${tmpDir}/inst"|"755 ${tmpDir}/inst/code"|"755 ${tmpDir}/inst/shcl"|"644 ${tmpDir}/inst/code/lib.rs") ;; + "755 d "*|"755 f ${tmpDir}/sys/opt/shcl/shcl"|"644 f "*|"777 l "*) ;; *) fBad "install.bash left a system install unreadable: ${row}" ;; esac -done < <(find "${tmpDir}/inst" -printf '%m %p\n' | sort) +done < <(find "${tmpDir}/sys/opt" "${tmpDir}/sys/usr/local/bin" "${tmpDir}/sys/usr/local/share/man" -printf '%m %y %p\n' | sort) +[[ -L "${tmpDir}/sys/usr/local/share/man/man1/shcl.1" ]] || fBad "install.bash did not link the man page" + +## 20260901b item 18: the "not on your PATH" note compared strings against +## `:dir:`, so a PATH element written with a trailing slash was not seen. +eval "$(sed -n '/^fOnPath()/,/^}/p' "${repoDir}/install.bash")" +( + PATH="/usr/bin:${tmpDir}/pbin/:/bin" + fOnPath "${tmpDir}/pbin" || fBad "install.bash: a PATH element with a trailing slash was not seen" + fOnPath "${tmpDir}/pbin/" || fBad "install.bash: a directory asked for with a trailing slash was not seen" + fOnPath "${tmpDir}/pbi" && fBad "install.bash: a PATH prefix was taken for the directory" + fOnPath "${tmpDir}/pbin/x" && fBad "install.bash: a deeper directory was taken for a PATH element" + exit 0 +) ## 20260830b item 9: the stable channel took GitHub's date-ordered "latest ## release" verbatim, so a patch back-ported to an older line after a newer one diff --git a/install-dev.bash b/install-dev.bash index d17f548..edb2102 100644 --- a/install-dev.bash +++ b/install-dev.bash @@ -287,8 +287,17 @@ echo echo "done. The gate is: cicd/cicd.bash --ci" echo "(rust-toolchain.toml pins the toolchain; the first cargo run fetches it.)" (( ${#hints[@]} )) && echo "note: the hinted packages above are still missing." +## By element, with a trailing slash ignored on either side: the shell +## resolves `bin/` fine, and a plain string compare did not. +fOnPath(){ + local dir="${1%/}" elem + while IFS= read -r -d: elem || [[ -n "${elem}" ]]; do + [[ "${elem%/}" == "${dir}" ]] && return 0 + done <<<"${orig_path}:" + return 1 +} for bindir in "${HOME}/.cargo/bin" "${gobin}"; do - [[ -n "${bindir}" && -d "${bindir}" && ":${orig_path}:" != *":${bindir}:"* ]] && echo "note: ${bindir} is not on your PATH - the tools installed there need it to be." + [[ -n "${bindir}" && -d "${bindir}" ]] && ! fOnPath "${bindir}" && echo "note: ${bindir} is not on your PATH - the tools installed there need it to be." done echo exit 0 diff --git a/install.bash b/install.bash index f4b7527..60dc05a 100644 --- a/install.bash +++ b/install.bash @@ -364,29 +364,60 @@ fWidenModes(){ ${run} chmod -R a+rX "$@" } -## Install. The binary goes in via a hidden temp + mv in the same dir, so a -## running copy only ever sees the complete old or new file. -${asroot} mkdir -p "${dest}" "$(dirname "${link}")" -${asroot} cp "${tmp}/shcl" "${dest}/.shcl.new" -${asroot} mv -f "${dest}/.shcl.new" "${dest}/shcl" -if (( have_dropins )); then - ${asroot} mkdir -p "${dest}/code" "${dest}/scripts" - ${asroot} cp "${tmp}"/code/* "${dest}/code/" - ${asroot} cp "${tmp}"/scripts/* "${dest}/scripts/" -fi -if (( have_docs )); then - ${asroot} mkdir -p "${dest}/man" "${dest}/completions" "$(dirname "${manlink}")" - ${asroot} cp "${tmp}"/man/* "${dest}/man/" - ${asroot} cp "${tmp}"/completions/* "${dest}/completions/" - ## Never over a real file: a man1/shcl.1 that is not ours came from a package. - if [[ -L "${manlink}" || ! -e "${manlink}" ]]; then - ${asroot} ln -sfn "${dest}/man/shcl.1" "${manlink}" +## The shallowest directory `mkdir -p DIR` will have to create, or nothing +## when DIR exists. The bin and man1 directories are usually there already +## on a system; when they are not, they are ours to widen too, and a system +## directory that was already there is left alone. +fTopMissing(){ + local dir="${1}" top="" + while [[ "${dir}" != "/" && "${dir}" != "." && ! -d "${dir}" ]]; do top="${dir}"; dir="$(dirname "${dir}")"; done + printf '%s' "${top}" +} + +## Lay the payload in ${tmp} down under ${dest} and link it. The binary goes +## in via a hidden temp + mv in the same dir, so a running copy only ever sees +## the complete old or new file. A function so the same steps run on a staged +## payload without the downloads in front of them. +fLayDown(){ + local desttop linkdir mandir + desttop="$(fTopMissing "${dest}")" + linkdir="$(fTopMissing "$(dirname "${link}")")" + ${asroot} mkdir -p "${dest}" "$(dirname "${link}")" + ${asroot} cp "${tmp}/shcl" "${dest}/.shcl.new" + ${asroot} mv -f "${dest}/.shcl.new" "${dest}/shcl" + if (( have_dropins )); then + ${asroot} mkdir -p "${dest}/code" "${dest}/scripts" + ${asroot} cp "${tmp}"/code/* "${dest}/code/" + ${asroot} cp "${tmp}"/scripts/* "${dest}/scripts/" fi -fi -${asroot} ln -sfn "${dest}/shcl" "${link}" -if [[ "${target}" == "system" ]]; then - fWidenModes "${asroot}" "${dest}" -fi + mandir="" + if (( have_docs )); then + mandir="$(fTopMissing "$(dirname "${manlink}")")" + ${asroot} mkdir -p "${dest}/man" "${dest}/completions" "$(dirname "${manlink}")" + ${asroot} cp "${tmp}"/man/* "${dest}/man/" + ${asroot} cp "${tmp}"/completions/* "${dest}/completions/" + ## Never over a real file: a man1/shcl.1 that is not ours came from a package. + if [[ -L "${manlink}" || ! -e "${manlink}" ]]; then + ${asroot} ln -sfn "${dest}/man/shcl.1" "${manlink}" + fi + fi + ${asroot} ln -sfn "${dest}/shcl" "${link}" + if [[ "${target}" == "system" ]]; then + fWidenModes "${asroot}" "${desttop:-${dest}}" ${linkdir:+"${linkdir}"} ${mandir:+"${mandir}"} + fi +} + +## Is DIR on the PATH? By element, with a trailing slash ignored on either +## side: the shell resolves `bin/` fine, and a plain string compare did not. +fOnPath(){ + local dir="${1%/}" elem + while IFS= read -r -d: elem || [[ -n "${elem}" ]]; do + [[ "${elem%/}" == "${dir}" ]] && return 0 + done <<<"${PATH}:" + return 1 +} + +fLayDown echo printf 'installed shcl %s -> %s\n' "${version}" "${link}" @@ -413,7 +444,7 @@ printf 'to remove it again: %s --uninstall --target=%s\n' "${rerun}" "${target}" ## check below, so say something when the symlink dir is off the PATH. It costs ## the man page too - man derives its search dirs from the bin dirs on PATH. linkdir="$(dirname "${link}")" -if [[ ":${PATH}:" != *":${linkdir}:"* ]]; then +if ! fOnPath "${linkdir}"; then # The PATH expansion is for the user to paste, not for us to expand here. # shellcheck disable=SC2016 printf 'note: %s is not on your PATH, so neither shcl nor "man shcl" will be found - add it with:\n export PATH="%s:$PATH"\n(put that line in your shell profile to make it stick)\n' "${linkdir}" "${linkdir}" diff --git a/project/backlog.md b/project/backlog.md index 580dd18..30cc235 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -166,17 +166,23 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191400 - Closed: 20260902-143000 - - 🔘 Item 16: a system install under umask 077 still leaves the man directory root-only when the installer has to create it. + - ✅ Item 16: a system install under umask 077 still leaves the man directory root-only when the installer has to create it. - Reproduced with the system paths redirected into a sandbox. The 20260830b fix widens the install root only; `man1` under `/usr/local/share/man` is created by a plain `mkdir` under the caller's umask and is not widened, and a fresh Debian does not have it. + - Fixed: the lay-down step notes, before each `mkdir -p`, the shallowest directory it will have to create (the install root's, the bin directory's, the man1 directory's) and widens those along with the install root; a directory that already existed is left alone. The step is a function now (`fLayDown`), so it can run on a staged payload. + - Pinned by `shell-regress.bash`, which runs `fLayDown` under umask 077 into a sandbox where `bin` and `man1` do not exist yet and requires every directory 755. With the widening narrowed back to the install root it reports four root-only directories. - Opened: 20260901-191500 + - Closed: 20260902-150000 - 🔘 Item 17: `sign-release.bash` signs before it checks the key, and checks nothing about the sums file. - Reproduced with a throwaway key: the run fails on the key check and leaves a valid-looking `.sig` beside the sums file. The tag check compares against `Cargo.toml` but never against the sums file's own name, and the sums are never verified against the files present, so a stale sums file from a rebuilt tree signs clean. - Opened: 20260901-191600 - - 🔘 Item 18: the installer's "not on your PATH" note fires when the directory is on PATH with a trailing slash. + - ✅ Item 18: the installer's "not on your PATH" note fires when the directory is on PATH with a trailing slash. - Reproduced. A string compare against `:dir:` misses `dir/`, which the shell resolves fine. Same shape in `install-dev.bash`. + - Fixed: a `fOnPath` helper in both installers walks PATH by element and ignores a trailing slash on either side. + - Pinned by `shell-regress.bash`: a PATH element `dir/` is seen, `dir/` asked for is seen, and a prefix or a deeper path is not. - Opened: 20260901-191700 + - Closed: 20260902-150000 - 🔘 Item 19: the profiler workload merges half its nodes and prints a hint per unit, so the profile measures hint formatting and stderr writes, and the run log carries a million hint lines. - Reproduced: the generator's trailing `service: svcN` line reopens the block above it, so every unit's two `port` leaves collide. 37% of the profiled CPU is the unbuffered hint stream. Today's run log is 93 MB, and a million of its lines are `H001`, from the profiler stage and the large-document fixpoint check. From ead24e241ec0a7e6e032cb0b375c8de8452ed44c Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:14:54 -0700 Subject: [PATCH 12/19] sign after the checks --- cicd/utility/shell-regress.bash | 32 ++++++++++++++++++ cicd/utility/sign-release.bash | 59 ++++++++++++++++++++++----------- project/backlog.md | 5 ++- 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index b0a4107..ad5a7d9 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -143,6 +143,38 @@ eval "$(sed -n '/^fOnPath()/,/^}/p' "${repoDir}/install.bash")" exit 0 ) +## 20260901b item 17: sign-release.bash wrote the signature first and checked +## the key after, so a run with the wrong key failed and left a .sig behind +## that looked finished; and nothing checked the sums file's name against the +## version, or its entries against the files. A throwaway key stands in for +## the wrong one, and every refusal must leave no .sig. +if command -v openssl >/dev/null 2>&1; then + sver="$(sed -n 's/^version *= *"\(.*\)".*/\1/p' "${repoDir}/source/rust/Cargo.toml" | head -1)" + openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:2048 -out "${tmpDir}/wrong.pem" 2>/dev/null + fSignRun(){ ## fSignRun DIR: run the signer on DIR with the throwaway key; stderr in signOut + signOut="$(bash "${repoDir}/cicd/utility/sign-release.bash" --key "${tmpDir}/wrong.pem" --dir "$1" --no-tag-check 2>&1 || true)" + } + ## Right name, right sums, wrong key: refused on the key, nothing written. + mkdir -p "${tmpDir}/sign1"; printf 'bin\n' > "${tmpDir}/sign1/shcl-${sver}-linux-x86_64" + (cd "${tmpDir}/sign1" && sha256sum "shcl-${sver}-linux-x86_64" > "shcl-${sver}-sha256sums.txt") + fSignRun "${tmpDir}/sign1" + [[ "${signOut}" == *"not this key"* ]] || fBad "sign-release.bash did not refuse the wrong key: ${signOut@Q}" + [[ ! -e "${tmpDir}/sign1/shcl-${sver}-sha256sums.txt.sig" ]] || fBad "sign-release.bash left a .sig behind after refusing the key" + ## Stale sums: an entry that no longer matches its file. + mkdir -p "${tmpDir}/sign2"; cp "${tmpDir}/sign1/"* "${tmpDir}/sign2/"; printf 'rebuilt\n' > "${tmpDir}/sign2/shcl-${sver}-linux-x86_64" + fSignRun "${tmpDir}/sign2" + [[ "${signOut}" == *"does not match the files"* ]] || fBad "sign-release.bash signed a stale sums file: ${signOut@Q}" + [[ ! -e "${tmpDir}/sign2/shcl-${sver}-sha256sums.txt.sig" ]] || fBad "sign-release.bash left a .sig behind after a stale sums file" + ## A sums file from another version. + mkdir -p "${tmpDir}/sign3"; printf 'bin\n' > "${tmpDir}/sign3/shcl-0.0.1-linux-x86_64" + (cd "${tmpDir}/sign3" && sha256sum "shcl-0.0.1-linux-x86_64" > "shcl-0.0.1-sha256sums.txt") + fSignRun "${tmpDir}/sign3" + [[ "${signOut}" == *"is not the sums file for ${sver}"* ]] || fBad "sign-release.bash signed a sums file for another version: ${signOut@Q}" + [[ ! -e "${tmpDir}/sign3/shcl-0.0.1-sha256sums.txt.sig" ]] || fBad "sign-release.bash left a .sig behind after a misnamed sums file" +else + echo "shell-regress: openssl not installed - signing rows skipped" +fi + ## 20260830b item 9: the stable channel took GitHub's date-ordered "latest ## release" verbatim, so a patch back-ported to an older line after a newer one ## shipped was handed out as stable. The fixture is in publish order, newest diff --git a/cicd/utility/sign-release.bash b/cicd/utility/sign-release.bash index 42dcd31..f95ae17 100755 --- a/cicd/utility/sign-release.bash +++ b/cicd/utility/sign-release.bash @@ -11,10 +11,12 @@ ## is offline, and this script is run by hand at release time. A key sitting ## in CI would be reachable by exactly the compromise this defends against. ## -## Signs, then verifies what it just wrote, then checks that the key used -## is the one the shipped installers actually trust - a signature made with -## the wrong key verifies perfectly on its own and fails for every user, so -## that last check is the one that matters. +## Checks first, signs last: the sums file has to be named for the version +## being cut and match the files beside it, and the key has to be the one +## the shipped installers actually trust - a signature made with the wrong +## key verifies perfectly on its own and fails for every user. Nothing is +## written until every check has passed, so a failed run leaves no .sig +## behind to be attached by mistake. ## Syntax: ## sign-release.bash --key FILE [--dir DIR] [--no-tag-check] ## --key FILE private signing key (PEM). Prompts if passphrase-protected. @@ -49,10 +51,12 @@ fUsage(){ ## is offline, and this script is run by hand at release time. A key sitting ## in CI would be reachable by exactly the compromise this defends against. ## -## Signs, then verifies what it just wrote, then checks that the key used -## is the one the shipped installers actually trust - a signature made with -## the wrong key verifies perfectly on its own and fails for every user, so -## that last check is the one that matters. +## Checks first, signs last: the sums file has to be named for the version +## being cut and match the files beside it, and the key has to be the one +## the shipped installers actually trust - a signature made with the wrong +## key verifies perfectly on its own and fails for every user. Nothing is +## written until every check has passed, so a failed run leaves no .sig +## behind to be attached by mistake. ## Syntax: ## sign-release.bash --key FILE [--dir DIR] [--no-tag-check] ## --key FILE private signing key (PEM). Prompts if passphrase-protected. @@ -95,29 +99,36 @@ command -v openssl >/dev/null || fDie "need openssl" ## sums file whose entries no download URL can reach, so refuse unless HEAD ## carries exactly v. `git tag --points-at` rather than `describe`: ## the cut puts two tags on the commit (v2.0.0 and source/go/v2.0.0). +ver="$(sed -n 's/^version *= *"\(.*\)".*/\1/p' "${root}/source/rust/Cargo.toml" | head -1)" +[[ -n "${ver}" ]] || fDie "cannot read the version from source/rust/Cargo.toml" if (( tagCheck )); then - ver="$(sed -n 's/^version *= *"\(.*\)".*/\1/p' "${root}/source/rust/Cargo.toml" | head -1)" - [[ -n "${ver}" ]] || fDie "cannot read the version from source/rust/Cargo.toml" headTags="$(git -C "${root}" tag --points-at HEAD 2>/dev/null || true)" if ! grep -qxF "v${ver}" <<<"${headTags}"; then fDie "HEAD is not tagged v${ver} (Cargo.toml says ${ver}); tag the cut first, or --no-tag-check for a rehearsal" fi fi -## Exactly one sums file, or we would be signing an ambiguous trust root. +## Exactly one sums file, or we would be signing an ambiguous trust root, and +## it has to be the one for this version: a stale file from an earlier cut +## signs just as well and would then be attached beside assets it never +## covered. mapfile -t sums < <(find "${dir}" -maxdepth 1 -type f -name '*-sha256sums.txt' | sort) (( ${#sums[@]} == 1 )) || fDie "expected exactly 1 *-sha256sums.txt in ${dir}, found ${#sums[@]}" 2 sumsfile="${sums[0]}" sigfile="${sumsfile}.sig" - -openssl dgst -sha256 -sign "${key}" -out "${sigfile}" "${sumsfile}" \ - || fDie "signing failed" - -## Verify what we just wrote, against the public half of the same key. -pub="$(mktemp)"; trap 'rm -f "${pub}"' EXIT +[[ "${sumsfile##*/}" == "shcl-${ver}-sha256sums.txt" ]] \ + || fDie "${sumsfile##*/} is not the sums file for ${ver} (Cargo.toml); rebuild the artifacts or fix the version" + +## Every entry has to match the file beside it. A sums file written before a +## rebuild is the same trap as a stale one: the signature would vouch for +## bytes nobody is shipping. +[[ -s "${sumsfile}" ]] || fDie "${sumsfile##*/} is empty" +(cd "${dir}" && sha256sum -c --quiet --strict "${sumsfile##*/}") \ + || fDie "${sumsfile##*/} does not match the files in ${dir}; rebuild the artifacts before signing" + +## The public half of the key, for the identity checks and the verify. +pub="$(mktemp)"; tmppub="$(mktemp)"; trap 'rm -f "${pub}" "${tmppub}"' EXIT openssl pkey -in "${key}" -pubout -out "${pub}" 2>/dev/null || fDie "cannot derive the public key" -openssl dgst -sha256 -verify "${pub}" -signature "${sigfile}" "${sumsfile}" >/dev/null 2>&1 \ - || fDie "wrote a signature that does not verify" ## The checks that actually catch mistakes: is this the key every shipped copy ## trusts? Signing with the wrong key produces a perfectly valid signature that @@ -125,7 +136,6 @@ openssl dgst -sha256 -verify "${pub}" -signature "${sigfile}" "${sumsfile}" >/de ## have to agree - the published .pub, the PEM inlined in install.bash, and the ## raw modulus inlined in install.ps1 - so any one of them drifting is caught ## here rather than by somebody's failed install. -tmppub="$(mktemp)"; trap 'rm -f "${pub}" "${tmppub}"' EXIT ## `|| true` inside the substitutions below: under pipefail a pipe that finds ## nothing fails the assignment and errexit ends the script with no message, so ## the guard after it would never run. @@ -158,6 +168,13 @@ if [[ -r "${root}/install.ps1" ]]; then [[ -n "${psmod}" && "${psmod}" == "${keymod}" ]] || fDie "install.ps1 carries a different key" fi +## Everything checked; sign, and verify what was written. A signature that +## does not verify is removed rather than left looking finished. +openssl dgst -sha256 -sign "${key}" -out "${sigfile}" "${sumsfile}" \ + || { rm -f "${sigfile}"; fDie "signing failed"; } +openssl dgst -sha256 -verify "${pub}" -signature "${sigfile}" "${sumsfile}" >/dev/null 2>&1 \ + || { rm -f "${sigfile}"; fDie "wrote a signature that does not verify"; } + printf 'signed %s\n' "${sumsfile##*/}" printf 'wrote %s\n' "${sigfile##*/}" printf 'key fp %s\n' "$(openssl pkey -in "${key}" -pubout 2>/dev/null | openssl pkey -pubin -outform DER -pubout 2>/dev/null | sha256sum | cut -d' ' -f1)" @@ -171,3 +188,5 @@ printf 'attach both the sums file and its .sig to the release.\n' ## - 2026-08-30 JC: The modulus and fingerprint guards are reachable again ## (a failed pipe used to end the script before them); help is framed ## with a blank line each side. +## - 2026-09-02 JC: Checks before the signature: the sums file's name and +## contents, then the key, and only then the write; a failed run leaves no .sig. diff --git a/project/backlog.md b/project/backlog.md index 30cc235..8f48099 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -173,9 +173,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191500 - Closed: 20260902-150000 - - 🔘 Item 17: `sign-release.bash` signs before it checks the key, and checks nothing about the sums file. + - ✅ Item 17: `sign-release.bash` signs before it checks the key, and checks nothing about the sums file. - Reproduced with a throwaway key: the run fails on the key check and leaves a valid-looking `.sig` beside the sums file. The tag check compares against `Cargo.toml` but never against the sums file's own name, and the sums are never verified against the files present, so a stale sums file from a rebuilt tree signs clean. + - Fixed: the signer checks before it writes: the sums file must be named `shcl--sha256sums.txt` for the `Cargo.toml` version, every entry must match the file beside it (`sha256sum -c --strict`), and the key must be the one all three shipped copies trust. Only then is the signature written, and one that fails its own verify is removed. + - Pinned by `shell-regress.bash`: a throwaway key against a correct sums file is refused with no `.sig` left, a stale entry and a sums file for another version are each refused before the key is looked at. The old signer failed all five checks. - Opened: 20260901-191600 + - Closed: 20260902-153000 - ✅ Item 18: the installer's "not on your PATH" note fires when the directory is on PATH with a trailing slash. - Reproduced. A string compare against `:dir:` misses `dir/`, which the shell resolves fine. Same shape in `install-dev.bash`. From 90f67d1c991901ba4231cfccff60cbb9f7fc7a7d Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:16:08 -0700 Subject: [PATCH 13/19] profiler workload loads clean --- cicd/config.bash | 14 ++++++++------ cicd/utility/include/largedoc-gen.bash | 8 ++++++-- cicd/utility/largedoc.bash | 11 ++++++++++- project/backlog.md | 5 ++++- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/cicd/config.bash b/cicd/config.bash index 0304aaf..09dfdf9 100644 --- a/cicd/config.bash +++ b/cicd/config.bash @@ -245,7 +245,9 @@ LARGEDOC_MIB=100 ## enough to sample meaningfully: the large-document generator at a few MiB. It ## used to be forty copies of the corpus concatenated, which made four lines in ## five merge into an existing node, so the graph measured merging and hint -## formatting rather than parsing. +## formatting rather than parsing. stderr is closed off on every run: a hint +## per unit once put a million lines in the log and a third of the samples in +## the write path. ## Kernel perf is locked down on this box (perf_event_paranoid=3), hence the ## in-process sampler. PROFILE_WORKLOAD_GEN / PROFILE_RUN are eval'd by the engine ## with PROFILE_WORKLOAD / PROFILE_OUT / PROFILE_SECS exported. @@ -255,18 +257,18 @@ PROFILE_BUILD_CMD=(cargo build --profile profiling --features profiling -j "${CP PROFILE_BIN="source/rust/target/profiling/${EXE_NAME}" PROFILE_OUT_DIR="cicd/artifacts/profiling" ## relative to repo root; gitignored PROFILE_WORKLOAD_GEN='source cicd/utility/include/largedoc-gen.bash; largedoc_gen 4 > "${PROFILE_WORKLOAD}"' -PROFILE_RUN='SHCL_PROFILE_OUT="${PROFILE_OUT}" SHCL_PROFILE_SECS="${PROFILE_SECS}" "${PROFILE_BIN}" fmt "${PROFILE_WORKLOAD}" >/dev/null' +PROFILE_RUN='SHCL_PROFILE_OUT="${PROFILE_OUT}" SHCL_PROFILE_SECS="${PROFILE_SECS}" "${PROFILE_BIN}" fmt "${PROFILE_WORKLOAD}" >/dev/null 2>&1' ## Wall-clock per surface, logged after the flamegraph: the graph shows where ## time goes inside fmt, these catch merge/validate/generate/set/read going ## quadratic without moving a sample. "name|command"; nonzero exit = FAILED ## (append `|| [ $? -eq N ]` where a nonzero exit is the expected outcome). PROFILE_TIMED=( - 'fmt|"${PROFILE_BIN}" fmt "${PROFILE_WORKLOAD}" >/dev/null' - 'merge|"${PROFILE_BIN}" fmt --layer="${PROFILE_WORKLOAD}" "${PROFILE_WORKLOAD}" >/dev/null' - 'reads|"${PROFILE_BIN}" instances "${PROFILE_WORKLOAD}" service >/dev/null && "${PROFILE_BIN}" count "${PROFILE_WORKLOAD}" service >/dev/null' + 'fmt|"${PROFILE_BIN}" fmt "${PROFILE_WORKLOAD}" >/dev/null 2>&1' + 'merge|"${PROFILE_BIN}" fmt --layer="${PROFILE_WORKLOAD}" "${PROFILE_WORKLOAD}" >/dev/null 2>&1' + 'reads|"${PROFILE_BIN}" instances "${PROFILE_WORKLOAD}" service >/dev/null 2>&1 && "${PROFILE_BIN}" count "${PROFILE_WORKLOAD}" service >/dev/null 2>&1' 'validate|"${PROFILE_BIN}" check --schema=project/conformance/021-schema-valid/schema.shcl "${PROFILE_WORKLOAD}" >/dev/null 2>&1 || [ $? -eq 6 ]' 'generate|"${PROFILE_BIN}" init --schema=project/conformance/026-init-schema/init-schema.shcl >/dev/null' - 'set|printf "int\tprofile.k\t1\nstring\tprofile.s\tv\nremove\tprofile.k\n" | "${PROFILE_BIN}" set "${PROFILE_WORKLOAD}" >/dev/null' + 'set|printf "int\tprofile.k\t1\nstring\tprofile.s\tv\nremove\tprofile.k\n" | "${PROFILE_BIN}" set "${PROFILE_WORKLOAD}" >/dev/null 2>&1' ) ## Stage 6: native release + cross targets. One per line: diff --git a/cicd/utility/include/largedoc-gen.bash b/cicd/utility/include/largedoc-gen.bash index 95d9153..1480aa4 100644 --- a/cicd/utility/include/largedoc-gen.bash +++ b/cicd/utility/include/largedoc-gen.bash @@ -9,7 +9,9 @@ ## one name, nesting, inline and bullet arrays, quoted values holding the ## separator, raw blocks, comments, blank lines, non-ASCII, and one array ## long enough to walk past any fixed element buffer. Nothing in it merges -## into an earlier line, so a profile of it measures parsing, not merging. +## into an earlier line, so a profile of it measures parsing, not merging, +## and it loads with no diagnostics at all (the large-document gate +## checks that; a hint per unit once made the profile measure stderr). ## History: At bottom of script. ## Copyright ÂĐ 2026 Bubbles (ID: XāŽŒāļ…რXÛģᛟԃლፀƅę“ĐāļŦÎīლჩ) @@ -38,7 +40,7 @@ largedoc_gen(){ "\t\tburst:\n" for (j = 0; j < 4; j++) s = s "\t\t\t* " ((j*i)%1000) "\n" s = s "\tnotes:\n\t\t~~~\n\t\tgenerated entry " i "\n\t\tsecond line\n\t\t~~~\n" \ - "service: svc" i "\n\tport: " (9000 + i%1000) "\n\n" + "service: svc" i "-b\n\tport: " (9000 + i%1000) "\n\n" printf "%s", s bytes += length(s) } @@ -56,3 +58,5 @@ declare -i isSourced_ldg7c=0; [[ "${BASH_SOURCE[0]}" == "${0}" ]] || isSourced_l ## History: ## - 2026-08-29 JC: Lifted out of largedoc.bash so the profiler runs the ## same document instead of forty concatenated copies of the corpus. +## - 2026-09-02 JC: The second instance of each service carries its own value; +## it used to reopen the first, so half the nodes merged and every unit hinted. diff --git a/cicd/utility/largedoc.bash b/cicd/utility/largedoc.bash index 9d0f84b..326f109 100755 --- a/cicd/utility/largedoc.bash +++ b/cicd/utility/largedoc.bash @@ -155,11 +155,20 @@ refOut="${work}/out-${refName}.shcl" if [[ -s "${refOut}" ]]; then refCli="${bindings[0]#*|}" echo - if "${refCli}" fmt "${refOut}" | cmp -s - "${refOut}"; then + if "${refCli}" fmt "${refOut}" 2>/dev/null | cmp -s - "${refOut}"; then echo "largedoc: fmt is a fixpoint at ${actualMib} MiB" else echo "largedoc: FAILED: fmt is not a fixpoint at ${actualMib} MiB" >&2; rc=1 fi + ## The generator promises a document nothing in it merges into and that + ## loads clean; the profiler's numbers depend on it. A hint per unit once + ## made the profile measure stderr. + summary="$("${refCli}" check "${doc}" 2>/dev/null | tail -1 || true)" + if [[ "${summary}" == "ok (0 diagnostic(s))" ]]; then + echo "largedoc: the generated document loads with no diagnostics" + else + echo "largedoc: FAILED: the generated document does not load clean: ${summary}" >&2; rc=1 + fi wideCount="$("${refCli}" get --int --array "${refOut}" wide | grep -c '' || true)" if ((wideCount == 20000)); then echo "largedoc: long array read back whole (${wideCount} elements)" diff --git a/project/backlog.md b/project/backlog.md index 8f48099..95db980 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -187,10 +187,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191700 - Closed: 20260902-150000 - - 🔘 Item 19: the profiler workload merges half its nodes and prints a hint per unit, so the profile measures hint formatting and stderr writes, and the run log carries a million hint lines. + - ✅ Item 19: the profiler workload merges half its nodes and prints a hint per unit, so the profile measures hint formatting and stderr writes, and the run log carries a million hint lines. - Reproduced: the generator's trailing `service: svcN` line reopens the block above it, so every unit's two `port` leaves collide. 37% of the profiled CPU is the unbuffered hint stream. Today's run log is 93 MB, and a million of its lines are `H001`, from the profiler stage and the large-document fixpoint check. - The generator's own header says nothing in it merges, and the config comment says it was introduced to stop measuring exactly this. + - Fixed: each unit's second `service` line carries its own value (`svcN-b`), so nothing merges and the document loads with no diagnostics; the profiler run and every timed workload close stderr as well as stdout, so the log no longer carries the hint stream. + - Pinned by the large-document gate, which now requires the generated document to load with exactly zero diagnostics (`ok (0 diagnostic(s))`). The old generator gives 6107 at 2 MiB. - Opened: 20260901-191800 + - Closed: 20260902-154500 - 🔘 Item 20: `flame-report.py` accepts a truncated or reshaped profile as a good one, and tracebacks on a non-UTF-8 file. - Reproduced: a profile cut off at 35 KB reports attribution summing to 9% at exit 0, and one with the row height changed reports 173% parse, both with the seen marker written. Random bytes give a decode traceback instead of the skip path. From e0c85115fd65acc15024b0594b06926224525fd9 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:18:47 -0700 Subject: [PATCH 14/19] flame-report refuses a partial graph --- cicd/utility/flame-report.py | 26 ++++++++++++++++++++++-- cicd/utility/shell-regress.bash | 36 +++++++++++++++++++++++++++++++++ project/backlog.md | 5 ++++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/cicd/utility/flame-report.py b/cicd/utility/flame-report.py index f417c4d..94191f7 100755 --- a/cicd/utility/flame-report.py +++ b/cicd/utility/flame-report.py @@ -25,7 +25,7 @@ from pathlib import Path from typing import NoReturn -STEP = 16 # flamegraph row height in the SVG, px (a child sits at parent_y - STEP) +STEP = 16.0 # flamegraph row height in the SVG, px, until fParse reads the real one off the rows SELF_TOP = 22 # self-time leaders to list INCL_TOP = 14 # inclusive-time buckets to list CHAIN = 4 # leaves whose caller chain we walk up to the root @@ -64,7 +64,14 @@ def fAttr(attrs: str, key: str) -> float | None: def fParse(path: Path) -> tuple[int, list[Frame]]: - text = path.read_text(encoding="utf-8") + ## Skip rather than trust anything that is not a whole graph: a file cut + ## off mid-write, or one whose rows are not where the row height says, + ## still yields a sample count and some frames, and the report it gave + ## summed to a fraction of the samples with the marker written. The row + ## height comes off the rows themselves; the root and the self-time sum + ## are the checks that a graph is all there. + global STEP + text = path.read_text(encoding="utf-8", errors="replace") m = re.search(r'total_samples="(\d+)"', text) total = int(m.group(1)) if m else 0 frames: list[Frame] = [] @@ -77,6 +84,17 @@ def fParse(path: Path) -> tuple[int, list[Frame]]: frames.append((name, x, y, w)) if not total or not frames: fSkip(f"could not parse a flamegraph out of {path}") + if not text.rstrip().endswith(""): + fSkip(f"{path} does not end in ; cut off mid-write") + ys = sorted({fr[2] for fr in frames}) + gaps = {round(b - a, 6) for a, b in zip(ys, ys[1:])} + if len(gaps) > 1: + fSkip(f"rows are not evenly spaced in {path} (gaps {sorted(gaps)}); not a whole flamegraph") + if gaps: + STEP = gaps.pop() + roots = [fr for fr in frames if fr[2] == ys[-1]] + if len(roots) != 1 or abs(roots[0][3] - total) > 1e-6: + fSkip(f"no single root frame spanning all {total} samples in {path}; not a whole flamegraph") return total, frames @@ -116,6 +134,8 @@ def parent(fr: Frame) -> Frame | None: return None selfOf = {fr: fr[3] - sum(c[3] for c in kids(fr)) for fr in frames} + if abs(sum(selfOf.values()) - total) > 1 or min(selfOf.values()) < -1e-6: + fSkip(f"self time sums to {sum(selfOf.values()):.0f} of {total} samples; frames are missing or misplaced, not a whole flamegraph") selfBy: dict[str, float] = {} inclBy: dict[str, float] = {} @@ -238,3 +258,5 @@ def main() -> None: ## History: ## - 20260712: Created from the SilkTerm sibling; attribution buckets redone for shcl (parse / emit / reads). ## - 20260829: Regexes compiled once; rows indexed and bisected instead of scanned per frame. +## - 2026-09-02 JC: A truncated or reshaped graph skips at exit 2 instead of +## reporting a fraction of it; the row height is read off the rows. diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index ad5a7d9..d04f55a 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -175,6 +175,42 @@ else echo "shell-regress: openssl not installed - signing rows skipped" fi +## 20260901b item 20: flame-report.py took any file with a sample count and one +## frame for a whole flamegraph, so a profile cut off mid-write reported a +## fraction of itself at exit 0 and recorded the marker; a row height other +## than the constant it assumed double-counted; a non-UTF-8 file was a +## traceback. Three frames are a whole graph; a graph missing the frame +## between a leaf and the root, one cut off before its closing tag, and +## random bytes must each skip at 2. +fFlame(){ ## fFlame FILE STEP FRAMES...: a flamegraph of 100 samples with rows STEP apart + local file="$1" step="$2"; shift 2 + { + printf '\n' + printf 'all (100 samples, 100%%)\n' "$((step * 3))" + for fr in "$@"; do + IFS='|' read -r fname fx fy fw <<<"${fr}" + printf '%s (%s samples)\n' "${fname}" "${fw}" "$((step * fy))" "${fx}" "${fw}" + done + printf '\n' + } > "${file}" +} +mkdir -p "${tmpDir}/flame" +fFlame "${tmpDir}/flame/flame_20260101-000000_whole.svg" 16 'shcl::Parser::parse|0|2|60' 'shcl::Document::to_canonical|60|2|40' 'shcl::scan_path|0|1|10' +fFlame "${tmpDir}/flame/flame_20260101-000000_tall.svg" 24 'shcl::Parser::parse|0|2|60' 'shcl::Document::to_canonical|60|2|40' 'shcl::scan_path|0|1|10' +fFlame "${tmpDir}/flame/flame_20260101-000000_gap.svg" 16 'shcl::Document::to_canonical|60|2|40' 'shcl::scan_path|0|1|10' +head -c 200 "${tmpDir}/flame/flame_20260101-000000_whole.svg" > "${tmpDir}/flame/flame_20260101-000000_cut.svg" +head -c 1000 /dev/urandom > "${tmpDir}/flame/flame_20260101-000000_junk.svg" +fFlameRun(){ flameRc=0; flameOut="$(python3 "${repoDir}/cicd/utility/flame-report.py" --file "$1" 2>&1)" || flameRc=$?; } +fFlameRun "${tmpDir}/flame/flame_20260101-000000_whole.svg" +[[ "${flameRc}" == 0 && "${flameOut}" == *"parse (tokenize/merge/diags) .: 60.0%"* ]] || fBad "flame-report.py misread a whole graph (rc ${flameRc}): ${flameOut@Q}" +fFlameRun "${tmpDir}/flame/flame_20260101-000000_tall.svg" +[[ "${flameRc}" == 0 && "${flameOut}" == *"parse (tokenize/merge/diags) .: 60.0%"* && "${flameOut}" == *"other ........................: 0.0%"* ]] || fBad "flame-report.py misread a graph with a different row height (rc ${flameRc}): ${flameOut@Q}" +for bad in gap cut junk; do + fFlameRun "${tmpDir}/flame/flame_20260101-000000_${bad}.svg" + [[ "${flameRc}" == 2 ]] || fBad "flame-report.py accepted a ${bad} graph (rc ${flameRc}): ${flameOut@Q}" + [[ "${flameOut}" != *Traceback* ]] || fBad "flame-report.py tracebacked on a ${bad} graph" +done + ## 20260830b item 9: the stable channel took GitHub's date-ordered "latest ## release" verbatim, so a patch back-ported to an older line after a newer one ## shipped was handed out as stable. The fixture is in publish order, newest diff --git a/project/backlog.md b/project/backlog.md index 95db980..f90f4c7 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -195,10 +195,13 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191800 - Closed: 20260902-154500 - - 🔘 Item 20: `flame-report.py` accepts a truncated or reshaped profile as a good one, and tracebacks on a non-UTF-8 file. + - ✅ Item 20: `flame-report.py` accepts a truncated or reshaped profile as a good one, and tracebacks on a non-UTF-8 file. - Reproduced: a profile cut off at 35 KB reports attribution summing to 9% at exit 0, and one with the row height changed reports 173% parse, both with the seen marker written. Random bytes give a decode traceback instead of the skip path. - Cause: any file with a sample count and one frame passes, and the row step is a hard-coded constant that nothing verifies. + - Fixed: the report skips at exit 2, with no marker written, unless the file ends in ``, its rows are evenly spaced, exactly one root frame spans every sample, and the self times sum to the sample count. The row height is read off the rows instead of assumed, and the file is decoded with replacement so bytes that are not UTF-8 skip like any other bad input. + - Pinned by `shell-regress.bash` with synthetic graphs: a whole one and one at another row height report 60/40 with nothing in `other`; a graph missing the frame between a leaf and the root, one cut off before its closing tag, and random bytes each skip at 2 with no traceback. The old script accepted the first two and tracebacked on the third. - Opened: 20260901-191900 + - Closed: 20260902-160000 - 🔘 Item 21: `lint-report.bash` flags the nested pre-push gate's own plan line. - Reproduced on today's log. The line it reports is the `-D warnings` in the clippy command the pre-push hook echoes during the publish stage, which is why only runs that push to dev or main show a warning. The cppcheck `--enable=warning` echo is already excluded; this one is not. From cfa94978753b3530d7259722fb2a0efd225521f1 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:19:29 -0700 Subject: [PATCH 15/19] lint-report skips the echoed clippy line --- cicd/utility/lint-report.bash | 7 +++++-- cicd/utility/shell-regress.bash | 11 +++++++++++ project/backlog.md | 5 ++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cicd/utility/lint-report.bash b/cicd/utility/lint-report.bash index 1fe31be..8e46eae 100755 --- a/cicd/utility/lint-report.bash +++ b/cicd/utility/lint-report.bash @@ -85,10 +85,12 @@ fi ## run's log (a passing run aborts on the first error). Drop the "0 warnings" noise, ## govulncheck's clean-result prose (its "found N vulnerabilities in modules you ## require, but your code doesn't appear to call" block is informational and prints -## on every run), and the echoed cppcheck command line, which carries the word. +## on every run), and the echoed command lines that carry the word: cppcheck's +## `--enable=warning`, and clippy's `-D warnings`, which the pre-push gate's +## nested run echoes during publish and which used to read as a finding. warns="$(grep -inE 'warning|rustsec-|vulnerab|unmaintained|yanked|error\[' "$log" 2>/dev/null \ | grep -viE 'generated 0 warnings|: 0 warnings|no warnings|0 warnings emitted' \ - | grep -viE 'no vulnerabilities found|affected by 0 vulnerabilities|this scan also found|appear to call|^[0-9]+:these vulnerabilities\.$|enable=warning' || true)" + | grep -viE 'no vulnerabilities found|affected by 0 vulnerabilities|this scan also found|appear to call|^[0-9]+:these vulnerabilities\.$|enable=warning|-D warnings' || true)" if [[ -n "$warns" ]]; then n=$(printf '%s\n' "$warns" | grep -c .); else n=0; fi tag="FLAG"; ((check)) && tag="NEW" @@ -103,3 +105,4 @@ fi ## Script history: ## - 20260709: Created. +## - 20260902: The echoed `-D warnings` of a nested clippy run is not a finding. diff --git a/cicd/utility/shell-regress.bash b/cicd/utility/shell-regress.bash index d04f55a..9e04416 100755 --- a/cicd/utility/shell-regress.bash +++ b/cicd/utility/shell-regress.bash @@ -211,6 +211,17 @@ for bad in gap cut junk; do [[ "${flameOut}" != *Traceback* ]] || fBad "flame-report.py tracebacked on a ${bad} graph" done +## 20260901b item 21: lint-report.bash counted the `-D warnings` in the clippy +## command line the pre-push gate's nested run echoes as a warning, so every +## run that pushed to dev read as one finding. A real clippy warning and a +## cppcheck one still count; the two echoed command lines do not. +printf 'Lint ...........: cargo clippy --all-targets -- -D warnings\nLint ...........: cppcheck --enable=warning,portability src.c\nOK: lint\n' > "${tmpDir}/run_20260101-000000.log" +lintOut="$(bash "${repoDir}/cicd/utility/lint-report.bash" --file "${tmpDir}/run_20260101-000000.log" 2>&1 || true)" +[[ "${lintOut}" == "CLEAN "* ]] || fBad "lint-report.bash counted an echoed command line as a warning: ${lintOut@Q}" +printf 'warning: unused variable: x\n --> src/main.rs:1:1\nsrc.c:12:3: warning: uninitialized variable [uninitvar]\n' >> "${tmpDir}/run_20260101-000000.log" +lintOut="$(bash "${repoDir}/cicd/utility/lint-report.bash" --file "${tmpDir}/run_20260101-000000.log" 2>&1 || true)" +[[ "${lintOut}" == "FLAG "*"(2 warning line(s))"* ]] || fBad "lint-report.bash missed a real warning: ${lintOut@Q}" + ## 20260830b item 9: the stable channel took GitHub's date-ordered "latest ## release" verbatim, so a patch back-ported to an older line after a newer one ## shipped was handed out as stable. The fixture is in publish order, newest diff --git a/project/backlog.md b/project/backlog.md index f90f4c7..253c7ee 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -203,9 +203,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-191900 - Closed: 20260902-160000 - - 🔘 Item 21: `lint-report.bash` flags the nested pre-push gate's own plan line. + - ✅ Item 21: `lint-report.bash` flags the nested pre-push gate's own plan line. - Reproduced on today's log. The line it reports is the `-D warnings` in the clippy command the pre-push hook echoes during the publish stage, which is why only runs that push to dev or main show a warning. The cppcheck `--enable=warning` echo is already excluded; this one is not. + - Fixed: the echoed `-D warnings` is excluded the way the cppcheck `--enable=warning` echo already was. Today's log reads CLEAN. + - Pinned by `shell-regress.bash`: a log holding both echoed command lines is CLEAN, and one with a clippy and a cppcheck warning appended is FLAG with exactly two lines. The old script counted three. - Opened: 20260901-192000 + - Closed: 20260902-161500 - 🔘 Item 22: the C++ veneer's `to_canonical()` never gives the read arena back, so a save loop grows without bound. - Measured: 1000 calls on a 2.9 MB document, 94 MB to 2.9 GB. Every other copying wrapper releases before its call; this one was missed. From 16d97373de5f7f3439582c46873ae19f59805544 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:20:07 -0700 Subject: [PATCH 16/19] veneer releases before to_canonical --- project/backlog.md | 5 ++++- source/c/shcl.hpp | 5 ++++- source/c/tests/veneer_smoke.cpp | 10 ++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/project/backlog.md b/project/backlog.md index 253c7ee..7a64fe4 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -210,9 +210,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-192000 - Closed: 20260902-161500 - - 🔘 Item 22: the C++ veneer's `to_canonical()` never gives the read arena back, so a save loop grows without bound. + - ✅ Item 22: the C++ veneer's `to_canonical()` never gives the read arena back, so a save loop grows without bound. - Measured: 1000 calls on a 2.9 MB document, 94 MB to 2.9 GB. Every other copying wrapper releases before its call; this one was missed. + - Fixed: `to_canonical()` releases the read arena before its call, like every other copying wrapper. + - Pinned by `veneer_smoke.cpp`: 200 `to_canonical()` calls on a 30 KB document leave the read arena holding at most two copies. The old veneer holds all 200. - Opened: 20260901-192100 + - Closed: 20260902-162000 - 🔘 Item 23: Go's two suppress filters return the caller's slice when nothing is disavowed, against their own comment, and `Diagnostics()` hands out the live internal slice. - Reproduced: the returned slice shares its backing array, so an append by the caller and the document's own next append overwrite each other. The in-place-filter bug fixed on 20260831 was the drop path; this is its sibling on the keep-everything path, which the test for it does not cover. diff --git a/source/c/shcl.hpp b/source/c/shcl.hpp index 10c5707..095e8d5 100644 --- a/source/c/shcl.hpp +++ b/source/c/shcl.hpp @@ -164,7 +164,10 @@ class Document { bool strict_failed() const { return shcl_strict_failed(d_.get()) != 0; } Strictness strictness() const { return static_cast(shcl_strictness_of(d_.get())); } - std::string to_canonical() const { return to_str(shcl_to_canonical(d_.get())); } + // The canonical text lives in the read arena like every other result, so + // it is released first the way the reads below are: a save loop otherwise + // holds every copy until the Document goes. + std::string to_canonical() const { shcl_reads_release(d_.get()); return to_str(shcl_to_canonical(d_.get())); } std::vector diagnostics() const { std::vector v; std::size_t n = shcl_diag_count(d_.get()); diff --git a/source/c/tests/veneer_smoke.cpp b/source/c/tests/veneer_smoke.cpp index 645787b..df1b936 100644 --- a/source/c/tests/veneer_smoke.cpp +++ b/source/c/tests/veneer_smoke.cpp @@ -261,6 +261,16 @@ int main() { std::size_t held_bytes = 0; for (const ShclBlock *b = raw->reads.head; b; b = b->next) held_bytes += b->used; CHECK(held_bytes <= 4096); + // The canonical text is a read result too. 200 copies of a 30 KB + // document held at once would be 6 MB; released per call it is one. + std::string big; + for (int i = 0; i < 2000; i++) big += "key" + std::to_string(i) + ": value" + std::to_string(i) + "\n"; + shcl_doc *rawBig = shcl_parse(big.data(), big.size()); + shcl::Document heldBig(rawBig); + for (int i = 0; i < 200; i++) CHECK(heldBig.to_canonical().size() == big.size()); + held_bytes = 0; + for (const ShclBlock *b = rawBig->reads.head; b; b = b->next) held_bytes += b->used; + CHECK(held_bytes <= 2 * big.size()); } if (fails) { std::fprintf(stderr, "veneer: %d failure(s)\n", fails); return 1; } From 35cc428299740d74830cd38c1de72af701351ffa Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:20:47 -0700 Subject: [PATCH 17/19] go diagnostics are copies --- project/backlog.md | 5 ++++- source/go/shcl.go | 9 ++++++--- source/go/shcl_test.go | 16 ++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/project/backlog.md b/project/backlog.md index 7a64fe4..d494a44 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -217,9 +217,12 @@ Every item carries the date it was opened and, once settled, the date it closed. - Opened: 20260901-192100 - Closed: 20260902-162000 - - 🔘 Item 23: Go's two suppress filters return the caller's slice when nothing is disavowed, against their own comment, and `Diagnostics()` hands out the live internal slice. + - ✅ Item 23: Go's two suppress filters return the caller's slice when nothing is disavowed, against their own comment, and `Diagnostics()` hands out the live internal slice. - Reproduced: the returned slice shares its backing array, so an append by the caller and the document's own next append overwrite each other. The in-place-filter bug fixed on 20260831 was the drop path; this is its sibling on the keep-everything path, which the test for it does not cover. + - Fixed: both suppressors copy on the keep-everything path, and `Diagnostics()` returns a copy of the document's list, so nothing handed out shares a backing array with the document. + - Pinned by `TestSuppressLeavesTheCallersDiagnosticsAlone`, extended: with a schema that disavows nothing, each suppressor's result and `Diagnostics()` itself must not point at the caller's or the document's array. Fails on the old code. - Opened: 20260901-192200 + - Closed: 20260902-163000 ### Features and enhancements diff --git a/source/go/shcl.go b/source/go/shcl.go index b2d861c..31ea3c6 100644 --- a/source/go/shcl.go +++ b/source/go/shcl.go @@ -2515,7 +2515,10 @@ func ParseLimited(text string, strictness Strictness, maxNodes, maxElements, max // Diagnostics is everything the load recorded (after LoadAndValidate, // validation findings too). func (d *Document) Diagnostics() []Diagnostic { - return d.diags + // A copy: the reference hands out a borrowed view nobody can append to, + // and a Go slice sharing the document's backing array would let a + // caller's append and the document's next one overwrite each other. + return append([]Diagnostic(nil), d.diags...) } // LostCount is how many lines or values parsing dropped that canonical @@ -2795,7 +2798,7 @@ func h001Head(name string) string { func SuppressDeclaredRepeats(schema *Document, diags []Diagnostic) []Diagnostic { names := disavowedNames(schema, func(c *constraint) bool { return c.repeat != nil && c.repeat[1] > 1 }) if len(names) == 0 { - return diags + return append([]Diagnostic(nil), diags...) } heads := make([]string, len(names)) for i, n := range names { @@ -3156,7 +3159,7 @@ func h002Head(name string) string { func SuppressDeclaredReopens(schema *Document, diags []Diagnostic) []Diagnostic { names := disavowedNames(schema, func(c *constraint) bool { return c.reopen }) if len(names) == 0 { - return diags + return append([]Diagnostic(nil), diags...) } heads := make([]string, len(names)) for i, n := range names { diff --git a/source/go/shcl_test.go b/source/go/shcl_test.go index 6412116..f7c9829 100644 --- a/source/go/shcl_test.go +++ b/source/go/shcl_test.go @@ -1551,6 +1551,22 @@ func TestSuppressLeavesTheCallersDiagnosticsAlone(t *testing.T) { if len(SuppressDeclaredRepeats(schema, diags)) != 1 { t.Fatal("the filter dropped no hint, so nothing above was proved") } + // The keep-everything path and Diagnostics() itself have to hand out + // their own backing array too: shared with the document's, a caller's + // append and the document's next append land in the same slot. + none := Parse("field: other\n") + for name, kept := range map[string][]Diagnostic{ + "SuppressDeclaredRepeats": SuppressDeclaredRepeats(none, diags), + "SuppressDeclaredReopens": SuppressDeclaredReopens(none, diags), + "Diagnostics": doc.Diagnostics(), + } { + if len(kept) != len(diags) { + t.Fatalf("%s: want %d diagnostics kept, got %d", name, len(diags), len(kept)) + } + if &kept[0] == &diags[0] || &kept[0] == &doc.diags[0] { + t.Fatalf("%s: returned the caller's or the document's own backing array", name) + } + } } func TestConvenienceTierFallsBackOnlyOnGood(t *testing.T) { From 146942058c10c74527d054e859a18de713c93b88 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:21:59 -0700 Subject: [PATCH 18/19] changelog, design note --- changelog.md | 26 ++++++++++++++++++++++++++ project/design.md | 6 ++++++ 2 files changed, 32 insertions(+) diff --git a/changelog.md b/changelog.md index f1cee0f..4fd2f08 100755 --- a/changelog.md +++ b/changelog.md @@ -72,6 +72,32 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- A line whose indent matches no open level (`E012`) and a `*` line with no space after it (`E013`) now hold their indent level, so what is written under them is skipped with them (`E018`) instead of attaching one level up, a fence line at a bad indent takes its whole body with it instead of parsing it as top-level bindings, and a second line at the same bad indent is refused the same way rather than binding. + +- A value after an index selector on the last segment (`a[0]: 2`) was dropped with no diagnostic and no lost count, so an in-place write deleted it at exit 0. It is reported (`E002`) and counted as lost now, as a value after a value selector always was. + +- A fragment mounted at one node by two schema paths reported every fault under it twice. + +- The `H001`/`H002` hints a schema disavows were matched on the schema's raw text, so a field path with an escaped quote in it kept its hint, and a `repeat` or `reopen` that faulted (`repeat: 0x2`, three elements) still silenced it. Both now go by the built schema. + +- A merge appended a layer's unmatched nodes grouped by name instead of in that file's order, and dropped a footer comment the layer repeated itself. Merging onto an empty document is the identity again. + +- The did-you-mean suggestion cost a full edit-distance table per name pair, so a schema and document with long field names took seconds to minutes to check. The distance is capped at the threshold and computed within that band, so it is linear in the name length. + +- Every binding spells a float the same way. C printed 17 digits for 46 exact powers of two where 16 read back, and the reference rounded an exact tie between two shortest spellings away from zero where Go, Python and C round to even (`2.9802322387695312e-08` came back as `...313` from one and `...312` from the other three). Ties round to even everywhere now. + +- On Windows the CLI aborted when the program reading its output closed early (`fmt` piped into `more`); it exits quietly now, as it dies quietly of SIGPIPE elsewhere. + +- The C CLI on Windows took its arguments in the active code page, so a path outside it was refused and a name the page best-fits (`ā.shcl` to `a.shcl`) reached the wrong file, `--write` included. It reads the wide command line now, and the header says paths are UTF-8 on every platform. A failed publish on Windows also left errno at 0, so the CLI printed `Success` beside its failure exit; the Win32 error is mapped onto errno now. + +- The `.deb` and `.rpm` declared no dependencies, so they installed on a system whose glibc is older than the binary needs and the binary then failed to load. They declare the glibc floor and libgcc read off the binary, and the deb carries its copyright and changelog files. + +- A system install under a tight umask left a bin or man1 directory the installer had to create root-only; the "not on your PATH" note fired when the directory was on PATH with a trailing slash. + +- The C++ veneer's `to_canonical()` never gave the read memory back, so a save loop grew without bound. + +- Go's `Diagnostics()` and both suppress filters could hand back a slice sharing the document's own backing array. + - A file of lines with no colon at a constant indent parsed in quadratic time - a 1 MB plain text file took half a minute, and neither `ParseLimited` cap could stop it because no nodes or elements were built. Each refused line is kept as trivia, and every following line rewalked the whole retained list. The list is walked only as far as an incoming line could change it now, so the parse is linear again. - `ParseLimited`'s element cap bounded nothing for an inline array: the line was built in full and refused afterwards, so 9 MB of input peaked at the same 256 MB with the cap as without, and in C the refused array stayed held for the document's lifetime. The count is taken before anything splits the value now, so a refused line costs its text and no more. diff --git a/project/design.md b/project/design.md index f7f9b5e..fb80fa1 100644 --- a/project/design.md +++ b/project/design.md @@ -319,6 +319,12 @@ Structure-only canonicalizer: block form, tabs, insertion order, minimal quoting - It was decided that merge adopts the parser's own empty-fill rule, so a merge and a parse of the concatenation agree. The fill is limited to raw blocks because that is the limit of the parser's rule: a valued instance still appends. +**A float is written with the fewest digits that read back, and an exact tie between two such spellings rounds to even.** Shortest-round-trip formatters agree on every double except two cases, and both had leaked into the output: at a power of two the rounding interval is lopsided, so the closest short spelling can fall outside it while its neighbor reads back, and on an exact tie between two spellings of the shortest length Rust's formatter rounds away from zero where Go's, Python's and glibc's round to even. + +- We decided on round to even: it is IEEE 754's own tie rule, what three of the four bindings already did, and what `repr` in Python and `strconv` in Go print, so a value read from another tool's output spells the same here. The reference takes the correctly rounded spelling of the shortest length whenever it reads back, and keeps its own shortest spelling only when it does not. + +- C tries the last-digit neighbors before adding a digit, which is what the shortest-digits algorithms find at a power of two. Every power of two and a fixed set of random doubles go through a float write in every binding in the cross-binding check, so a formatter that drifts on either case is caught there. + ### Saving a file - **A save publishes a new file in the old one's place.** Write a temp file beside the target, then move it over. That is what makes an interrupted save unable to truncate a config, and it is also the source of every limitation below: the bytes are new, so anything the old file carried outside its contents has to be deliberately carried across or it is gone. From d5d399ff62f44ed4b77f61cd3bca6c07dd3a63b4 Mon Sep 17 00:00:00 2001 From: Jim Collier <32471972+jim-collier@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:19:35 -0700 Subject: [PATCH 19/19] c float read-back without strtod --- project/backlog.md | 2 +- source/c/shcl.h | 78 +++++++++++++++++++++++++++++++++++++++++++--- style-guide.md | 2 ++ 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/project/backlog.md b/project/backlog.md index d494a44..1452575 100644 --- a/project/backlog.md +++ b/project/backlog.md @@ -147,7 +147,7 @@ Every item carries the date it was opened and, once settled, the date it closed. - Cause: the wide publish calls set the Win32 last error and nothing maps it to `errno`, against what the header promises. - Fixed: a failed publish maps `GetLastError` onto errno (`EACCES` for a sharing or lock violation, `ENOENT`, `EEXIST`, `ENOSPC`, `EINVAL`, `EIO` for the rest), and the temp-file unlink on the failure path no longer overwrites the errno the failure left, on either platform. - Pinned by a windows-only fixture in the C runner: a write over a target held open without delete sharing, and one to a device name, both fail with a non-zero errno. Fails on the old header, passes now. - - Note: wine's `strtod` (and mingw's `__mingw_strtod`) read `7.67844768714563e-239` one ulp high where glibc, UCRT and Python agree, so under wine the C runner fails corpus `080` in the float formatter's read-back test. The C binding is only as exact as its C runtime's `strtod`; the hosted job decides what real windows does. + - Note: wine's `strtod`, mingw's `__mingw_strtod` and the hosted windows runner's C runtime all read `7.67844768714563e-239` one ulp high where glibc and Python agree, which failed corpus `080` on the C runner there. The formatter's read-back test is now integer arithmetic on the double's rounding interval (`f64_interval` / `f64_reads_back`), agreeing with glibc on 20 million spellings, so the C spelling no longer depends on the libc. The parser's float reads still go through `strtod`; recorded as a deviation in `style-guide.md`. - Opened: 20260901-191200 - Closed: 20260902-140000 diff --git a/source/c/shcl.h b/source/c/shcl.h index ee08562..daa8634 100644 --- a/source/c/shcl.h +++ b/source/c/shcl.h @@ -4120,13 +4120,78 @@ shcl_str shcl_to_canonical(shcl_doc *d) { // --- format helpers + remaining public API ---------------------------------- +// Whether a decimal spelling reads back as exactly v, decided with integer +// arithmetic rather than strtod: more than one C runtime (msvcrt, and wine's) +// parses some 15-digit spellings one ulp off, and a spelling that only reads +// back on a correct libc is not a spelling every binding agrees on. A double +// is m * 2^e; a decimal d * 10^k reads back when it sits inside v's rounding +// interval, half a spacing each way, except below a power of two where the +// spacing halves. A midpoint reads back only when m is even (ties to even). +// Both ends are scaled to integers by 2^S * 10^T once per value; a candidate +// then costs one small multiply and a shift before the compare. +typedef struct { uint32_t w[96]; int n; } ShclBig; +static void big_set(ShclBig *b, uint64_t v) { memset(b, 0, sizeof *b); b->w[0] = (uint32_t)v; b->w[1] = (uint32_t)(v >> 32); b->n = b->w[1] ? 2 : (b->w[0] ? 1 : 0); } +static void big_mul_small(ShclBig *b, uint32_t m) { + uint64_t carry = 0; + for (int i = 0; i < b->n; i++) { uint64_t t = (uint64_t)b->w[i] * m + carry; b->w[i] = (uint32_t)t; carry = t >> 32; } + if (carry && b->n < (int)(sizeof b->w / sizeof b->w[0])) b->w[b->n++] = (uint32_t)carry; +} +static void big_mul_pow10(ShclBig *b, int t) { + static const uint32_t p10[9] = { 1u, 10u, 100u, 1000u, 10000u, 100000u, 1000000u, 10000000u, 100000000u }; + for (; t >= 9; t -= 9) big_mul_small(b, 1000000000u); + if (t > 0 && t < 9) big_mul_small(b, p10[t]); +} +static void big_shl(ShclBig *b, int bits) { + int limbs = bits / 32, cap = (int)(sizeof b->w / sizeof b->w[0]); + if (limbs && b->n) { + if (b->n + limbs > cap) limbs = cap - b->n; + memmove(b->w + limbs, b->w, (size_t)b->n * sizeof b->w[0]); + memset(b->w, 0, (size_t)limbs * sizeof b->w[0]); + b->n += limbs; + } + if (bits % 32) big_mul_small(b, 1u << (bits % 32)); +} +static int big_cmp(const ShclBig *a, const ShclBig *b) { + if (a->n != b->n) return a->n < b->n ? -1 : 1; + for (int i = a->n; i-- > 0;) if (a->w[i] != b->w[i]) return a->w[i] < b->w[i] ? -1 : 1; + return 0; +} +typedef struct { ShclBig lo, hi; int S, T, even; } ShclF64Interval; +// exp10 is the decimal exponent of v's 17-digit spelling: the smallest k any +// shorter spelling can carry is exp10 - 16, which fixes T for all of them. +static void f64_interval(double v, int exp10, ShclF64Interval *iv) { + uint64_t bits; memcpy(&bits, &v, sizeof bits); + int E = (int)((bits >> 52) & 0x7FF); uint64_t F = bits & 0xFFFFFFFFFFFFFull; + uint64_t m = E ? (F | (1ull << 52)) : F; int e = E ? E - 1075 : -1074; + int above = e - 1, below = (E > 1 && F == 0) ? e - 2 : e - 1; // log2 of each half-width + iv->S = below < 0 ? -below : 0; + iv->T = exp10 - 16 < 0 ? 16 - exp10 : 0; + iv->even = (m & 1) == 0; + // v -+ 2^x = (m * 2^(e-x) -+ 1) * 2^x, and e - x is 1 or 2. + big_set(&iv->lo, (m << (e - below)) - 1); big_shl(&iv->lo, below + iv->S); big_mul_pow10(&iv->lo, iv->T); + big_set(&iv->hi, (m << (e - above)) + 1); big_shl(&iv->hi, above + iv->S); big_mul_pow10(&iv->hi, iv->T); +} +static int f64_reads_back(const char *tmp, const ShclF64Interval *iv) { + // The spelling: digits then an exponent, sign already known to match v. + const char *s = tmp; if (*s == '-' || *s == '+') s++; + uint64_t d = 0; int nd = 0; + for (; *s && *s != 'e' && *s != 'E'; s++) if (*s >= '0' && *s <= '9') { d = d * 10 + (uint64_t)(*s - '0'); nd++; } + if (!nd || *s == '\0') return 0; + int k = atoi(s + 1) - (nd - 1); + if (k + iv->T < 0) return 0; + ShclBig D; big_set(&D, d); big_mul_pow10(&D, k + iv->T); big_shl(&D, iv->S); + int cl = big_cmp(&D, &iv->lo), ch = big_cmp(&D, &iv->hi); + if (cl > 0 && ch < 0) return 1; + return (cl == 0 || ch == 0) && iv->even; +} + // The correctly rounded string at a precision is the closest one, but at a // power of two the rounding interval is lopsided, and the neighbor one digit // up or down can read back while the closest does not. The shortest-digits // algorithms the other bindings use find it; stepping the last digit of the // "%.*e" text in tmp by delta (with carry) and reading it back does the same. // A carry past the leading digit is a shorter spelling, already tried. -static int f64_neighbor(const char *tmp, double v, int delta, char *out) { +static int f64_neighbor(const char *tmp, const ShclF64Interval *iv, int delta, char *out) { strcpy(out, tmp); char *e = strchr(out, 'e'); if (!e || e == out) return 0; @@ -4141,7 +4206,7 @@ static int f64_neighbor(const char *tmp, double v, int delta, char *out) { p--; } if (*(out[0] == '-' ? out + 1 : out) == '0') return 0; - return strtod(out, NULL) == v; + return f64_reads_back(out, iv); } size_t shcl_format_f64(double v, char *out) { @@ -4149,12 +4214,15 @@ size_t shcl_format_f64(double v, char *out) { if (isinf(v)) { if (v < 0) { memcpy(out, "-inf", 4); return 4; } memcpy(out, "inf", 3); return 3; } if (v == 0.0) { if (signbit(v)) { memcpy(out, "-0", 2); return 2; } out[0] = '0'; return 1; } char tmp[64], alt[64]; int prec; + // A locale's decimal point is whatever it is; the digit walks skip it. + ShclF64Interval iv; + snprintf(tmp, sizeof tmp, "%.16e", v); + f64_interval(v, atoi(strchr(tmp, 'e') + 1), &iv); for (prec = 1; prec <= 17; prec++) { snprintf(tmp, sizeof tmp, "%.*e", prec - 1, v); - if (strtod(tmp, NULL) == v) break; - if (f64_neighbor(tmp, v, 1, alt) || f64_neighbor(tmp, v, -1, alt)) { memcpy(tmp, alt, sizeof tmp); break; } + if (f64_reads_back(tmp, &iv)) break; + if (f64_neighbor(tmp, &iv, 1, alt) || f64_neighbor(tmp, &iv, -1, alt)) { memcpy(tmp, alt, sizeof tmp); break; } } - // The round-trip above needed tmp in the host locale; the scan below wants '.'. { const char *dp = dec_point(); size_t dn = strlen(dp); if (dn != 1 || *dp != '.') { diff --git a/style-guide.md b/style-guide.md index b428fb9..a230307 100644 --- a/style-guide.md +++ b/style-guide.md @@ -109,6 +109,8 @@ New bindings (Tier 3) follow the same recipe: port the reference function-for-fu - Deliberate deviation: an allocation failure inside a parse or a validate unwinds and the call returns NULL, where the other three abort. Their languages abort on allocation failure and there is nothing there to mirror, while a C consumer embedding the header has a process that is not the library's to end. Everywhere else - a read, a write, a merge on a document already built - the `SHCL_OOM()` hook is still the answer. +- Deliberate deviation: the float formatter decides whether a spelling reads back with its own integer arithmetic, not `strtod`. The other three have a shortest-digits formatter in their runtime; C has `printf` and `strtod`, and more than one C runtime (msvcrt, wine's) parses some 15-digit spellings one ulp off, which made the C spelling of a value depend on the libc it was built against. The parser's float reads still go through `strtod`, so a value read on such a runtime can be one ulp off; the header cannot fix a libc. + - Deliberate deviation: `shcl_compact` has no counterpart in the other three. A write lands in the document's bump arena and the value it replaced stays there until `shcl_free`, so a process rewriting one field in a loop grows by a few dozen bytes per write; the other three reclaim the old value through their runtimes. Compaction rebuilds the document into fresh arenas, carrying the diagnostics, the lost count and the strictness with it, so a save or a strict gate afterwards reads the same. The C++ veneer exposes it as `compact()`. - Deliberate deviation: `shcl_reads_release` has no counterpart in the other three. Read results are copied into the document's read arena, which only `shcl_free` reclaims - the right default for a read-once consumer and the documented contract. A process polling one document in a loop needs a way out, and the other three bindings have one for free because they hand back owned collections their runtime reclaims. The C++ veneer calls it on every read, since it copies each result into owned std types the moment it gets it.