fix(compiler): user-rebound 'dispatch' wins over OP_DISPATCH; W012/W013 derive from the registry (#459)#462
Merged
Conversation
…13 derive from the registry (#459) The name-keyed dispatch superinstruction compiled 'dispatch of [t, k, a]' to OP_DISPATCH on the name alone, silently ignoring any user binding — including a write-through 'dispatch is ...' inside a function body (in_globals makes that mutate the global builtin binding). The guard is a unit-level scan (all scopes, all binder positions, plus the eval dynamic escape E003 honors) and a compile-time env check for the REPL/embed sequential-unit case; every guard fails open to the semantically identical normal call path. The parenthesized #355 form now takes the normal path too (one argument, per the #405 contract). Decided: report/report_value/observe on a plain variable are observer special forms (compiler-resolved to the binding's slot trajectory, like predicates and interrogatives); rebinding does not change them and W013 now warns on the attempt. W012/W013's builtin set derives from register_builtins() + ext_names.h (the E003 derivation) instead of a hand list that had drifted ~120 names behind the binary. The scratch env is freed per lint run: LeakSanitizer cannot trace tagged EigsSlot pointers, so keeping it fails detect_leaks=1. Gates: release 2578/2578, ASan+UBSan detect_leaks=1 2578/2578, embed_stack_soak OK. Closes #459 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes the silent hijack from the #405 review: the name-keyed
dispatchsuperinstruction ignored any user binding of the name.The three facets of #459
1. Silent hijack (the real bug).
dispatch of [table, key, arg]compiled toOP_DISPATCHon the identifier name alone. A userdefine dispatch(a, b, c)— or a plaindispatch is ...inside a function body, which writes through to the global builtin binding viain_globals— was silently bypassed. The guard now suppresses the superinstruction when:dispatchanywhere (any scope, any binder position — the scan descends into function bodies because of the write-through rule), oreval(the same dynamic escape E003 honors), orbuiltin_dispatchbinding (REPL line N, embed eval — sequential units in one env).Every guard fails open to the normal call path, whose builtin fallback is semantically identical, so a spurious hit only costs the fast path. Known residual (documented in the emit-site comment): a
load_file'd module that rebindsdispatchfor a sibling unit compiled earlier — cross-unit, same class as E003's boundary.2. W013/W012 blind spot. The lint's hand-copied
BUILTINS[]array was ~120 names behind the binary (dispatch,chr,eval, every post-list builtin shadowed clean).is_builtin_namenow derives fromregister_builtins()+register_store_builtins()+ext_names.h+ the compiler-resolved special forms — the E003 derivation, per that pass's own "never a hand list" rule. The scratch env is freed at the end of every lint run (eigenscript_lintandlint_collect): LSan can't trace taggedEigsSlotpointers, so a kept env fails thedetect_leaks=1gate (caught by the ASan suite run, fixed in this PR).3. The #355 paren form.
dispatch of ([t, k, a])is ONE argument by the #405 contract; it now takes the normal call path instead of relying on the builtin's coincidental agreement. Pinned in test_dispatch.eigs.The decided position on the observer forms
report/report_value/observeon a plain variable are observer special forms — compiler-resolved to the named binding's slot trajectory, an operation on the name (like predicates and interrogatives), not the value. Rebinding does not change them; W013 now warns on the attempt. Rationale: routing them through the guard would silently flip pre-rebind calls from slot-trajectory classification to the value-path builtins — a subtler wrong answer than the one being fixed.dispatchhas no such excuse (plain builtin, identical fallback), so rebinding it wins. Documented in BUILTINS.md + DIAGNOSTICS.md.Tests
test_dispatch_rebind.eigs— module-scope define/reassign rebinds + paren-form-with-rebind (3)test_dispatch_rebind_fn.eigs— fn-body write-through rebind; also proves fail-open preserves pre-rebind builtin behavior (2)test_dispatch_rebind_eval.eigs— dynamic escape: rebind only via eval'd code (1)test_dispatch.eigs— paren-form/opcode agreement pin (facet 3)test_lint.sh— W013 ondispatchandreport, W012 onchrOP_DISPATCH(verified viaEIGS_DUMP_BC).Gates
detect_leaks=1: 2578/2578embed_stack_soak: OK (the scan is recursive over the AST like the existing walkers; no big stack frames)Downstream
None needed (verified, correcting the earlier assumption): ouroboros has no dispatch special case anywhere in src/ or aot/ — it compiles
dispatch of [t, k, a]as a generic named call, which already honors user rebinding. This change only NARROWS the pre-existing (behaviorally equal) bytecode difference on clean units.Closes #459
🤖 Generated with Claude Code