Lazy if, eval budget, compile-time registration checks, Identifiers(), and opt-in helper groups#25
Merged
Merged
Conversation
…rs, and helper groups A batch of reliability and ergonomics improvements: - `if(cond, then, else)` is now a lazy special form instead of an eager builtin: only the branch the condition selects evaluates, so guard idioms like `if(n != 0, total/n, 0)` work. It is always available (no WithBuiltins needed) and shadowable like every other form. - `WithEvalBudget(n)` bounds the AST nodes a single Run may evaluate, giving hostile nesting (`map(xs, map(xs, map(xs, it)))`) a deterministic, cheap failure instead of burning a core until the context deadline. - `WithFunctions` registrations are validated at Compile time: nil entries, non-function values, and unsupported signatures fail with ErrCompile instead of hiding until the first call. - `Program.Identifiers()` exposes the sorted set of env-resolved names the expression references, for load-time env validation and dependency tracking. - Runtime-error panic recovery now covers every reflect dispatch path (registered functions and env callables, not just bound methods). - New opt-in builtin groups kept out of the default sandbox surface: MathFuncs (min, max, abs, floor, ceil, round), StringFuncs (trim, split, join, replace, startsWith, endsWith), and CollectionFuncs (first, last, sum, slice). - CI gains a short fuzz smoke step over all six fuzz targets. Docs (spec, guides, llms.txt, README) updated in lockstep, with new docs-honesty tests pinning the changed claims. https://claude.ai/code/session_01Nw3onLb6YHSRU1kjnKdaph
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the highest-value items from the post-review improvement list (#24 follow-up discussion), focused on reliability, robustness, and ergonomics. Everything stays backwards compatible except where the old behavior was itself the bug (details below).
What's in here
if(cond, then, else)is now a lazy special formThe eager
ifbuiltin evaluated both branches, so the natural guard idiom failed:ifmoves out ofBuiltins()and into the special-form machinery alongsidemap/filter/try, so only the selected branch evaluates. It is now always available (noWithBuiltinsneeded) and shadowable through the usual env→funcs rules — a user-registeredifstill wins and restores eager calling. The dispatch path for keyword-backed forms (map,if) was unified so shadowing always checks the user-visible name.Compatibility: only observable when the untaken branch would have errored — exactly the case users want fixed — or when calling
ifwithoutWithBuiltins, which previously errored and now works.WithEvalBudget(n): deterministic work boundMaxSourceLengthandMaxEvalDepthbound memory and stack but not CPU:map(xs, map(xs, map(xs, it)))over a 10k-element env list is 10¹² predicate evaluations from a ~30-byte expression, and a context deadline still lets it burn a core for the full timeout. The budget counts one unit per AST node evaluated (including each per-element predicate re-evaluation) and fails the Run withErrEvaluate: evaluation budget exceededthe moment it's exhausted. Each Run gets the full budget via a shallow Program copy, so concurrent Runs never share a counter, andtry(...)can't catch its way past exhaustion. Default unlimited; zero hot-path cost when unused (one nil check per node).Compile-time registration validation
WithFunctionspreviously swallowedprepareFuncerrors, so{"f": 42}, nil entries, and bad signatures (three returns, second return noterror) compiled fine and failed only when called.Compilenow collects and returns these wrapped inErrCompile— load-time failure is the point of the Compile/Run split.Compatibility note: this is the one deliberate tightening. Registrations that could never be called successfully now fail at
Compileinstead of at call time, which also closes the undocumented loophole of registering non-function constants viaWithFunctions(constants belong in the env, which is unchanged).Program.Identifiers()Sorted, deduplicated set of env-resolved names the expression references, computed once at compile. Excludes literals,
it/indexwhere an iterating form binds them, registered function names, and special-form names in call position — so hosts can validate expressions against a known env shape at load time ("strict mode" for free), track dependencies, or invalidate caches. Statically shadowing-aware where possible (a registeredfilterdisables theitbinding in the walker too); the unavoidable env-shadowing corner is documented.Panic recovery on every reflect dispatch path
Bound methods already recovered
runtime.Errorpanics; registered functions and env-stored callables didn't, so one buggy callee took down the host throughRun. The same recover wrapper now covers all three reflect dispatch paths. Deliberatepanic(v)with non-runtime values still propagates.Opt-in builtin groups
Builtins()stays minimal so the sandbox story stays clean; the most-requested helpers ship as opt-in sets that every host previously re-implemented slightly differently:expr.MathFuncs()—min,max(variadic, int64-preserving),abs(MinInt64-checked),floor,ceil,roundexpr.StringFuncs()—trim,split,join,replace,startsWith,endsWithexpr.CollectionFuncs()—first,last,sum(overflow-checked),slice(xs, i, j)(covers the rejectedxs[i:j]syntax; negative indices, clamping, runes for strings)All pure, deterministic, allocation bounded by input size.
CI: fuzz smoke
15s runs of all six fuzz targets on every push/PR — the rewriter bugs fixed in #24 were exactly the kind of thing seeds alone missed.
Docs
Spec (
ifmoved to special forms, new groups section, budget under limits, registration validation), sandboxing/registering-functions/templates/higher-order-patterns guides,llms.txt, README, and the sandboxing example updated in the same change, with new docs-honesty tests (TestGuide_*,TestDocsExample10_*) pinning the changed claims per the repo convention.Testing
go build ./...,go vet ./...,gofmtcleango test -race -count=1 ./...passes (existingif-eagerness and call-time-signature tests updated to the new intended semantics)budget_test.go,identifiers_test.go,builtin_groups_test.go,registration_test.goFuzzEvalsmoke run passed locallyhttps://claude.ai/code/session_01Nw3onLb6YHSRU1kjnKdaph
Generated by Claude Code