diff --git a/Makefile b/Makefile index 50f16a332..897175dca 100644 --- a/Makefile +++ b/Makefile @@ -2133,6 +2133,79 @@ check-zero-range-negative-control: bin/schema test/zero_range_negative_main.cpp test/zero_range_negative_main.cpp -o build/schema_test_zero_range_negative ./build/schema_test_zero_range_negative +# THE ENUM-BOUND GATE and its NEGATIVE CONTROL (docs/SPEC-TABLES.md §2.4, +# §11, schema#605). A POSITIONAL ARRAY WHOSE BOUND FOLDS FROM AN ENUM is +# refused in a table body and a union arm, on the bound's PROVENANCE rather +# than its spelling: `[E.Max]T`, `[E.Count]T` and `[N]T` under a `const N` +# that folds from either are one bound however it is spelled. +# +# The gate runs the compiler over test/tables/enumbound, one unit a shape, and +# reads three answers back. Every refused shape must be refused and its +# diagnostic must name the field or the arm, the enum, the constant where the +# bound reaches the enum through one, and `[E]T` as the fix. The THREE +# POSITIVE CONTROLS must compile: the packet wire, whose `[E.Max]T` is a plain +# array the connect gate covers; a bound that folds from no enum, which is +# refused for reaching an enum and never for being positional; and the +# `type`-held case, which is schema#606's ruling and not this refusal's. +# +# The NEGATIVE CONTROL is §2.4's own: it REMOVES THE CONSTANT FOLD FROM THE +# BOUND CHECK through `go build -overlay` (no tracked file is written), and +# every row whose bound reaches its enum through a constant must then compile +# clean. It is targeted rather than blanket, so the two DIRECT spellings must +# stay refused under the same sabotage: a control that turned the whole rule +# off would go red for a reason that says nothing about the fold. +.PHONY: check-enum-bound-negative-control +check-enum-bound-negative-control: bin/schema + @mkdir -p build/enum-bound + @set -e; for u in BodyMax:ShipType.Max BodyCount:ShipType.Count \ + BodyConstMax:SlotCount BodyConstCount:SlotCount BodyFolded:SlotCount; do \ + unit=$${u%%:*}; bound=$${u#*:}; \ + if ./bin/schema check test/tables/enumbound/$$unit.schema > build/enum-bound/$$unit.log 2>&1; then \ + echo "GATE FAILED: $$unit.schema compiled in a table body"; exit 1; \ + fi; \ + grep -q "\[$$bound\]int32 is refused in a table body" build/enum-bound/$$unit.log || \ + { echo "GATE FAILED: $$unit.schema was refused, but not as a table body's own bound"; cat build/enum-bound/$$unit.log; exit 1; }; \ + grep -q "spell it \[ShipType\]int32" build/enum-bound/$$unit.log || \ + { echo "GATE FAILED: $$unit.schema names no fix"; cat build/enum-bound/$$unit.log; exit 1; }; \ + done + @set -e; for u in ArmMax:ShipType.Max ArmCount:ShipType.Count \ + ArmConstMax:SlotCount ArmConstCount:SlotCount ArmFolded:SlotCount; do \ + unit=$${u%%:*}; bound=$${u#*:}; \ + if ./bin/schema check test/tables/enumbound/$$unit.schema > build/enum-bound/$$unit.log 2>&1; then \ + echo "GATE FAILED: $$unit.schema compiled in a union arm"; exit 1; \ + fi; \ + grep -q "union Payload: arm ships: \[$$bound\]int32 is refused in a union arm" build/enum-bound/$$unit.log || \ + { echo "GATE FAILED: $$unit.schema was refused, but not at the arm"; cat build/enum-bound/$$unit.log; exit 1; }; \ + grep -q "table Fleet's field payload reaches Payload" build/enum-bound/$$unit.log || \ + { echo "GATE FAILED: $$unit.schema names no table reaching the union"; cat build/enum-bound/$$unit.log; exit 1; }; \ + done + @set -e; for unit in BodyConstMax BodyConstCount BodyFolded ArmConstMax ArmConstCount ArmFolded; do \ + grep -q "the bound SlotCount folds from ShipType\." build/enum-bound/$$unit.log || \ + { echo "GATE FAILED: $$unit.schema names no constant, and the constant is all a reader can see"; cat build/enum-bound/$$unit.log; exit 1; }; \ + done + @set -e; for unit in ControlPacket ControlPlain ControlTypeHeld; do \ + ./bin/schema check test/tables/enumbound/$$unit.schema > build/enum-bound/$$unit.log 2>&1 || \ + { echo "POSITIVE CONTROL FAILED: $$unit.schema did not compile"; cat build/enum-bound/$$unit.log; exit 1; }; \ + done + @echo "gate: ten shapes refused on the bound's provenance, three controls compile" + @sed 's|if entry == nil \|\| entry.decl == nil \|\| visiting\[e.Name\] {|if true { // SABOTAGED: the constant fold removed from the bound check|' \ + internal/check/tablekeyed.go > build/tablekeyed-no-fold.gotext + @grep -q SABOTAGED build/tablekeyed-no-fold.gotext || \ + { echo "NEGATIVE CONTROL FAILED: the sabotage patched nothing"; exit 1; } + @printf '{"Replace":{"%s/internal/check/tablekeyed.go":"%s/build/tablekeyed-no-fold.gotext"}}\n' \ + "$(CURDIR)" "$(CURDIR)" > build/enum-bound-no-fold-overlay.json + @go build -overlay=build/enum-bound-no-fold-overlay.json -o build/schema-no-enum-bound-fold ./cmd/schema + @set -e; for unit in BodyConstMax BodyConstCount BodyFolded ArmConstMax ArmConstCount ArmFolded; do \ + ./build/schema-no-enum-bound-fold check test/tables/enumbound/$$unit.schema > build/enum-bound/$$unit-nofold.log 2>&1 || \ + { echo "NEGATIVE CONTROL FAILED: $$unit.schema was still refused without the constant fold, so the fold is not what refuses it"; cat build/enum-bound/$$unit-nofold.log; exit 1; }; \ + done + @set -e; for unit in BodyMax BodyCount ArmMax ArmCount; do \ + if ./build/schema-no-enum-bound-fold check test/tables/enumbound/$$unit.schema > build/enum-bound/$$unit-nofold.log 2>&1; then \ + echo "NEGATIVE CONTROL FAILED: $$unit.schema compiled too, so the sabotage removed the whole rule and not the fold"; exit 1; \ + fi; \ + done + @echo "negative control: without the constant fold, six folded rows compile clean and the four direct spellings stay refused" + # THE VARIANT-ORDER NEGATIVE CONTROL (SPEC §3.1, issue #462). An enum value # rides as its declaration ordinal and a flags variant as its bit position, so # the projection carries both declarations' variant names in declaration order: @@ -2822,6 +2895,7 @@ test: build/schema_test build/schema_test_guard build/schema_test_tables build/s $(MAKE) wide-table-odd-length-negative-control $(MAKE) wide-table-byte-length-negative-control $(MAKE) check-zero-range-negative-control + $(MAKE) check-enum-bound-negative-control $(MAKE) projection-variant-order-negative-control $(MAKE) projection-wire-law-negative-control $(MAKE) projection-union-arm-order-negative-control diff --git a/docs/SPEC-TABLES.md b/docs/SPEC-TABLES.md index cd0d50a32..cea8ca281 100644 --- a/docs/SPEC-TABLES.md +++ b/docs/SPEC-TABLES.md @@ -1182,17 +1182,15 @@ diagnostic does not name the field, the enum and the fix. and every row whose bound reaches its enum through a constant compiles clean without it. -**CHECKER STATUS: `[E.Max]T` IS REFUSED, THE OTHER SPELLINGS ARE NOT.** -`schema check` refuses `[E.Max]T` in a table body and in a union arm, naming -the field, the enum and `[E]T` as the fix. It accepts `[E.Count]T` and `[N]T` -under a `const N` that folds from either, with no diagnostic and exit 0, so a -unit that spells the bound either of those ways compiles and carries the -positional class this rule exists to close. The rule above follows the bound's -PROVENANCE and the checker still follows its spelling, and closing that gap is -owed as schema#540. Two sections rest on the refusal being made whole, §4.1's +**CHECKER STATUS: THE REFUSAL FOLLOWS THE PROVENANCE.** `schema check` refuses +every spelling above in a table body and in a union arm, reading the bound's +provenance and not its text: `[E.Max]T`, `[E.Count]T`, and `[N]T` under a +`const N` that folds from either at any depth of constant arithmetic. The +diagnostic names the field, the enum, the constant where the bound reaches the +enum through one, and `[E]T` as the fix; an arm's names the arm and the table +that reaches the union. Two sections rest on this refusal being whole, §4.1's count of the silent class and SPEC.md §3.1's one exception to reachability, and -each is written from this rule rather than from the tree. This paragraph is -deleted by the implementation PR that closes the gap. +both stand on the tree as well as on the rule. **RULING STATUS: the type-held case is ruled on schema#606.** Until then a `type` no table reaches keeps the spelling and a `type` a table reaches is @@ -9843,12 +9841,10 @@ in build version (§20.5). table closure, `| max = K` headroom and variant id collisions, each diagnostic naming the keying field that pulled the enum in. A slot value no variant names is a SAVE failure, not a silent `None` (§3.2). - **CHECKER STATUS: `[E.Max]T` is refused in a table body and in a union arm. - `[E.Count]T` and `[N]T` under a `const N` that folds from either are - accepted there today with no diagnostic**, because the checker still - follows the spelling where the rule follows the provenance, owed as - schema#540 (§2.4), and this sentence is deleted by the implementation PR - that closes the gap. + **CHECKER STATUS: `[E.Max]T`, `[E.Count]T` and `[N]T` under a `const N` that + folds from either are all refused in a table body and in a union arm**, on + the bound's provenance, and an arm's diagnostic names the arm and the table + that reaches the union (§2.4). **RULING STATUS: the type-held case is ruled on schema#606**, and until then a `type` a table reaches is not refused (§2.4). - **Maps** (§2.8): a map in a `type` body; a key that is an enum (the diff --git a/docs/USAGE.md b/docs/USAGE.md index 7aa9e358a..bc09e532c 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1938,10 +1938,10 @@ Only the table wire keys the slots. **And a positional array whose bound comes from an enum is REFUSED in a table body and a union arm, by name**, with `[E]T` named as the fix. The refusal follows where the bound comes from and not how it is spelled, so `[E.Max]T`, -`[E.Count]T` and `[N]T` under a `const N = E.Max` all take it. *The compiler -refuses `[E.Max]T` today and still reads the other two as plain bounds, so -`[E.Count]T` and the constant fold still -compile ([#540](https://github.com/mas-bandwidth/schema/issues/540)).* +`[E.Count]T` and `[N]T` under a `const N = E.Max` all take it, at any depth of +constant arithmetic. The diagnostic names the constant where the bound reaches +the enum through one, and an arm's names the arm and the table that reaches +the union. An ordinal-indexed array is a positional vocabulary, and a table has exactly one of those — `flags` — so the refusal is what keeps the closed class closed: you cannot reopen it by spelling the bound diff --git a/docs/VERSIONING.md b/docs/VERSIONING.md index 0138e8f54..42cd1dac6 100644 --- a/docs/VERSIONING.md +++ b/docs/VERSIONING.md @@ -306,7 +306,7 @@ does today. | a flags variant inserted or removed | silent | refuses | moves, and so does the protocol id | | a flags variant reordered or renamed in place | **silent** | **refuses** | moves: the cook projection digests each variant's bit position, and the protocol id moves too | | a union arm reordered or renamed | `unknown` for an arm this reader lacks; a reorder is silent and safe | warns on a vanished name | moves, and the protocol id with it where a `type` reaches the union: the arm names are what a same-typed reorder moves | -| a keyed array made positional | `kind_mismatch` | refuses; and in a TABLE body the positional spelling is refused by name (SPEC-TABLES.md §2.4, §11) | moves | +| a keyed array made positional | `kind_mismatch` | refuses; and in a TABLE body and in a UNION ARM the positional spelling is refused by name, on the bound's provenance, so `[E.Max]T`, `[E.Count]T` and `[N]T` under a `const N` that folds from either are all refused there (SPEC-TABLES.md §2.4, §11) | moves | | a keyed array's key enum swapped for another | `unknown`, one per slot; the kind stays | refuses | moves | | a map's KEY kind changed, or its KEY bound tightened (SPEC-TABLES.md §2.8) | a changed kind is one `kind_mismatch` for the map, which reads empty. A tightened bound drops the entries that no longer fit and counts `clamped`, one per entry | **refuses** a changed kind, warns on a tightened bound | moves | | an array changed between `[]T` and `[..N]T` (SPEC-TABLES.md §2.9) | nothing where the count fits the new bound, `clamped` past it: the two are the same bytes | warns on the direction that ADDS a bound, as any capacity shrunk; passes on the one that removes it | moves: the storage is a reference and a count on one side and the maximum inline on the other | @@ -1071,11 +1071,6 @@ repository not yet behind it. The 3.0.0 release holds the list at zero. - #432: the cook triple, and the byte-order sentences in five places. - #441: the retired-names ledger. - #446: the evolution table's fixtures. -- #540: the refusal following the bound's PROVENANCE rather than its spelling. - `[E.Max]T` is refused in a table body and a union arm, but `[E.Count]T` and - `[N]T` under a `const N` that folds from either still compile there, which - SPEC-TABLES.md §2.4 and §11 state as refused, so those two spellings reopen - the class §4.1 closed. The `type`-held case is ruled on #606. - #525: retain-unknown in the eight ports, `internal/tablewire`'s own retention and the fuzzer leg that needs it, the MESSAGE form's `LoadRetain`, and the conformance rows. The C++ reference and the two report counters are diff --git a/internal/check/check.go b/internal/check/check.go index b666c21bc..575ab9c80 100644 --- a/internal/check/check.go +++ b/internal/check/check.go @@ -1464,9 +1464,14 @@ func (c *checker) resolveField(owner string, f *ast.Field, inTable bool) *ir.Fie out.ArrayBound = out.KeyEnumRef.Max out.ArrayExpr = f.Array.Hi default: - if !c.checkPositionalKeyedSpelling(f, inTable) { - return nil - } + // A POSITIONAL BOUND THAT FOLDS FROM AN ENUM is refused in a + // table body and a union arm on the bound's PROVENANCE + // (docs/SPEC-TABLES.md §2.4, §11), and that reads a closure + // rather than a field, so it runs in + // checkPositionalEnumBoundInClosure once the closure is known. + // The bound still evaluates here: the refusal reports on the + // resolved field, and a bound that cannot be evaluated is a + // different diagnostic that belongs at the field. hi, ok := c.evalInt(f.Array.Hi) if !ok { return nil @@ -2246,6 +2251,7 @@ func (c *checker) checkTables() { } c.tableClosure = closure c.checkTableArmsReached(closure) + c.checkPositionalEnumBoundInClosure() names := make([]string, 0, len(closure)) for name := range closure { diff --git a/internal/check/tablekeyed.go b/internal/check/tablekeyed.go index cbbe99642..f1a7fbab2 100644 --- a/internal/check/tablekeyed.go +++ b/internal/check/tablekeyed.go @@ -1,39 +1,193 @@ // The enum-keyed array's spellings in the checker (docs/SPEC-TABLES.md §2.4): -// `[E]T` is the table form, whose slots ride by variant NAME, and `[E.Max]T` -// is the positional one the TYPE wire keeps and a table body refuses. +// `[E]T` is the table form, whose slots ride by variant NAME, and a POSITIONAL +// array whose bound FOLDS FROM an enum is what a table body and a union arm +// refuse. The refusal reads the bound's PROVENANCE rather than its spelling, +// so `[E.Max]T`, `[E.Count]T` and `[N]T` under a `const N` that folds from +// either are one rule (schema#605). package check import ( + "fmt" + "sort" + "github.com/mas-bandwidth/schema/v2/internal/ast" + "github.com/mas-bandwidth/schema/v2/ir" ) -// checkPositionalKeyedSpelling refuses `[E.Max]T` IN A TABLE BODY, where -// `[E]T` is the table form (docs/SPEC-TABLES.md §2.4, §11). +// enumBound is what the provenance walk found under a bound: the enum the +// bound folds from, the selector at the far end of the fold, and the constant +// the FIELD spells where it reaches the enum through one. `through` is what +// the diagnostic names, because a reader looking at the field sees the +// constant and nothing about the enum. +type enumBound struct { + enum string + sel string // "Max" or "Count" + through string // "" where the bound names the enum itself +} + +// enumBoundProvenance follows a bound expression to the enum it folds from, +// through named constants and through constant arithmetic at any depth +// (SPEC.md §4.2). A bound that folds from an enum is ONE BOUND however it is +// spelled, so this walk and not the field's own text is what the refusal below +// reads. +// +// It looks at an ENUM alone. A `flags` declaration carries Max and Count too +// and is refused as a bound by its own rule, a mask naming no single slot +// (§2.4, §11); a bound that reaches neither is a plain positional array and +// stands wherever it is spelled. +// +// `visiting` guards the constant graph. A reference cycle among constants is +// its own compile error (SPEC.md §4.2), reported where the constants resolve, +// and this walk runs over the same graph, so it carries its own guard rather +// than recursing forever on a unit that is already refused. +func (c *checker) enumBoundProvenance(e ast.Expr, visiting map[string]bool) (enumBound, bool) { + switch e := e.(type) { + case *ast.MaxExpr: + if e.Sel != "Max" && e.Sel != "Count" { + return enumBound{}, false + } + if _, isEnum := c.astDecls[e.Enum].(*ast.EnumDecl); !isEnum { + return enumBound{}, false + } + return enumBound{enum: e.Enum, sel: e.Sel}, true + case *ast.IdentExpr: + entry := c.constant[e.Name] + if entry == nil || entry.decl == nil || visiting[e.Name] { + return enumBound{}, false + } + visiting[e.Name] = true + found, ok := c.enumBoundProvenance(entry.decl.Expr, visiting) + delete(visiting, e.Name) + if !ok { + return enumBound{}, false + } + // the OUTERMOST constant is the one the FIELD spells, and the walk + // returns through every level, so each assignment overwrites the + // deeper one and the name a reader can see is what survives + found.through = e.Name + return found, true + case *ast.ParenExpr: + return c.enumBoundProvenance(e.X, visiting) + case *ast.UnaryExpr: + return c.enumBoundProvenance(e.X, visiting) + case *ast.BinaryExpr: + if found, ok := c.enumBoundProvenance(e.X, visiting); ok { + return found, true + } + return c.enumBoundProvenance(e.Y, visiting) + } + return enumBound{}, false +} + +// refusePositionalEnumBound is the diagnostic, one shape for both scopes. // // AN ORDINAL-INDEXED ARRAY IS A POSITIONAL VOCABULARY AND A TABLE MAY HAVE -// ONLY ONE. A `[E.Max]T` field carries its elements by position, so inserting -// a variant in the middle of E lands every later element one slot off, in -// every file already written, with nothing on the wire that could say so — -// the silent class §4.1 names. Keyed slots ride by NAME (§3.2), so a middle -// insert moves no slot, and refusing the positional spelling here is what -// leaves `flags` as the only positional vocabulary a table has, and therefore -// the only exception the reachability-scoped projection needs (SPEC §3.1). +// ONLY ONE. Such a field carries its elements by position, so inserting a +// variant in the middle of E lands every later element one slot off, in every +// file already written, with nothing on the wire that could say so: the silent +// class §4.1 names. Keyed slots ride by NAME (§3.2), so a middle insert moves +// no slot, and refusing the positional spelling here is what leaves `flags` as +// the only positional vocabulary a table body and a union arm have, and +// therefore the only exception the reachability-scoped projection needs +// (SPEC.md §3.1). +// +// `where` names the declaration and the field or the arm, `scope` names the +// body the rule is stated on, and `reach` names the table that pulled a union +// in. The diagnostic names the field, the enum the bound folds from, the +// constant where the bound reaches the enum through one, and the fix. +func (c *checker) refusePositionalEnumBound(where, scope, reach string, f *ir.Field) { + // THE BOUND'S OWN SHAPES ONLY: `[N]T`, the fixed positional array. `[E]T` + // is the table form and carries a KeyEnum, `[..N]T` and `[A..N]T` are a + // count rather than an enum's extent, and `[]T` has no bound at all. + if f.Array != ir.ArrayFixed || f.KeyEnum != "" || f.ArrayExpr == nil { + return + } + found, ok := c.enumBoundProvenance(f.ArrayExpr, map[string]bool{}) + if !ok { + return + } + fold := "" + if found.through != "" { + fold = fmt.Sprintf(", because the bound %s folds from %s.%s", found.through, found.enum, found.sel) + } + elem := ir.TableTypeSpelling(f) + c.errf(f.ArrayExpr.ExprPos(), "%s: [%s]%s is refused in %s%s%s. An ordinal-indexed array is a POSITIONAL vocabulary, so inserting a variant in the middle of %s lands every later element one slot off in every file already written, with nothing on the wire that could say so. Instead spell it [%s]%s, whose slots ride by variant name (docs/SPEC-TABLES.md §2.4, §11)", + where, exprSpelling(f.ArrayExpr), elem, scope, fold, reach, found.enum, found.enum, elem) +} + +// checkPositionalEnumBoundInClosure is §2.4's refusal over the scope §2.4 +// states: A TABLE BODY AND A UNION ARM. It runs once the closure is known, +// which is what lets a union arm's diagnostic name the table that reaches the +// union, the way #572's closure refusal names the edge that pulled a `type` +// in. An arm is a field line (§2.6), so the arm and the table body's own field +// take one rule and one sentence. // -// On the TYPE wire the spelling stays legal and positional, unchanged: a -// `type` body's `[E.Max]T` is a plain array whose extent is the variant count, -// and every fact of it projects. The refusal is the TABLE body's alone. -func (c *checker) checkPositionalKeyedSpelling(f *ast.Field, inTable bool) bool { - if !inTable || f.Array == nil || f.Array.Kind != ast.ArrayFixed { - return true - } - m, ok := f.Array.Hi.(*ast.MaxExpr) - if !ok || m.Sel != "Max" { - return true - } - if _, isEnum := c.astDecls[m.Enum].(*ast.EnumDecl); !isEnum { - return true - } - c.errf(m.Pos, "field %s: [%s.Max]%s is refused in a table body. An ordinal-indexed array is a POSITIONAL vocabulary, so inserting a variant in the middle of %s lands every later element one slot off in every file already written, with nothing on the wire that could say so. Instead spell it [%s]%s, whose slots ride by variant name (docs/SPEC-TABLES.md §2.4, §11)", - f.Name, m.Enum, scalarSpelling(f.Type), m.Enum, m.Enum, scalarSpelling(f.Type)) - return false +// THE WALK IS THE FENCE. It starts at TABLE bodies and descends UNIONS alone, +// so a `type` a table closure reaches is never visited. That case has two +// answers that exclude each other, refusing the shape in every reached `type` +// or keying the table wire for an enum-extent array wherever it is declared, +// and it is ruled on schema#606. Widening this walk to `type` bodies is the +// one edit that decides the ruling, so it is not made here. +// +// The PACKET WIRE is untouched for the same reason: a `type` no table reaches +// is not in the closure, its `[E.Max]T` is a plain positional array whose +// extent every fact of projects, and the connect gate covers a variant insert +// because the protocol id moves with the spelling (SPEC.md §3.1). +func (c *checker) checkPositionalEnumBoundInClosure() { + // SORTED: map iteration order must not shuffle the diagnostics run to + // run, and the sort is also what makes the reaching table a STABLE choice + // where two tables carry one union. + roots := make([]string, 0, len(c.tables)) + for name := range c.tables { + roots = append(roots, name) + } + sort.Strings(roots) + + // ONE FIELD, ONE DIAGNOSTIC. A union two tables carry resolves once, so + // its arms are the same fields under both edges and a second visit would + // refuse one declaration twice. + reported := map[*ir.Field]bool{} + for _, name := range roots { + st := c.tables[name] + if st == nil { + continue + } + for _, f := range st.Fields { + if reported[f] { + continue + } + reported[f] = true + c.refusePositionalEnumBound(fmt.Sprintf("table %s: field %s", name, f.Name), "a table body", "", f) + if un, ok := f.Type.Ref.(*ir.Union); ok && f.Type.Kind == ir.TNamed { + c.checkUnionArmBounds(un, name, f.Name, reported, map[*ir.Union]bool{}) + } + } + } +} + +// checkUnionArmBounds refuses the same bound in the arms of a union a table +// body carries, descending an arm that is itself a union: such an arm brings +// its own arms onto the table wire (docs/SPEC-TABLES.md §2.6), so it reaches +// the closure through the same edge and takes the same rule. +func (c *checker) checkUnionArmBounds(un *ir.Union, table, edge string, reported map[*ir.Field]bool, seen map[*ir.Union]bool) { + if seen[un] { + return + } + seen[un] = true + reach := fmt.Sprintf(", and table %s's field %s reaches %s", table, edge, un.Name) + for _, v := range un.Variants { + f := v.F + if f == nil { + continue // a PAYLOAD-FREE arm carries no bound (SPEC §4.8) + } + if inner, ok := f.Type.Ref.(*ir.Union); ok && f.Type.Kind == ir.TNamed { + c.checkUnionArmBounds(inner, table, edge, reported, seen) + continue + } + if reported[f] { + continue + } + reported[f] = true + c.refusePositionalEnumBound(fmt.Sprintf("union %s: arm %s", un.Name, f.Name), "a union arm", reach, f) + } } diff --git a/internal/check/tablemap.go b/internal/check/tablemap.go index 192e6820d..d023e2779 100644 --- a/internal/check/tablemap.go +++ b/internal/check/tablemap.go @@ -318,14 +318,28 @@ func boundSpelling(a *ast.ArrayBound) string { return "" } +// exprSpelling renders a bound as the source spells it, so a diagnostic quotes +// the declaration back rather than a placeholder. Constant ARITHMETIC is part +// of a bound's spelling (SPEC.md §4.2), and the enum-bound refusal +// (docs/SPEC-TABLES.md §2.4) reads bounds like `[Grade.Max + 1]` that are +// nothing but arithmetic, so the operators render too. "N" is left for an +// expression the grammar admits and nothing here names. func exprSpelling(e ast.Expr) string { switch e := e.(type) { case *ast.IntLit: return e.Value.String() + case *ast.FloatLit: + return "N" // a float is not a bound; the refusal for one is its own case *ast.IdentExpr: return e.Name case *ast.MaxExpr: return e.Enum + "." + e.Sel + case *ast.ParenExpr: + return "(" + exprSpelling(e.X) + ")" + case *ast.UnaryExpr: + return e.Op + exprSpelling(e.X) + case *ast.BinaryExpr: + return exprSpelling(e.X) + " " + e.Op + " " + exprSpelling(e.Y) } return "N" } diff --git a/internal/check/tables_test.go b/internal/check/tables_test.go index daedf3d06..1f0a1e267 100644 --- a/internal/check/tables_test.go +++ b/internal/check/tables_test.go @@ -1496,3 +1496,120 @@ type Loadout { t.Fatalf("the table form [ShipType]Cfg did not compile: %v", errs) } } + +// TestEnumBoundProvenanceCorpus is §2.4's "HELD BY TEST: one diagnostics row a +// SHAPE, red first", read from the corpus rather than restated here. +// +// THE RULE FOLLOWS THE BOUND'S PROVENANCE AND NOT ITS SPELLING. `[E.Max]T`, +// `[E.Count]T` and `[N]T` under a `const N` that folds from either are one +// bound, so one file a shape stands in test/tables/enumbound and this test +// holds each to its answer: the refused shapes in a TABLE BODY and in a UNION +// ARM, and the three controls that hold the other edge. A row is red if the +// unit compiles, or if the diagnostic stops naming the field, the enum, the +// constant the field spells, and `[E]T` as the fix. +// +// The corpus is read rather than inlined because the same files are what +// `check-enum-bound-negative-control` runs the compiler over: a shape stated +// in one place is a shape that cannot drift between the two. +func TestEnumBoundProvenanceCorpus(t *testing.T) { + // want is every substring the diagnostic must carry; an empty want is a + // CONTROL, which must compile clean. + cases := []struct { + file string + want []string + }{ + // THE BOUND'S OWN SHAPES, IN A TABLE BODY + {file: "BodyMax.schema", want: []string{ + "table Fleet: field ships", "[ShipType.Max]int32 is refused in a table body", + "spell it [ShipType]int32"}}, + {file: "BodyCount.schema", want: []string{ + "table Fleet: field ships", "[ShipType.Count]int32 is refused in a table body", + "spell it [ShipType]int32"}}, + {file: "BodyConstMax.schema", want: []string{ + "table Fleet: field ships", "[SlotCount]int32 is refused in a table body", + "the bound SlotCount folds from ShipType.Max", "spell it [ShipType]int32"}}, + {file: "BodyConstCount.schema", want: []string{ + "table Fleet: field ships", "[SlotCount]int32 is refused in a table body", + "the bound SlotCount folds from ShipType.Count", "spell it [ShipType]int32"}}, + {file: "BodyFolded.schema", want: []string{ + "table Fleet: field ships", "[SlotCount]int32 is refused in a table body", + "the bound SlotCount folds from ShipType.Max", "spell it [ShipType]int32"}}, + + // THE UNION ARM'S SHAPE, in every spelling the bound has: the + // diagnostic names the ARM as well, and the table that reaches the + // union beside it. + {file: "ArmMax.schema", want: []string{ + "union Payload: arm ships", "[ShipType.Max]int32 is refused in a union arm", + "table Fleet's field payload reaches Payload", "spell it [ShipType]int32"}}, + {file: "ArmCount.schema", want: []string{ + "union Payload: arm ships", "[ShipType.Count]int32 is refused in a union arm", + "table Fleet's field payload reaches Payload", "spell it [ShipType]int32"}}, + {file: "ArmConstMax.schema", want: []string{ + "union Payload: arm ships", "[SlotCount]int32 is refused in a union arm", + "the bound SlotCount folds from ShipType.Max", + "table Fleet's field payload reaches Payload", "spell it [ShipType]int32"}}, + {file: "ArmConstCount.schema", want: []string{ + "union Payload: arm ships", "[SlotCount]int32 is refused in a union arm", + "the bound SlotCount folds from ShipType.Count", + "table Fleet's field payload reaches Payload", "spell it [ShipType]int32"}}, + {file: "ArmFolded.schema", want: []string{ + "union Payload: arm ships", "[SlotCount]int32 is refused in a union arm", + "the bound SlotCount folds from ShipType.Count", + "table Fleet's field payload reaches Payload", "spell it [ShipType]int32"}}, + + // THE CONTROLS HOLD THE OTHER EDGE (§2.4): the packet wire is + // untouched, a bound that folds from no enum stands wherever it is + // spelled, and the `type`-held case schema#606 rules on keeps the + // spelling. + {file: "ControlPacket.schema"}, + {file: "ControlPlain.schema"}, + {file: "ControlTypeHeld.schema"}, + } + + dir := filepath.Join("..", "..", "test", "tables", "enumbound") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("the enum-bound corpus: %v", err) + } + onDisk := map[string]bool{} + for _, e := range entries { + if strings.HasSuffix(e.Name(), ".schema") { + onDisk[e.Name()] = true + } + } + for _, tc := range cases { + if !onDisk[tc.file] { + t.Errorf("%s: the corpus file is gone, and a shape with no unit is a shape nothing holds", tc.file) + continue + } + delete(onDisk, tc.file) + src, err := os.ReadFile(filepath.Join(dir, tc.file)) + if err != nil { + t.Errorf("%s: %v", tc.file, err) + continue + } + errs := runUnit(t, map[string]string{tc.file: string(src)}) + if len(tc.want) == 0 { + if len(errs) > 0 { + t.Errorf("%s is a CONTROL and it did not compile: %v", tc.file, errs) + } + continue + } + if len(errs) == 0 { + t.Errorf("%s compiled in a table closure, and a variant inserted in the middle of ShipType would land every later element one slot off in every file already written, with nothing on the wire that could say so", tc.file) + continue + } + joined := "" + for _, e := range errs { + joined += e.Error() + "\n" + } + for _, want := range tc.want { + if !strings.Contains(joined, want) { + t.Errorf("%s: no diagnostic says %q; got:\n%s", tc.file, want, joined) + } + } + } + for name := range onDisk { + t.Errorf("%s is in the corpus and no row holds it, so nothing says what its answer is", name) + } +} diff --git a/test/tables/V1.schema b/test/tables/V1.schema index b90f08d2f..abef0395a 100644 --- a/test/tables/V1.schema +++ b/test/tables/V1.schema @@ -8,10 +8,19 @@ package tblv1 // a bounded array's BOUND is not wire identity: V2 shrinks MaxSlots from 6 -// to 3, and tally is sized off Grade.Max + 1, which grows when Grade does. -// Both directions still load (docs/SPEC-TABLES.md §4). +// to 3, and TallySlots grows from 3 to 4. Both directions still load +// (docs/SPEC-TABLES.md §4). const MaxSlots = 6 +// tally and ledger below are POSITIONAL arrays in a table body, so their +// bounds fold from no enum: a bound that folds from one is refused there on +// the bound's PROVENANCE (docs/SPEC-TABLES.md §2.4, §11), whatever arithmetic +// stands between the two. The extents are what the generations exercise, so +// they are spelled as the plain numbers they are. +const TallySlots = 3 + +const LedgerSlots = 3 + enum Mode { Alpha, Beta } // Grade gains Silver in the MIDDLE in V2: under ordinals Gold would slide @@ -56,22 +65,22 @@ table Cell table Cfg { - a int32 = 5 | min = 0, max = 1000 + a int32 = 5 | min = 0, max = 1000 b float32 = 1.5 mode Mode = Beta name string(32) inner Inner - items [..8]int32 | min = 0, max = 255 + items [..8]int32 | min = 0, max = 255 grade Grade grades [..4]Grade podium [3]Grade slots [..MaxSlots]int32 - tally [Grade.Max + 1]int32 + tally [TallySlots]int32 effect Effect bank [Slot]Cell tokens [Slot]int32 ranks [Slot]Grade - ledger [Grade.Max + 1]int32 + ledger [LedgerSlots]int32 extra ?Inner tier ?int32 mark ?Grade diff --git a/test/tables/V2.schema b/test/tables/V2.schema index 6ab9a952e..b3692319a 100644 --- a/test/tables/V2.schema +++ b/test/tables/V2.schema @@ -8,6 +8,12 @@ package tblv2 const MaxSlots = 3 +// tally is a POSITIONAL array in a table body, so its bound folds from no +// enum: a bound that folds from one is refused there on the bound's +// PROVENANCE (docs/SPEC-TABLES.md §2.4, §11). It is 4 here against V1's 3, +// which is the growing bound the generations exercise. +const TallySlots = 4 + enum Mode { Alpha, Beta } enum Grade { Bronze, Silver, Gold } @@ -56,14 +62,14 @@ table Cfg a float32 = 5.0 c bool = true mode Mode = Beta - title string(32) | was = "name" + title string(32) | was = "name" inner Inner - items [..8]int32 | min = 0, max = 255 + items [..8]int32 | min = 0, max = 255 grade Grade grades [..4]Grade podium [3]Grade slots [..MaxSlots]int32 - tally [Grade.Max + 1]int32 + tally [TallySlots]int32 effect Effect bank [Slot]Cell tokens [Slot]int32 diff --git a/test/tables/enumbound/ArmConstCount.schema b/test/tables/enumbound/ArmConstCount.schema new file mode 100644 index 000000000..a819ca701 --- /dev/null +++ b/test/tables/enumbound/ArmConstCount.schema @@ -0,0 +1,20 @@ +package ebarmconstcount + +// THE UNION ARM'S SHAPE, WITH THE BOUND FOLDED THROUGH A CONSTANT +// (docs/SPEC-TABLES.md §2.4, §11): the provenance walk and the arm scope are +// one rule, so the arm takes the refusal the constant hides. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Count + +union Payload +{ + ships [SlotCount]int32 + plain int32 +} + +table Fleet +{ + payload Payload +} diff --git a/test/tables/enumbound/ArmConstMax.schema b/test/tables/enumbound/ArmConstMax.schema new file mode 100644 index 000000000..fb7a7389a --- /dev/null +++ b/test/tables/enumbound/ArmConstMax.schema @@ -0,0 +1,20 @@ +package ebarmconstmax + +// `[N]T` UNDER `const N = E.Max` IN A UNION ARM (docs/SPEC-TABLES.md §2.4, +// §11): the constant hides the enum from the arm's own spelling, and the +// refusal follows the fold rather than the spelling. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Max + +union Payload +{ + ships [SlotCount]int32 + plain int32 +} + +table Fleet +{ + payload Payload +} diff --git a/test/tables/enumbound/ArmCount.schema b/test/tables/enumbound/ArmCount.schema new file mode 100644 index 000000000..16a04c850 --- /dev/null +++ b/test/tables/enumbound/ArmCount.schema @@ -0,0 +1,19 @@ +package ebarmcount + +// `[E.Count]T` IN A UNION ARM (docs/SPEC-TABLES.md §2.4, §11): the second +// direct spelling, in the scope the table body carries. An arm is a field +// line, so the bound reaches the enum exactly as a table body's own field +// does and takes the same refusal. + +enum ShipType { Fighter, Bomber, Scout } + +union Payload +{ + ships [ShipType.Count]int32 + plain int32 +} + +table Fleet +{ + payload Payload +} diff --git a/test/tables/enumbound/ArmFolded.schema b/test/tables/enumbound/ArmFolded.schema new file mode 100644 index 000000000..4939e0417 --- /dev/null +++ b/test/tables/enumbound/ArmFolded.schema @@ -0,0 +1,22 @@ +package ebarmfolded + +// `[N]T` UNDER A CONSTANT THAT FOLDS THROUGH ANOTHER CONSTANT, IN A UNION ARM +// (docs/SPEC-TABLES.md §2.4, §11): the deepest spelling in the widest scope. +// Constant arithmetic runs to any depth (SPEC.md §4.2), so the walk that finds +// the enum under one constant finds it under two and an operator. + +enum ShipType { Fighter, Bomber, Scout } + +const Base = ShipType.Count +const SlotCount = Base * 3 - Base * 2 + +union Payload +{ + ships [SlotCount]int32 + plain int32 +} + +table Fleet +{ + payload Payload +} diff --git a/test/tables/enumbound/ArmMax.schema b/test/tables/enumbound/ArmMax.schema new file mode 100644 index 000000000..ea562f633 --- /dev/null +++ b/test/tables/enumbound/ArmMax.schema @@ -0,0 +1,19 @@ +package ebarmmax + +// THE UNION ARM'S SHAPE (docs/SPEC-TABLES.md §2.4, §11): a table body carries +// the union, so the arm's array carries the table body's own hazard and takes +// the table body's own refusal, with the arm named and the table that reaches +// the union named beside it. + +enum ShipType { Fighter, Bomber, Scout } + +union Payload +{ + ships [ShipType.Max]int32 + plain int32 +} + +table Fleet +{ + payload Payload +} diff --git a/test/tables/enumbound/BodyConstCount.schema b/test/tables/enumbound/BodyConstCount.schema new file mode 100644 index 000000000..18190ca9a --- /dev/null +++ b/test/tables/enumbound/BodyConstCount.schema @@ -0,0 +1,13 @@ +package ebbodyconstcount + +// `[N]T` UNDER `const N = E.Count` IN A TABLE BODY (docs/SPEC-TABLES.md §2.4, +// §11): the other selector, reached the same way. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Count + +table Fleet +{ + ships [SlotCount]int32 +} diff --git a/test/tables/enumbound/BodyConstMax.schema b/test/tables/enumbound/BodyConstMax.schema new file mode 100644 index 000000000..50c4e63b1 --- /dev/null +++ b/test/tables/enumbound/BodyConstMax.schema @@ -0,0 +1,14 @@ +package ebbodyconstmax + +// `[N]T` UNDER `const N = E.Max` IN A TABLE BODY (docs/SPEC-TABLES.md §2.4, +// §11): the bound reaches the enum through a constant, so nothing about the +// field's own spelling names the enum and the refusal follows the fold. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Max + +table Fleet +{ + ships [SlotCount]int32 +} diff --git a/test/tables/enumbound/BodyCount.schema b/test/tables/enumbound/BodyCount.schema new file mode 100644 index 000000000..d54895879 --- /dev/null +++ b/test/tables/enumbound/BodyCount.schema @@ -0,0 +1,13 @@ +package ebbodycount + +// `[E.Count]T` IN A TABLE BODY (docs/SPEC-TABLES.md §2.4, §11): a second +// spelling of one bound. `Count` is the declared variant count and `Max` the +// extent, and under no headroom they are the same number, so the array is the +// same positional vocabulary either way. + +enum ShipType { Fighter, Bomber, Scout } + +table Fleet +{ + ships [ShipType.Count]int32 +} diff --git a/test/tables/enumbound/BodyFolded.schema b/test/tables/enumbound/BodyFolded.schema new file mode 100644 index 000000000..556581248 --- /dev/null +++ b/test/tables/enumbound/BodyFolded.schema @@ -0,0 +1,16 @@ +package ebbodyfolded + +// `[N]T` UNDER A CONSTANT THAT FOLDS THROUGH ANOTHER CONSTANT +// (docs/SPEC-TABLES.md §2.4, §11): the fold runs at any depth of constant +// arithmetic (SPEC.md §4.2), so a bound two constants and one operator away +// from the enum is the same bound. + +enum ShipType { Fighter, Bomber, Scout } + +const Base = ShipType.Max +const SlotCount = Base * 2 - Base + +table Fleet +{ + ships [SlotCount]int32 +} diff --git a/test/tables/enumbound/BodyMax.schema b/test/tables/enumbound/BodyMax.schema new file mode 100644 index 000000000..7e849ca67 --- /dev/null +++ b/test/tables/enumbound/BodyMax.schema @@ -0,0 +1,12 @@ +package ebbodymax + +// `[E.Max]T` IN A TABLE BODY (docs/SPEC-TABLES.md §2.4, §11): the bound names +// the enum directly, which is the one spelling the checker already refuses. +// The unit stands beside the four below so the whole shape list is one corpus. + +enum ShipType { Fighter, Bomber, Scout } + +table Fleet +{ + ships [ShipType.Max]int32 +} diff --git a/test/tables/enumbound/ControlPacket.schema b/test/tables/enumbound/ControlPacket.schema new file mode 100644 index 000000000..4c29830d0 --- /dev/null +++ b/test/tables/enumbound/ControlPacket.schema @@ -0,0 +1,23 @@ +package ebcontrolpacket + +// THE POSITIVE CONTROL, THE PACKET WIRE (docs/SPEC-TABLES.md §2.4): a unit no +// table reaches spells every one of the five bounds and compiles. The packet +// wire is positional by construction and the connect gate covers a variant +// insert, because the protocol id moves with the extent, so the refusal must +// not reach here. A control that goes red says the refusal took a spelling +// from the packet wire on its way to the table wire. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Max +const CountSlots = ShipType.Count +const Folded = SlotCount * 2 - SlotCount + +type Loadout +{ + direct [ShipType.Max]int32 + counted [ShipType.Count]int32 + named [SlotCount]int32 + named2 [CountSlots]int32 + folded [Folded]int32 +} diff --git a/test/tables/enumbound/ControlPlain.schema b/test/tables/enumbound/ControlPlain.schema new file mode 100644 index 000000000..387bbe87a --- /dev/null +++ b/test/tables/enumbound/ControlPlain.schema @@ -0,0 +1,26 @@ +package ebcontrolplain + +// THE POSITIVE CONTROL, A BOUND THAT FOLDS FROM NO ENUM (docs/SPEC-TABLES.md +// §2.4): a literal bound and a constant bound stand in a table body and in a +// union arm the table body carries. The rule reads the bound's PROVENANCE, so +// a positional array is refused for reaching an enum and never for being +// positional. A control that goes red says the walk stopped reading provenance +// and started refusing fixed arrays. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = 4 * 2 + +union Payload +{ + ships [SlotCount]int32 + plain int32 +} + +table Fleet +{ + literal [8]int32 + named [SlotCount]int32 + keyed [ShipType]int32 + payload Payload +} diff --git a/test/tables/enumbound/ControlTypeHeld.schema b/test/tables/enumbound/ControlTypeHeld.schema new file mode 100644 index 000000000..2af3a2def --- /dev/null +++ b/test/tables/enumbound/ControlTypeHeld.schema @@ -0,0 +1,22 @@ +package ebcontroltypeheld + +// THE FENCE, schema#606's CASE (docs/SPEC-TABLES.md §2.4): a `type` a table +// reaches keeps the spelling. The hazard is real one body away and the two +// answers that close it exclude each other, so the page states neither until +// the ruling lands and this refusal stops at the table body and the union arm. +// A control that goes red says the walk crossed into the owner's case. + +enum ShipType { Fighter, Bomber, Scout } + +const SlotCount = ShipType.Max + +type Inner +{ + direct [ShipType.Max]int32 + named [SlotCount]int32 +} + +table Root +{ + inner Inner +} diff --git a/test/tables/enumbound/README.md b/test/tables/enumbound/README.md new file mode 100644 index 000000000..720b519e1 --- /dev/null +++ b/test/tables/enumbound/README.md @@ -0,0 +1,16 @@ +# `test/tables/enumbound`: the enum-bound refusal's corpus + +One unit a SHAPE, for the rule docs/SPEC-TABLES.md §2.4 and §11 state on the +bound's PROVENANCE: a positional array whose bound folds from an enum is +refused in a TABLE BODY and in a UNION ARM, and `[E]T` is the table form. + +`Body*.schema` and `Arm*.schema` are the refused shapes, one file for each +spelling the bound has in each of the two scopes. `Control*.schema` are the +units the rule leaves alone: the packet wire, a bound that folds from no enum, +and the `type`-held case schema#606 rules on. + +`internal/check/tables_test.go` reads this directory and asserts each file's +answer, so a shape is red the moment it compiles or the diagnostic stops +naming the field, the enum and the fix. Nothing here is swept by `make check` +or `make fmt`: the refused units are the point, and a corpus `make check` +sweeps must compile.