fix(semantic): hoist Annex B block-scoped function declarations to var scope#20728
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Merging this PR will not alter performance
Comparing Footnotes
|
8ba94c9 to
7da639e
Compare
|
@sapphi-red, could you take a look at the handling in Semantic, which looks clean and follows the spec. |
sapphi-red
left a comment
There was a problem hiding this comment.
Implementation looks good to me.
I think we should add a test that ensures
console.log(`typeof foo is ${typeof foo}`);
if (true) {
function foo() {
return 1;
}
}is minified to
console.log(typeof foo); // should not be replaced with 'function' nor 'undefined'
if (true) {
function foo() {
return 1;
}
}just in case.
7da639e to
b4c07df
Compare
14c83d6 to
23050fa
Compare
Added |
|
I will take a look at those failed semantic tests later. |
Merge activity
|
…r scope (#20728) This PR replaces the mangler-level fix in #20705 with a semantic-level fix that is simpler and allows better name reuse. ## Summary In sloppy mode JavaScript, function declarations inside blocks (`if`, `try`, `switch`, `{}`) have **Annex B.3.2.1** semantics: they create an implicit `var`-like binding in the enclosing function scope at block exit. The semantic analysis was not modeling this hoisting, which caused the mangler to assign conflicting names to block-scoped functions and outer `var` bindings — changing program behavior at runtime. ### How the fix works Hoist plain (non-async, non-generator) function declarations from block scopes to the enclosing var scope during binding, using the **same pattern as `var` hoisting** (binder.rs lines 46-98): 1. `move_binding(block_scope, var_scope, name)` — moves the binding 2. `set_symbol_scope_id(symbol_id, var_scope)` — updates the symbol's scope 3. `hoisting_variables.insert(block_scope, name, symbol_id)` — keeps a hoisting entry in the block scope for redeclaration checks This makes the mangler handle Annex B functions naturally alongside other var-scope bindings, with **zero mangler changes needed**. Name reuse from sibling scopes is preserved (unlike the mangler-level fix which was overly conservative). ### Guards - `is_declaration` — function expressions are bound in their own (var) scope - `!self.r#async && !self.generator` — Annex B only applies to plain functions - `!builder.source_type.is_typescript()` — Annex B is JavaScript-only - `!scope_flags.is_var()` — already in a var scope, no hoisting needed - `!scope_flags.is_strict_mode()` — no Annex B in strict mode / modules Fixes #20610. Fixes #14316. Downstream: [vitejs/vite#22009](vitejs/vite#22009), [rolldown/rolldown#8791](rolldown/rolldown#8791). ## Test plan - 13 sloppy-mode mangler snapshot test cases (all pass without mangler changes) - 975 linter tests pass - 40 semantic unit tests pass - Parser conformance: improved (babel negative 1687→1689, test262 negative stays 100%) - Semantic conformance: ~10 new mismatches in `semantic_test262` — these are transformer rebuild differences where the transformed JS output correctly gets Annex B hoisting that the original TS parse didn't have
412a86a to
9a5ff73
Compare

close: #20705
This PR replaces the mangler-level fix in #20705 with a semantic-level fix that is simpler and allows better name reuse.
Summary
In sloppy mode JavaScript, function declarations inside blocks (
if,try,switch,{}) have Annex B.3.2.1 semantics: they create an implicitvar-like binding in the enclosing function scope at block exit. The semantic analysis was not modeling this hoisting, which caused the mangler to assign conflicting names to block-scoped functions and outervarbindings — changing program behavior at runtime.How the fix works
Hoist plain (non-async, non-generator) function declarations from block scopes to the enclosing var scope during binding, using the same pattern as
varhoisting (binder.rs lines 46-98):move_binding(block_scope, var_scope, name)— moves the bindingset_symbol_scope_id(symbol_id, var_scope)— updates the symbol's scopehoisting_variables.insert(block_scope, name, symbol_id)— keeps a hoisting entry in the block scope for redeclaration checksThis makes the mangler handle Annex B functions naturally alongside other var-scope bindings, with zero mangler changes needed. Name reuse from sibling scopes is preserved (unlike the mangler-level fix which was overly conservative).
Guards
is_declaration— function expressions are bound in their own (var) scope!self.r#async && !self.generator— Annex B only applies to plain functions!builder.source_type.is_typescript()— Annex B is JavaScript-only!scope_flags.is_var()— already in a var scope, no hoisting needed!scope_flags.is_strict_mode()— no Annex B in strict mode / modulesFixes #20610. Fixes #14316.
Downstream: vitejs/vite#22009, rolldown/rolldown#8791.
Test plan
semantic_test262— these are transformer rebuild differences where the transformed JS output correctly gets Annex B hoisting that the original TS parse didn't have