From 39d59aec2366b45e6d9fa5dd99e190449bfae8f2 Mon Sep 17 00:00:00 2001 From: M Date: Mon, 31 Aug 2026 15:07:59 +0200 Subject: [PATCH] Pin the prep-time guards, and say why an operand's set is the narrow one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guards report an invariant break instead of computing around it, and no data reaches them, so they are called directly: coverage 97.4% -> 97.7%, which also covers the empty-cover skip through a literal operand. operandDraw's doc keeps why neither wider set works, since both were tried here — containment reaches a sibling the operand never renders, the render closure reaches a source two operands share — and drops the rest. Co-Authored-By: Claude Opus 5 --- builtins_test.go | 24 ++++++++++++++++++++++++ calc_test.go | 13 +++++++++++++ reference.go | 10 ++++------ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/builtins_test.go b/builtins_test.go index 0a41f3a..01b79fc 100644 --- a/builtins_test.go +++ b/builtins_test.go @@ -193,3 +193,27 @@ func TestRegistryShapes(t *testing.T) { } } } + +// TestArgGuardsPanic pins the guards that report a builtin arg its check should +// have rejected. No data reaches them — checkFunc runs a builtin's check before +// compileOps ever calls prep — so they are exercised directly. +func TestArgGuardsPanic(t *testing.T) { + for name, call := range map[string]func(){ + "atoi on an unvalidated arg": func() { atoi("nope") }, + "atof on an unvalidated arg": func() { atof("nope") }, + } { + mustPanic(t, name, call) + } +} + +// mustPanic fails unless call panics, which is what separates a reported +// invariant break from a silently wrong value. +func mustPanic(t *testing.T, name string, call func()) { + t.Helper() + defer func() { + if recover() == nil { + t.Errorf("%s: no panic, want the invariant reported", name) + } + }() + call() +} diff --git a/calc_test.go b/calc_test.go index 69cb8bb..32fe72d 100644 --- a/calc_test.go +++ b/calc_test.go @@ -103,6 +103,19 @@ func TestCalcTokenOperandsReadsOnlyACalc(t *testing.T) { } } +// TestCalcGuardsPanic pins the guards on calc's own invariants: checkCalc parsed +// the expression before prep sees it, and indexVars places every name calcVars +// read, so a break in either is reported rather than computed around. +func TestCalcGuardsPanic(t *testing.T) { + for name, call := range map[string]func(){ + "prep on an expression that does not parse": func() { calcPrep([]string{"1 +"}) }, + "an operand name never placed": func() { calcVar("n").eval(nil) }, + "indexVars on a name it did not read": func() { indexVars(calcVar("n"), map[string]int{}) }, + } { + mustPanic(t, name, call) + } +} + // --- one draw, one value: a calc operand reads the expansion's draw --- // TestCalcOperandReadsTheExpansionsDraw pins the one-draw rule for a calc operand. diff --git a/reference.go b/reference.go index bb62a21..ea04fb1 100644 --- a/reference.go +++ b/reference.go @@ -104,12 +104,10 @@ func cover(n node, into map[node]bool) { } // operandDraw collects what one held draw of a {calc()} operand answers for: that -// one node. A calc renders its operand whole, so the draw is the value that render -// produces, and another route conflicts only by naming the same node. Containment -// is the wrong set here — a sibling the operand never renders is no part of its -// value — and so is the render closure, which would read two names drawing from one -// shared source as one draw. A literal is left out for the reason cover leaves one -// out. +// one node, since a calc renders its operand whole. Neither wider set works — +// containment reaches a sibling the operand never renders, and the render closure +// reaches a source two operands share, and neither of those can disagree with it. +// A literal is left out for the reason cover leaves one out. func operandDraw(n node, into map[node]bool) { if _, fixed := n.(literal); fixed { return