fix(vm): resolve OP_SEED/OP_TAILCALL opcode collision#562
Merged
Conversation
PR #503 (seed builtin) assigned OP_SEED = 189 and PR #518 (VM tail-call optimisation) later assigned OP_TAILCALL = 189. In the dispatch match OP_SEED is listed first and silently shadows OP_TAILCALL, so VM tail-call dispatch has been broken on main since the TCO landed. Clippy also promotes the resulting unreachable-pattern warning to an error, blocking every PR's lint check. Renumber OP_SEED to 190 (next free byte). OP_TAILCALL keeps 189 since it's on the hot dispatch path and has been wired up longer. No other references to OP_SEED need updating - the constant is named, not spelled-out, in the compile/jit/cranelift sites.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Summary
PR #503 (seed builtin) and PR #518 (VM tail-call optimisation) both assigned opcode
189insrc/vm/mod.rs.OP_SEEDis listed first in the dispatch match and silently shadowsOP_TAILCALL, so VM tail-call dispatch has been broken onmainsince TCO landed.The collision also produces a clippy
unreachable_patternswarning. With-D warningsin CI that becomes an error, blocking every PR's lint check until this lands.Fix
Renumber
OP_SEEDfrom 189 to 190 (next free opcode byte, verified by agrep -n "u8 = 19[0-9]"scan).OP_TAILCALLkeeps 189 because it is on the hot dispatch path and has been wired up longer.No call sites need updating - the opcode is referenced via the
OP_SEEDsymbol insrc/vm/compile_vm.rs,src/vm/jit_cranelift.rs, andsrc/vm/compile_cranelift.rs, so the constant change is enough.Test plan
cargo build --release --features craneliftcleancargo clippy --release --features cranelift --all-targets -- -D warningsclean (no more unreachable_patterns)cargo test --release --features cranelift --test regression_tco --test regression_tco_vm --test regression_rng_parity --test regression_rand_bytesall passcargo test --release --features craneliftgreen except for one pre-existing failure inregression_reserved_names_doc::spec_reserved_short_names_match_builtin_registry, caused by the crypto-primitives PR that just landed addingb64/hexbuiltins without updating SPEC.md - unrelated to this fix