parser: fix "Scope mismatch while visiting" panic from macro tagged templates#31693
Conversation
…emplates When a tagged template's tag resolves to a macro import, every macro dispatch path in e_template (dead control flow, macros disabled, node_modules, macro call failure) replaced the expression and returned before visiting the template parts. Scopes recorded during the parse pass for arrows/functions inside the interpolations were then never consumed by the visit pass, panicking with "Scope mismatch while visiting" on the next scope push. Visit the parts right after the tag, before the macro dispatch, the same way e_call visits its arguments before macro handling. Also syncs Cargo.lock with bun_bin's manifest (bstr was added to Cargo.toml without the lock update).
|
Warning Review limit reached
More reviews will be available in 31 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Comment |
|
Updated 2:05 AM PT - Jun 2nd, 2026
❌ @robobun, your commit c4473c3 has 7 failures in
🧪 To try this PR locally: bunx bun-pr 31693That installs a local version of the PR into your bun-31693 --bun |
StatusReproduced on current main (debug) and release import { mac } from './macro.ts' with { type: 'macro' };
mac`a${() => { let q = 1; }}b`; // panic: Scope mismatch while visitingplus the dead-flow ( Cause: Fix: visit parts before the macro dispatch (mirrors Waiting on CI. |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
Not adding |
There was a problem hiding this comment.
I didn't find any issues — the reorder is sound and mirrors e_call — but since this is in the core parser visit pass, a quick human glance would be good before merge.
Extended reasoning...
Overview
This PR reorders the e_template visit in src/js_parser/visit/visit_expr.rs so that template interpolation parts are visited immediately after the tag and before the macro-dispatch block. Previously, all four macro-dispatch outcomes (dead-flow, no_macros, node_modules, macro-call failure) returned early without visiting the parts, leaving their parse-pass scope entries unconsumed and tripping the Scope mismatch while visiting panic on the next scope push. The fix is a pure code-motion: the existing for part in e_.parts_mut() loop moves up ~90 lines, and the enclosing if let Some(tag) is split into two checks so the loop sits between them. Three regression tests are added (live tag, dead-flow, namespace-member tag), plus a one-line Cargo.lock sync for bun_bin's bstr dep.
I verified that the non-macro path (regular templates and non-macro tagged templates) has identical visit order before and after: tag → parts → fold. The only behavioral change is for macro-tagged templates, where parts are now visited before the early returns — which is exactly the invariant the scope-replay mechanism requires, and matches how e_call orders its args visit (lines ~2019-2021) before its macro dispatch (lines ~2174+). The tag binding moving from pre-visit to post-visit only affects tag.loc in one error message, which is benign.
Security risks
None. No auth, crypto, permissions, or untrusted-input handling is touched. The change is internal AST-visit ordering.
Level of scrutiny
Moderate-to-high. The diff itself is small, mechanical, and follows the same pattern as three prior merged fixes in this bug family (#31231 / #31340 / #31533). However, visit_expr.rs is on the hot path for every JS/TS file Bun transpiles, so a regression here has very wide blast radius. That's the only reason I'm deferring rather than approving outright.
Other factors
- No bugs flagged by the bug-hunting system.
- No CODEOWNERS cover these paths.
- Regression tests cover all three reported variants and assert both the absence of the panic and the presence of the intended error/output.
- The PR notes that the related macro/transpiler/template-literal/bundler_string suites pass locally.
- The
Cargo.lockchange is a trivial lockfile regeneration from a prior commit's manifest edit.
Repro
Any tagged-template macro invocation whose interpolations contain a scope-creating expression (arrow, function, class) panics instead of reporting the intended
template literal macro invocations are not supportederror. The dead-code variant (false && mac…``), the macros-disabled path, and the node_modules path crash the same way. Panics on release builds (kind mismatch) and debug builds (loc mismatch); same structure existed in the Zig-eravisitExpr.zig`, so this predates the Rust port. Crash signature matches Sentry BUN-3BYK (`Scope mismatch while visiting`, also seen in Zig-era releases).Cause
The parser records every scope pushed during the parse pass in
scopes_in_order; the visit pass replays them in the same order and panics on divergence. Ine_template(src/js_parser/visit/visit_expr.rs), when the tag resolves to a macro ref, every dispatch outcome returns before thefor part in e_.parts_mut()visit loop:is_control_flow_dead→ replaced withundefinedno_macros→ error +undefinednode_modules→ error +undefinedmacro_context.call(...)failure → plainreturn(and template invocations currently always fail withtemplate literal macro invocations are not supported,src/js_parser_jsc/Macro.rs)The scopes recorded for arrows/functions inside the interpolations are never consumed, so the next scope the visit pass pushes reads a stale entry and trips the
Scope mismatch while visitingcheck. Same bug family as #31231 / #31340 / #31533 (constructs dropped without consuming/discarding their recorded scopes).Fix
Visit the template parts right after visiting the tag, before the macro dispatch — the same ordering
e_calluses (arguments are visited before its macro handling). All dispatch paths may then freely replace the expression: the parts' scope entries have already been consumed. The fall-through case no longer re-visits parts.Also syncs
Cargo.lockwithbun_bin's manifest (bstrwas added toCargo.tomlin 90f334a without the lock update; any local cargo invocation regenerates this line).Verification
test/bundler/transpiler/scope-mismatch-panic.test.ts(live tag, dead-flow, namespace-member tag). All three panic without the fix and pass with it.test/bundler/transpiler/{scope-mismatch-panic,macro-test,transpiler,template-literal}.test.tsandtest/bundler/bundler_string.test.tsall pass.Note: #30545 (tagged-template macro support, feature) inserts a parts visit before the macro call, which would cover the live path but not the dead-flow / macros-disabled / node_modules returns. This fix is independent and minimal; #30545 rebases on top by dropping its duplicate visit loop (its fold-flag wrapper can stay).