From e9bb89f044a01d7590864187dee03c09df48c7a6 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Mon, 28 Sep 2026 00:43:20 +0200 Subject: [PATCH] Correct and trim the comments around the path walks --- README.md | 2 +- docs/decisions.md | 6 +++--- family.go | 4 ++-- hold.go | 3 +-- holdfence.go | 4 ++-- node.go | 2 +- path.go | 24 ++++++++++-------------- render.go | 10 +++------- 8 files changed, 23 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index b021c71..d39b9aa 100644 --- a/README.md +++ b/README.md @@ -870,7 +870,7 @@ doc.go the package doc, and the vocabulary the package is written in fejkdata.go Generator, New, options, the embedded data set, List node.go the node model and JSON -> node compilation table.go tables: the rows TSV, its options and links, row selection and draws -path.go the dotted-path walk with its selectors, and proving a path resolves +path.go the dotted-path walks with their selectors, and proving a path resolves render.go Fake and the recursive renderer (choices, format strings, expansions) record.go records: Record, the JSON/CSV/SQL serializers, and their entry points struct.go structs: FakeStruct, fake tags, and a field's Go type as its column's datatype diff --git a/docs/decisions.md b/docs/decisions.md index fe5b4e6..867a86d 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -325,7 +325,7 @@ Go ties a package to one directory, so folders would split the API into packages ## The performance gate asserts allocations, not wall-clock time -`AllocsPerRun` is deterministic across machines, so a ±10% ceiling does not flake under +`AllocsPerRun` is deterministic across machines, so a 10% ceiling does not flake under CI load, while time varies with the machine and its neighbours. A rendering slowdown almost always costs an allocation too (a lost pre-size, a per-item map, an extra copy). The benchmark suite (the README's Development) reports time for a human, not as a pass/fail gate. @@ -689,8 +689,8 @@ though `hasRead` states the same stop rule. Valid while the two differ in where 2026-09-28, larv-review on systems-architect's finding; awaiting approval. Goals 2 and 11: `pathCheck`, `pathCover`, `pathProbe` and `pathDraw` each loop over `stepInto` and `table.route` themselves, and `descend` builds its probe in place, so the probe's -`drawn` map stays on the stack; its caller always builds that map, since no walker -picks a behaviour by a nil field. One walk calling them through an interface or a type +`drawn` map stays on the stack; its caller always builds that map, so `readRow` never +tests it for nil. One walk calling them through an interface or a type parameter leaks the walker and the pins it holds, an allocation per reference read. Valid while Go's escape analysis treats a call through an interface or a type parameter as leaking its receiver. diff --git a/family.go b/family.go index 0051deb..f941f8a 100644 --- a/family.go +++ b/family.go @@ -24,8 +24,8 @@ type tableSel struct { spelling string } -// tableReadOf replays a reference's selectors through the walk a render uses, so -// two paths pinning one row by different routes compare equal. checkPath proved +// tableReadOf replays a reference path through pathProbe, so two paths pinning one +// row by different routes compare equal. checkPath proved // each selector names a row. func tableReadOf(head node, a arm, leaf node) *tableRead { t, isTable := head.(*table) diff --git a/hold.go b/hold.go index ec51a05..81ee2a0 100644 --- a/hold.go +++ b/hold.go @@ -25,8 +25,7 @@ type draw struct { // token addresses by dotted path, or a field an operand reads — is drawn once and // kept, so {place.postal-code} and {place.locality} read one row, either read twice // gives one value, and a shown operand is the operand computed. Every other name is -// drawn afresh, so {word} {word} still draws twice. checkTokens, checkPath and -// linkRefs prove every step, so the walk cannot fail. +// drawn afresh, so {word} {word} still draws twice. func readField(s *session, t *template, held *hold, sc renderScope, a arm) draw { if s.trace != nil { traceRead(s.trace, t, sc, a) diff --git a/holdfence.go b/holdfence.go index cbbd04f..b839621 100644 --- a/holdfence.go +++ b/holdfence.go @@ -97,8 +97,8 @@ func operandReader(t *template, head string) string { return "" } -// coverPath collects what holding one path pins: every choice level the path -// passes through, whole, and the leaf it renders. +// coverPath collects what holding one path pins: the first choice level it passes, +// whole, else the leaf it renders; it reads no row. func coverPath(n node, tail []string, into map[node]bool) { _, _ = (&pathCover{into: into}).walk(n, tail) } diff --git a/node.go b/node.go index 62c791d..b6c274a 100644 --- a/node.go +++ b/node.go @@ -23,7 +23,7 @@ func (*folder) isNode() {} // choice picks one of its items. cum holds cumulative weights for a weighted // pick; when nil the choice is uniform and selection is O(1). shared is the set of -// relative dot paths every item can address, so descend and List both read the one +// relative dot paths every item can address, so carriedByAll and List both read the one // answer to what a path may reach through this choice. type choice struct { items []node diff --git a/path.go b/path.go index ee3d118..4966a32 100644 --- a/path.go +++ b/path.go @@ -125,8 +125,7 @@ func nameSegments(segs []string) []string { return out } -// stepInto is what seg names under n: a folder's entry or a template's field. A -// missing segment is an error, so no walk reaches past what the data holds. +// stepInto is what seg names under n: a folder's entry or a template's field. func stepInto(n node, seg string) (node, error) { if isSelector(seg) { return nil, fmt.Errorf("%s is not a table, so it has no row to select", selectorOf(seg)) @@ -148,7 +147,7 @@ func stepInto(n node, seg string) (node, error) { } // tableRoute is how a path passes one table: the row it reads, by selector or -// drawn, and what it goes on into, a linked table it descends to among them. +// drawn, what it goes on into, and whether that is a linked table. type tableRoute struct { sel string draw bool @@ -188,7 +187,7 @@ func (t *table) route(tail []string, descended bool) (tableRoute, error) { // pathCheck proves a path resolves whichever way the draws go: every variant of a // choice carries the rest of it, and is walked, a selector names a row inside the // rows selected before it, and no level read carries a repeat or a drawGroup. It -// collects every leaf the path may render; level and tail name a refused level. +// collects every leaf the path may render; level names the head in its errors. type pathCheck struct { pins pinSet level string @@ -258,7 +257,6 @@ func (w *pathCheck) enter(t *template, rest []string) error { return nil } -// pathCover is coverPath's walk: it stops at a choice and reads no row. type pathCover struct{ into map[node]bool } func (w *pathCover) walk(n node, tail []string) (node, error) { @@ -285,9 +283,8 @@ func (w *pathCover) walk(n node, tail []string) (node, error) { return n, nil } -// pathProbe proves a path resolves, walking one variant of a choice, so a -// path that resolves resolves whichever variants and rows are drawn. It draws -// nothing, marking in drawn the tables a draw would pin. +// pathProbe proves a path resolves without drawing: carriedByAll lets one variant +// of a choice stand for all. It marks in drawn the tables a draw would pin. type pathProbe struct { pins pinSet drawn map[*table]bool @@ -345,8 +342,8 @@ type pathDraw struct { a *arm } -// drawPath walks w over a path proved first, by a probe or by New's fences, so the -// walk cannot fail. head is what n is reached by, for the panic. +// drawPath walks w over a path proved first, by a probe or at load, so the walk +// cannot fail. head names n in the panic. func drawPath(n node, tail []string, head string, w *pathDraw) node { leaf, err := w.walk(n, tail) if err != nil { @@ -440,8 +437,7 @@ func carriedByAll(c *choice, rest []string) error { } // unreachableInChoice reports that a path cannot step through this choice, listing -// what every variant does carry. It reads the precomputed set, so a failing path -// costs no more than a rendering one. +// what every variant does carry. func unreachableInChoice(c *choice, want string) error { if len(c.shared) == 0 { return fmt.Errorf("no variant of this %d-way choice carries %q", len(c.items), want) @@ -454,8 +450,8 @@ func unreachableInChoice(c *choice, want string) error { return fmt.Errorf("not every variant of this %d-way choice carries %q; all carry %v", len(c.items), want, offered) } -// checkPath proves a dotted tail as pathCheck does, so a path that validates here -// resolves on every render, and a typo is a New-time error. +// checkPath runs pathCheck over a path read at load; level names the head in its +// errors. func checkPath(n node, tail []string, level string) error { _, err := (&pathCheck{level: level, tail: tail}).run(n) return err diff --git a/render.go b/render.go index d5a1b20..c7b100b 100644 --- a/render.go +++ b/render.go @@ -36,14 +36,10 @@ func (f *Generator) Fake(path string) (string, error) { return render(f.rand, n, sc), nil } -// descend walks named fields to the node a path names, pinning the table rows it -// selects or draws in sc. It is the one render-side step that can fail, because the -// path comes from the caller and may name a field or a row that does not exist. A -// choice consumes no segment, so the rest of the path must be one every variant -// carries before a variant is picked — a path that resolves at all resolves on -// every call. +// descend walks a caller's path to the node it names, pinning in sc the rows it +// selects or draws. func descend(s *session, root node, segments []string, sc renderScope) (node, error) { - // Walked once without drawing first, so a path that fails moves no seeded stream. + // docs/decisions.md#a-path-is-walked-once-without-drawing-before-it-is-walked-for-real if _, err := (&pathProbe{drawn: map[*table]bool{}}).walk(root, segments); err != nil { return nil, err }