mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(minifier): keep regenerated catch vars bound to surviving catch
`KeepVar` always rewrote a `var` that redeclares a catch parameter to the
enclosing var binding. When the regenerated `var` stays inside a catch that
is still referenced, a later pass merged `e = 2` into it and then inlined the
declarator away, dropping the hoisted binding:
function f() { try { throw 1 } catch (e) { while (0) { var e } e = 2; use(e) } return e }
// => ...catch (e) { use(2) } return e (ReferenceError)
Now the regenerated `var` keeps redeclaring the innermost surviving catch
parameter with the same name when that parameter is referenced, and links it
to the enclosing binding if it was a different (outer) catch.
Also:
- minifier: drop a catch-var initializer that only writes an unread catch
parameter while the enclosing binding is used (`var e = 2` -> `var e`).
- mangler: give a linked catch parameter the enclosing binding's slot
directly instead of allocating a slot that is then overwritten.
- mangler: skip the original declaration when scanning redeclarations for
keep_names, since it is already checked.
- semantic: look up intermediate-scope bindings once while hoisting `var`.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LCkrX7gAxgeMP9xtYxbkNa
This commit is contained in:
9 files changed
+160
-32
No files matched your search
@@ -76,7 +76,8 @@ impl<'a, 'b: 'a, 't> NameSymbolCollector<'a, 'b, 't> {
|
||||
}
|
||||
|
||||
fn has_name_set_redeclaration_node(&self, symbol_id: SymbolId) -> bool {
|
||||
self.scoping.symbol_redeclarations(symbol_id).iter().any(|redeclaration| {
|
||||
// The first entry is the original declaration, which `collect` already checked.
|
||||
self.scoping.symbol_redeclarations(symbol_id).iter().skip(1).any(|redeclaration| {
|
||||
let node = self.ast_nodes.get_node(redeclaration.declaration);
|
||||
self.is_name_set_declare_node(node, symbol_id)
|
||||
})
|
||||
|
||||
@@ -656,12 +656,19 @@ impl<'a> SlotAssignment<'a> {
|
||||
|
||||
// Sort `bindings` in declaration order.
|
||||
tmp_bindings.clear();
|
||||
tmp_bindings.extend(
|
||||
bindings
|
||||
.values()
|
||||
.copied()
|
||||
.filter(|&binding| constraints.is_mangle_candidate(binding, scoping)),
|
||||
);
|
||||
tmp_bindings.extend(bindings.values().copied().filter(|&binding| {
|
||||
// A catch parameter redeclared by `var` must print the enclosing binding's
|
||||
// name. That binding's scope is an ancestor, so its slot is already assigned
|
||||
// and kept live through this catch (see below). Share it instead of taking a
|
||||
// slot of its own.
|
||||
if !catch_var_bindings.is_empty()
|
||||
&& let Some(var_binding) = scoping.catch_var_binding(binding)
|
||||
{
|
||||
slots[binding.index()] = slots[var_binding.index()];
|
||||
return false;
|
||||
}
|
||||
constraints.is_mangle_candidate(binding, scoping)
|
||||
}));
|
||||
if tmp_bindings.is_empty() {
|
||||
continue;
|
||||
}
|
||||
@@ -804,14 +811,6 @@ impl<'a> SlotAssignment<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
// The printed `var` must declare the enclosing binding's name as well as assign
|
||||
// to the catch parameter. Unassigned outer slots preserve both original names.
|
||||
for (catch, outer) in catch_var_bindings {
|
||||
if slots[catch.index()] != SLOT_UNASSIGNED {
|
||||
slots[catch.index()] = slots[outer.index()];
|
||||
}
|
||||
}
|
||||
|
||||
let total_slots = slot_liveness.len();
|
||||
Self { slots, total_slots, eval_reserved_names }
|
||||
}
|
||||
|
||||
@@ -8,6 +8,8 @@ use oxc_syntax::symbol::SymbolId;
|
||||
|
||||
pub struct KeepVar<'a> {
|
||||
vars: Vec<(Ident<'a>, Span, Option<SymbolId>)>,
|
||||
/// Simple catch parameters whose catch clauses are inside the visited statement.
|
||||
removed_catch_params: Vec<SymbolId>,
|
||||
}
|
||||
|
||||
impl<'a> VisitJs<'a> for KeepVar<'a> {
|
||||
@@ -41,6 +43,16 @@ impl<'a> VisitJs<'a> for KeepVar<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
fn visit_catch_clause(&mut self, it: &CatchClause<'a>) {
|
||||
if let Some(param) = &it.param
|
||||
&& let BindingPattern::BindingIdentifier(id) = ¶m.pattern
|
||||
&& let Some(symbol_id) = id.symbol_id.get()
|
||||
{
|
||||
self.removed_catch_params.push(symbol_id);
|
||||
}
|
||||
self.visit_block_statement(&it.body);
|
||||
}
|
||||
|
||||
fn visit_variable_declaration(&mut self, it: &VariableDeclaration<'a>) {
|
||||
if it.kind.is_var() {
|
||||
it.bound_names(&mut |ident| {
|
||||
@@ -52,26 +64,29 @@ impl<'a> VisitJs<'a> for KeepVar<'a> {
|
||||
|
||||
impl<'a> KeepVar<'a> {
|
||||
pub fn new() -> Self {
|
||||
Self { vars: std::vec![] }
|
||||
Self { vars: std::vec![], removed_catch_params: std::vec![] }
|
||||
}
|
||||
|
||||
pub fn get_variable_declaration(
|
||||
self,
|
||||
ctx: &crate::TraverseCtx<'a>,
|
||||
ctx: &mut crate::TraverseCtx<'a>,
|
||||
) -> Option<ArenaBox<'a, VariableDeclaration<'a>>> {
|
||||
if self.vars.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let symbol_ids = self
|
||||
.vars
|
||||
.iter()
|
||||
.map(|&(name, _, symbol_id)| {
|
||||
symbol_id.map(|symbol_id| self.declared_symbol(name, symbol_id, ctx))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let builder = ctx.builder();
|
||||
|
||||
let kind = VariableDeclarationKind::Var;
|
||||
let decls = ArenaVec::from_iter_in(
|
||||
self.vars.into_iter().map(|(name, span, symbol_id)| {
|
||||
// Initializers are gone, so only the enclosing var binding is declared.
|
||||
// The catch scope itself may have been removed with the dead statement.
|
||||
let symbol_id =
|
||||
symbol_id.map(|id| ctx.scoping().catch_var_binding(id).unwrap_or(id));
|
||||
self.vars.into_iter().zip(symbol_ids).map(|((name, span, _), symbol_id)| {
|
||||
let id = symbol_id.map_or_else(
|
||||
|| BindingPattern::new_binding_identifier(span, name, builder),
|
||||
|symbol_id| {
|
||||
@@ -88,9 +103,52 @@ impl<'a> KeepVar<'a> {
|
||||
Some(VariableDeclaration::boxed(SPAN, kind, decls, false, builder))
|
||||
}
|
||||
|
||||
/// The symbol a regenerated `var name` declares where the visited statement was.
|
||||
///
|
||||
/// A `var` that redeclares a catch parameter carries the catch symbol, and the enclosing
|
||||
/// var binding is recorded separately. The regenerated `var` sits inside the innermost
|
||||
/// surviving catch clause with the same name, if any. When that catch parameter is
|
||||
/// referenced, the `var` must keep redeclaring it: later passes may merge an assignment
|
||||
/// to the parameter into the `var`, and only the catch symbol stops them from treating
|
||||
/// that as a plain declaration of the enclosing binding. Otherwise nothing in the catch
|
||||
/// can observe the difference, so the `var` declares the enclosing binding directly.
|
||||
fn declared_symbol(
|
||||
&self,
|
||||
name: Ident<'a>,
|
||||
symbol_id: SymbolId,
|
||||
ctx: &mut crate::TraverseCtx<'a>,
|
||||
) -> SymbolId {
|
||||
let scoping = ctx.scoping();
|
||||
let Some(var_binding) = scoping.catch_var_binding(symbol_id) else { return symbol_id };
|
||||
let var_scope_id = scoping.symbol_scope_id(var_binding);
|
||||
let mut scope_id = Some(scoping.symbol_scope_id(symbol_id));
|
||||
let mut enclosing_catch = None;
|
||||
while let Some(id) = scope_id
|
||||
&& id != var_scope_id
|
||||
{
|
||||
if let Some(binding) = scoping.get_binding(id, name)
|
||||
&& scoping.symbol_flags(binding).is_catch_variable()
|
||||
&& !self.removed_catch_params.contains(&binding)
|
||||
{
|
||||
enclosing_catch = Some(binding);
|
||||
break;
|
||||
}
|
||||
scope_id = scoping.scope_parent_id(id);
|
||||
}
|
||||
match enclosing_catch {
|
||||
Some(catch) if !scoping.symbol_is_unused(catch) => {
|
||||
if catch != symbol_id {
|
||||
ctx.scoping_mut().add_catch_var_binding(catch, var_binding);
|
||||
}
|
||||
catch
|
||||
}
|
||||
_ => var_binding,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_variable_declaration_statement(
|
||||
self,
|
||||
ctx: &crate::TraverseCtx<'a>,
|
||||
ctx: &mut crate::TraverseCtx<'a>,
|
||||
) -> Option<Statement<'a>> {
|
||||
self.get_variable_declaration(ctx).map(Statement::VariableDeclaration)
|
||||
}
|
||||
|
||||
@@ -266,6 +266,13 @@ impl<'a> PeepholeOptimizations {
|
||||
// records the mutation for the fixed-point loop driver.
|
||||
ctx.drop_variable_declarator(&decl);
|
||||
} else {
|
||||
if let Some(mut init) = Self::take_dead_catch_var_init(&mut decl, kind, ctx) {
|
||||
if Self::remove_unused_expression(&mut init, ctx) {
|
||||
ctx.drop_expression(&init);
|
||||
} else {
|
||||
Self::push_new_expression_stmt_to_result(init, result, ctx);
|
||||
}
|
||||
}
|
||||
if let Some(Statement::VariableDeclaration(prev_var_decl)) = result.last_mut()
|
||||
&& kind == prev_var_decl.kind
|
||||
{
|
||||
|
||||
@@ -148,6 +148,28 @@ impl<'a> PeepholeOptimizations {
|
||||
}
|
||||
}
|
||||
|
||||
/// `var e = init` inside `catch (e)` stays to declare the enclosing binding, but `init`
|
||||
/// only writes the catch parameter. When that parameter is never read, take `init` out
|
||||
/// so the caller can drop it or keep just its side effects.
|
||||
pub fn take_dead_catch_var_init(
|
||||
decl: &mut VariableDeclarator<'a>,
|
||||
kind: VariableDeclarationKind,
|
||||
ctx: &TraverseCtx<'a>,
|
||||
) -> Option<Expression<'a>> {
|
||||
if !kind.is_var() || decl.init.is_none() || !Self::can_remove_unused_declarators(ctx) {
|
||||
return None;
|
||||
}
|
||||
let BindingPattern::BindingIdentifier(ident) = &decl.id else { return None };
|
||||
let symbol_id = ident.symbol_id.get()?;
|
||||
ctx.scoping().catch_var_binding(symbol_id)?;
|
||||
if !Self::symbol_is_unused_by_count(symbol_id, ctx)
|
||||
|| Self::class_initializer_name_needs_to_be_kept(decl, ctx)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
decl.init.take()
|
||||
}
|
||||
|
||||
/// Filter unused declarators out of a `KeepVar`-synthesized `var`
|
||||
/// statement. Both callers pass `KeepVar` output: a TRANSIENT statement
|
||||
/// that is not (yet) part of the live tree, whose declarators never
|
||||
|
||||
@@ -8,13 +8,9 @@ use crate::{
|
||||
|
||||
#[test]
|
||||
fn catch_var_preserves_enclosing_binding() {
|
||||
for source in [
|
||||
"function f() { try { throw 0 } catch (y) { var y = 1 } return y } use(f);",
|
||||
"function f(y) { try { throw 0 } catch (y) { var y = 1 } return y } use(f);",
|
||||
test_same_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { var [y] = value } return y } use(f);",
|
||||
] {
|
||||
test_same_smallest(source);
|
||||
}
|
||||
);
|
||||
test_smallest(
|
||||
"function f() { while (false) { try {} catch (y) { var y = 1 } } return y } use(f);",
|
||||
"function f() { var y; return y } use(f);",
|
||||
@@ -23,6 +19,34 @@ fn catch_var_preserves_enclosing_binding() {
|
||||
"function f() { return y; try {} catch (y) { var y = 1 } } use(f);",
|
||||
"function f() { return y; var y } use(f);",
|
||||
);
|
||||
// The initializer only writes the unread catch parameter.
|
||||
test_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { var y = 1 } return y } use(f);",
|
||||
"function f() { try { throw 0 } catch (y) { var y } return y } use(f);",
|
||||
);
|
||||
test_smallest(
|
||||
"function f(y) { try { throw 0 } catch (y) { var y = 1 } return y } use(f);",
|
||||
"function f(y) { try { throw 0 } catch (y) { var y } return y } use(f);",
|
||||
);
|
||||
test_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { var y = g() } return y } use(f);",
|
||||
"function f() { try { throw 0 } catch (y) { g(); var y } return y } use(f);",
|
||||
);
|
||||
// A `var` regenerated from dead code inside a surviving catch still redeclares the
|
||||
// catch parameter, so it must not be inlined away from the enclosing binding.
|
||||
test_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { while (0) { var y } y = 2; use(y) } return y } use(f);",
|
||||
"function f() { try { throw 0 } catch (y) { var y = 2; use(y) } return y } use(f);",
|
||||
);
|
||||
// When the inner catch is removed, the regenerated `var` redeclares the outer catch.
|
||||
test_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { while (0) { try {} catch (y) { var y } } y = 2; use(y) } return y } use(f);",
|
||||
"function f() { try { throw 0 } catch (y) { var y = 2; use(y) } return y } use(f);",
|
||||
);
|
||||
test_smallest(
|
||||
"function f() { try { throw 0 } catch (y) { while (0) { try {} catch (y) { var y } } } return y } use(f);",
|
||||
"function f() { try { throw 0 } catch (y) { var y } return y } use(f);",
|
||||
);
|
||||
}
|
||||
|
||||
// Leak regression: dropping an unused declarator must walk the whole
|
||||
|
||||
@@ -76,15 +76,17 @@ impl<'a> Binder<'a> for VariableDeclarator<'a> {
|
||||
// local bindings here to find catch parameters without repeating diagnostics.
|
||||
// Type-only bindings (type aliases, interfaces) live in a separate
|
||||
// declaration space, so a hoisted `var` must not merge into them.
|
||||
if builder.scoping.scope_has_binding(scope_id, name)
|
||||
if let Some(symbol_id) = builder.scoping.get_binding(scope_id, name)
|
||||
&& let Some(symbol_id) =
|
||||
builder.check_redeclaration(scope_id, span, name, excludes)
|
||||
builder.check_redeclared_symbol(symbol_id, span, name, excludes)
|
||||
&& builder.scoping.symbol_flags(symbol_id).can_be_referenced_by_value()
|
||||
&& builder.scoping.symbol_flags(symbol_id).is_value()
|
||||
{
|
||||
// A simple catch parameter can be redeclared by `var`, but the `var`
|
||||
// also creates a separate binding in the enclosing var scope. Its
|
||||
// initializer assigns to the nearest catch binding.
|
||||
// initializer assigns to the nearest catch binding. Destructured catch
|
||||
// parameters have already reported a redeclaration error above; they
|
||||
// are linked the same way so recovery keeps the two bindings apart.
|
||||
if builder.scoping.symbol_flags(symbol_id).is_catch_variable() {
|
||||
catch_symbol_id.get_or_insert(symbol_id);
|
||||
continue;
|
||||
|
||||
@@ -584,7 +584,17 @@ impl<'a> SemanticBuilder<'a> {
|
||||
.scoping
|
||||
.get_binding(scope_id, name)
|
||||
.or_else(|| self.hoisting_variables.get(&(scope_id, name)).copied())?;
|
||||
self.check_redeclared_symbol(symbol_id, span, name, excludes)
|
||||
}
|
||||
|
||||
/// Like [`Self::check_redeclaration`], for a symbol that has already been looked up.
|
||||
pub(crate) fn check_redeclared_symbol(
|
||||
&self,
|
||||
symbol_id: SymbolId,
|
||||
span: Span,
|
||||
name: Ident<'a>,
|
||||
excludes: SymbolFlags,
|
||||
) -> Option<SymbolId> {
|
||||
let flags = self.scoping.symbol_flags(symbol_id);
|
||||
|
||||
// A named function expression binds its own name in the function's scope, so a same-named
|
||||
|
||||
@@ -428,6 +428,11 @@ impl Scoping {
|
||||
self.catch_var_bindings.get(&symbol_id).copied()
|
||||
}
|
||||
|
||||
/// Record that a `var` in this catch parameter's body also declares `var_binding`.
|
||||
pub fn add_catch_var_binding(&mut self, catch_symbol_id: SymbolId, var_binding: SymbolId) {
|
||||
self.catch_var_bindings.insert(catch_symbol_id, var_binding);
|
||||
}
|
||||
|
||||
/// Iterate `(catch parameter, enclosing var binding)` pairs in unspecified order.
|
||||
pub fn catch_var_bindings(&self) -> impl Iterator<Item = (SymbolId, SymbolId)> + '_ {
|
||||
self.catch_var_bindings.iter().map(|(&catch, &outer)| (catch, outer))
|
||||
|
||||
Reference in new issue
Block a user