Skip to content

fix: make evaluation, type checking and syntax rules agree - #34

Open
myzie wants to merge 14 commits into
mainfrom
fix/consistent-value-rules
Open

myzie wants to merge 14 commits into
mainfrom
fix/consistent-value-rules

Conversation

@myzie

@myzie myzie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

What changes for the user

expr now gives the same result for an expression on every evaluation path. Check now infers every value Run can produce. Syntax features combine correctly. string, sprintf, templates, Compile and Check stay bounded on hostile input.

See it

The same program run on main and on this branch:

                               main                                   this branch
pp.Name     (pp is **User)     ada                                    ada
pp?.Name                       error: cannot select "Name" on **User  ada
id == 9007199254740992         true                                   false
  (id is ID(2^53+1))
m[1.5]      (map[int]string)   one                                    error: map key must be integer, got 1.5
Check of a 2,000-hop ?. chain  17.959s                                6ms

Why

Hosts pass expr many shapes of Go value. Before this PR the same value could give different results by path. The env shape, optimization, or . versus ?. could change the result. Check could accept an expression whose result had a type outside the inferred one. Some host values crashed the process, and some valid inputs took minutes to compile or check. This PR moves each rule into one implementation and tests every path against it.

Where to start reading

Read the commits in order. Each commit is one rule, its callers, its tests and its spec text.

  1. 9be96df…9b32ad0: fixes found before this session (map keys, conversions, check unions, syntax rewrites, cyclic map keys).
  2. 6ae1809: access.go holds the one implementation of x.y, x[i], ?., ?[ and env lookup. One cached callSig serves every host function call.
  3. 2558fee: an equivalence test for each optimization.
  4. 06f6e0d: bounds on formatting, Calls() and predicate errors. Crash-prone cases run in child processes.
  5. d49fe69, 8b64847: Check soundness harness and 8 soundness fixes.
  6. 69bea8b…efaa422: shared source scanner, linear template parsing, correct error positions.
  7. 4f1e32b: Check and Identifiers are linear in nesting and chain length.
  8. ffa03e8: generated host values and function signatures.

Decisions to check

  • filter(xs, p)[k] runs p on every element, as filter(xs, p) does. Errors and host calls must not depend on the optimization. Benchmark_filterFirst is 131x slower (0.5µs to 68µs). find(xs, p) stops at the first match. Rejected: stop early and accept different errors.
  • Integers compare exactly at any width and signedness. A float on either side still compares as float64. Rejected: the old spec rule, compare all numbers as float64. That made 2^53+1 == 2^53 true while > said otherwise.
  • An integer map key accepts only an integral float. Slice indexes already follow this rule. A float passed to an integer function parameter still truncates, as Go's conversion does. This is now documented. Rejected: reject fractional floats for parameters too. That would break hosts that rely on truncation.
  • Formatting can't fail, so its bounds appear as markers in the text: <cycle>, <...> after 1000 levels, <truncated> after 2^20 values. Rejected: return an error from string(). That would change a signature that never fails.
  • ?. and ?[ treat any typed nil as nil, the same nil that == nil reports. Rejected: short-circuit on an untyped nil only.

How to verify

go vet ./... && go test ./...                        # about 6s
go test -run 'TestValueMatrix|TestCallMatrix|TestEquivalence|TestApplyBinary' -v .
go test -run TestIsolatedCrashRegressions -v .       # 68 cases, each in a child process
go test -run TestStaticAnalysisGrowthIsLinear -v .   # x3–x7.5 time for x4 input
EXPR_HOSTGEN_SEEDS=20000 go test -run 'TestHostValuesRandom|TestHostFuncsRandom' .
go test -run xxx -fuzz=FuzzHostValues -fuzztime=60s .

Evidence in the test suite:

  • Value matrix: 47 Go values, about 120 expressions each. Each runs with a map env and a struct env, optimized and not. On the code before this session it reports 109 disagreements.
  • Syntax matrix: 21,600 cases compare each form with its parenthesized form. On the old rewrite code, 4,180 cases disagree.
  • Check soundness: a generated schema, values and expressions. A result must fit its inferred type, and Run must not fail with a type error that Check missed.
  • Generated host values: an access path is built while the value is walked, so each read has a known answer. A generated function records the arguments it gets. 20,000 seeds and 40,000 fuzz runs pass.

Risk and rollout

No API changes. Some behavior changes, all in docs/reference/spec.md:

  • **T receivers now work with ?. and ?[. A *[N]T can be indexed.
  • ?. on a typed nil func, chan or map gives nil, not an error.
  • Integer equality and ordering are exact. Equality used to round above 2^53.
  • m[1.5] on an integer-keyed map fails. It used to read m[1].
  • A function with an unsupported result shape fails before it runs. It used to run first.
  • Map entries in string() sort ties by key type, then by value.
  • Check infers narrower types in more places. It also reports some expressions it used to accept: computed object keys and always-nil results.
  • Error messages quote a higher-order predicate only when the predicate is 64 syntax nodes or fewer.

Benchmarks are within ±8% of main, except Benchmark_filterFirst (see above). Compile allocates more for rewritten expressions, about 5.4 KB instead of 4.5 KB for a ?. example.

Not in this PR

  • Structs as objects. Check types a struct as an object, but len, has, keys, entries and o["k"] reject structs at run time. Only . and ?. work. One side should change; this needs a decision.
  • Multi-line JSON literals without trailing commas still fail, because Go inserts semicolons. This is documented today.
Reviews
  • No independent implementation review has run yet. The type-checking and syntax work came from two subagents in separate worktrees. I merged their commits and ran the full suite on the result.

🤖 Generated with Claude Code

myzie and others added 14 commits November 1, 2026 22:05
mapKey, convertValue, callResultErr and wrapCallErr replace three
copies of map-key conversion and two of call-result handling. Unhashable
keys, short slice-to-array conversions, incomparable interface-holding
structs, concrete error returns and nil Funcs now return ErrEvaluate
instead of panicking. filter(xs, p)[k] runs the predicate on every
element, as the general path does. sum promotes to float regardless of
element order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ypes

Two nullable sides may be equal; a computed object key may replace any
earlier field; a nullable enum key may be nil; slice of a string|list
union keeps both kinds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jsonlit no longer reads [6][0] * 2 as an array-of-pointer type; ?. and ?[
accept typed slice/array/map literals; optaccess rewrites a chain in one
linear pass; ${{}} renders an empty object.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cyclic map keys rendered through string(), sprintf and templates now
print <cycle> instead of overflowing the stack. The regression runs in
a subprocess so a crash cannot take down the suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Selection and indexing had three copies each (general, reflect-space
chain, optional); env lookup two; reflect calls three. They now share
selectRV/indexRV (strict or optional mode), lookupEnvRV, and a cached
callSig used by registered functions, env functions, struct-field
functions and bound methods.

Rules this pins down, all in the spec:
- receivers unwrap through pointers and interfaces at any depth, so
  **T works under ., ?., [] and ?[ alike;
- ?. and ?[ treat any typed nil as nil;
- integers compare exactly (named int types, uint64 above MaxInt64),
  instead of through float64;
- an integer map key takes only an integral float, like slice indexes;
- an unsupported result signature fails before the function runs.

Did-you-mean hints were nondeterministic between equally close names;
candidates are now sorted first.

value_matrix_test.go runs 47 host values (typed nils, interface-holding
structs, short slices, concrete errors, wide and named integers) through
a map env and a struct env, optimized and not, and checks the documented
relations between ., ?., try, ==, != and truthiness. call_matrix_test.go
does the same for 28 function shapes across registered, env and field
calls. Against the previous code the value matrix reports 109
disagreements.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
withoutOptimizations compiles a program that skips constant folding,
the prepared-call cache, the reflect selector and index chains,
filter(...)[k] streaming and the typed binary operators. Each
optimization now has an equivalence test (optimize_equiv_test.go) next
to its benchmark, comparing values, error categories and messages, and
host-function calls in order; bench_test.go points at them.

The tests found three places where optimized and general evaluation
differed:
- -9223372036854775808 only parsed when folding ran; the MinInt64
  literal rule is part of the language and now always applies.
- int64 == int64 went through float64 on the fast path, so
  2^53+1 == 2^53 was true there (and while < said otherwise).
- filter(xs, p)[k] out of range omitted the [0, n) the general path
  reports, including for a nil list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Formatting now threads one state through its recursion, map keys
included, carrying cycle detection, a depth bound (1000, then <...>)
and a work bound (2^20 values, then <truncated>). A list holding the
same list twice, nested 64 times, rendered 2^64 leaves; a 200,000-deep
acyclic list overflowed the stack. Templates check the same bounds
before handing a composite to encoding/json, which has neither.

isCompositeKind looped forever on a self-referential pointer type; it
now uses the bounded unwrapReceiver.

Calls built every argument's Source and Identifiers eagerly, which is
cubic in the nesting of calls (a 64 KiB pipe chain ran for minutes);
they are now computed on first use. Errors quoted every enclosing
predicate in full at each nesting level of a form (a 64 KiB nest of
map(...) took over a minute to fail); predicates larger than 64 nodes
are no longer quoted.

TestIsolatedCrashRegressions runs 68 crash-shaped cases, each in a
child process with a 64 MiB stack and a timeout, and absorbs the
cyclic map-key regression from format_regression_test.go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nown

An inferred type now includes every value Run can produce from inputs
the env types allow, and stays checked where the value is known:

- A computed object key opens the object with the key's value as the
  type of its unlisted fields, which may be absent; an enum key adds
  each value as a field that may be absent. {"a": 2, k: c}.b is now
  reported, and ?.b types as bool?.
- first, last and find of a nil or always-empty list type as nil, as
  do map/filter/sortBy/flatMap over nil (an empty list).
- An operation that always fails (indexing with nil, reading a field of
  nil) types as never, which drops out of a join: try(xs[nil], "s") is
  "s". ?[nil] of a nil receiver is still nil.
- flatMap adds a non-list body as one element, so its element type is
  the body's (minus nil), not any.
- o?[k] on a closed object with no fields is nil.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
check_soundness_test.go pairs random schemas (nullable values, unions,
enums, absent fields, open objects) with representative values for
each (nil, every variant and enum value, empty and non-empty lists,
typed Go slices), and runs a corpus plus random expressions over every
operator, builtin and form. Where Check accepts an expression, each
successful Run result must fit the inferred type, and Run must not fail
on a wrong kind, nil receiver or missing field. Seeds are fixed;
EXPR_SOUNDNESS_SEED and EXPR_SOUNDNESS_ITERS widen the search.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
internal/srcedit holds the token scanner, bracket pairing and edit
splicing that jsonlit and optaccess each carried a copy of. Apply
also returns a map from output offsets back to input offsets, which
the compiler uses to report errors where the user wrote them.

Scan takes a raw string's end from the source, not from the
scanner's literal, which drops carriage returns.

Two rewrites become linear: jsonlit finds every bracket's top-level
comma in one pass instead of rescanning each nested `[` (a 64 KiB
nested array went from 1.8s to 18ms), and optaccess emits its edits
in source order and looks up openers in a table instead of walking
back to them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Parsing copied the rest of the template for every `${`, counted
lines from the start for every segment, and searched the whole
remainder for `$$` on every opener. A 64 KiB template of `${x?.a}`
segments allocated 209 MB. The template is now converted to bytes
once, lines are counted by a forward-only counter, and the next
escape position is cached. The same template now allocates 17 MB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each source rewrite (keyword fields, ?. / ?[, JSON literals, map/if)
shifted the positions of every later parse and validation error by the
bytes it inserted: `x.type ^ 1` reported 1:18. compileWithConfig now
composes the rewrites' offset maps, translates parser errors through
them, and on a validation or pipe error re-runs those passes with the
positions mapped back to the source, so a successful compile pays
nothing for the translation.

compileWithConfig documents the pipeline: each stage's order, input,
output, and the invariants later stages rely on. The engine's keyword
rewrites use the shared srcedit scanner, and the keyword-field rewrite
skips scanning when no `.` is followed by a keyword.

New coverage:
- TestSyntaxMatrix crosses 36 receivers, 24 access operators, 26
  surrounding contexts and 5 comment placements (21,600 cells) and
  requires each expression to behave like its parenthesized forms.
- TestSyntaxMatrixTemplates renders the same expressions under four
  delimiter pairs.
- TestSyntaxMatrixErrorPositions checks that an error after any matrix
  primary names its own line:column.
- TestCompileGrowthIsLinear bounds Compile and NewTemplate allocation
  as input doubles, up to MaxSourceLength, for 13 syntax shapes;
  benchmarks measure each shape and the rewrites alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ngth

Check looked names up by walking every enclosing form's scope, and
collectIdentifiers did the same for element bindings, so a nest of
6,000 forms took 380 ms; scopes now memoize what their parent answered
(9 ms). A chain of && or || recollected and reprinted the narrowings of
its whole left side at every operator, 7 s for a 64 KiB chain; the
chain is now checked as a list of operands, each narrowing added once.
applyNarrowing printed every optional-typed node to compare it with
the narrowed expressions, which did not finish within 90 s on a deep
?. chain under x != nil; it now skips nodes too large to print as any of them.

TestStaticAnalysisGrowthIsLinear times Compile, Check, Identifiers and
Calls on 14 shapes at a quarter and all of MaxSourceLength; every shape
grows between x3 and x7.5 for x4 input. Also gofmt check_test.go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hostgen_test.go draws bounded random Go types with reflect (structs with
interface fields, tags and unexported fields, pointers, arrays, maps
keyed by named, array and interface types), values with typed nils and
nested interfaces, and function signatures via reflect.FuncOf. Two
oracles make the checks semantic rather than crash-only: an access path
is built while walking the value, so V.A[0]?.B must return what the walk
found (or fail as missing, and be nil under ?.); and a generated
function records the arguments it received, which must equal any
argument already of the parameter's type. Every value also goes through
the value-matrix relations and every path. 400 seeds run by default
(EXPR_HOSTGEN_SEEDS widens it; 20,000 pass), FuzzHostValues fuzzes the
seed (40,000 executions clean), and seeds that found bugs are kept.

It found two bugs, now fixed and kept as named regressions:
- reflect.Value.Comparable reports [0][2]map[K]V comparable (it checks
  each of no elements) while == on it panics; canCompare checks the type
  too, for equality and map keys.
- map keys that render alike (1 and uint16(1) in a map[any]V) fell back
  to map order, so string(m) varied between runs; ties now break on key
  type, then rendered value.

CLAUDE.md records the new conventions: optimizations need equivalence
tests, value rules stay in the shared helpers, crash-shaped tests run
isolated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@mobiusops mobiusops Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve: The access, optional-access, and comparison paths I read match the spec, and I found no bug in them.

This moves the value-shape rules into access.go and its shared callers, as CLAUDE.md now asks. The behavior changes match docs/reference/spec.md.

Nits

  • N1. The doc comment on applyInt64Binary still says equality compares as float64. It now compares exactly (l == r). (program.go:730)

Sensitive paths

  • CLAUDE.md: adds the rules for access.go, the optimization equivalence tests, and the isolated-test rule. They match the new file layout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant