Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
573 changes: 561 additions & 12 deletions crates/swc_ecma_minifier/src/compress/optimize/iife.rs

Large diffs are not rendered by default.

106 changes: 100 additions & 6 deletions crates/swc_ecma_minifier/src/compress/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ use swc_common::{util::take::Take, EqIgnoreSpan, Mark};
use swc_ecma_ast::*;
use swc_ecma_usage_analyzer::alias::{collect_infects_from, AliasConfig};
use swc_ecma_utils::{
class_has_side_effect, collect_decls, contains_this_expr, find_pat_ids, ExprExt, Remapper,
class_has_side_effect, collect_decls, contains_ident_ref, contains_this_expr, find_pat_ids,
ExprExt, Remapper,
};
use swc_ecma_visit::VisitMutWith;

use super::Optimizer;
use super::{iife::has_conflicting_var_decl, Optimizer};
use crate::{
compress::{
optimize::{util::is_valid_for_lhs, BitCtx},
Expand Down Expand Up @@ -746,12 +747,105 @@ impl Optimizer<'_> {
usage,
)
{
if f.function
// Collect parameter names for checking conflicting var
// declarations
let param_names: FxHashSet<_> = f
.function
.params
.iter()
.any(|param| matches!(param.pat, Pat::Rest(..) | Pat::Assign(..)))
{
return;
.filter_map(|p| match &p.pat {
Pat::Ident(id) => Some(id.sym.clone()),
Pat::Assign(a) => {
if let Pat::Ident(id) = &*a.left {
Some(id.sym.clone())
} else {
None
}
}
_ => None,
})
.collect();

// Check if the function body contains any var declarations that
// conflict with parameter names
let has_conflicting_vars = has_conflicting_var_decl(body, &param_names);

// Check for rest parameters (can't inline) and default
// parameters with side effects or that reference other
// parameters (unsafe to inline)
for (param_idx, param) in f.function.params.iter().enumerate() {
match &param.pat {
Pat::Rest(..) => return,
Pat::Assign(assign) => {
// Skip inlining if the default value has side effects
if assign.right.may_have_side_effects(self.ctx.expr_ctx) {
return;
}

// Skip inlining if there's a conflicting var declaration
// in the function body that has the same name as this
// parameter. Due to var hoisting, such declarations
// effectively reassign the parameter, but they may have
// different SyntaxContext values.
if let Pat::Ident(param_id) = &*assign.left {
if has_conflicting_vars
&& param_names.contains(&param_id.sym)
{
Comment on lines +790 to +793
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition has_conflicting_vars && param_names.contains(&param_id.sym) has a logical issue. The function has_conflicting_var_decl returns true if ANY var declaration in the body conflicts with ANY parameter name. The subsequent check param_names.contains(&param_id.sym) will always be true for a parameter (since we're iterating over parameters), making it redundant. The code should instead check if THIS specific parameter has a conflicting var declaration. Consider refactoring has_conflicting_var_decl to return FxHashSet<Atom> containing only the parameter names that have conflicts, then check conflicting_names.contains(&param_id.sym).

Suggested change
if let Pat::Ident(param_id) = &*assign.left {
if has_conflicting_vars
&& param_names.contains(&param_id.sym)
{
if let Pat::Ident(_param_id) = &*assign.left {
if has_conflicting_vars {

Copilot uses AI. Check for mistakes.
return;
}
}

// Skip inlining if the parameter is reassigned.
// E.g., `function(a = 2) { for (var [a] of [[1]]); }` -
// `a` is reassigned through destructuring, so
// inlining the default value would be incorrect.
if let Pat::Ident(param_id) = &*assign.left {
if let Some(param_usage) =
self.data.vars.get(&param_id.to_id())
{
if param_usage
.flags
.contains(VarUsageInfoFlags::REASSIGNED)
{
return;
}
} else {
// No usage data - be conservative
return;
}
}

// Skip inlining if the default value references any
// earlier parameter. E.g., `function(a, b = a) {...}` -
// `b = a` references `a`, which makes inlining unsafe.
for earlier_param in
f.function.params.iter().take(param_idx)
{
let earlier_ident = match &earlier_param.pat {
Pat::Ident(id) => Some(&id.id),
Pat::Assign(a) => {
if let Pat::Ident(id) = &*a.left {
Some(&id.id)
} else {
// Complex pattern - be conservative
return;
}
}
_ => {
// Complex pattern - be conservative
return;
}
};

if let Some(ident) = earlier_ident {
if contains_ident_ref(&*assign.right, ident) {
return;
}
}
}
Comment on lines +821 to +845
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When checking for references to earlier parameters in inline.rs (lines 821-845), the code returns early if any earlier parameter has a complex pattern (lines 830-831, 834-836). While this is conservative and safe, it might be overly restrictive. Consider: if the default value doesn't reference any identifiers at all (e.g., it's just a literal like 42), it would still skip inlining due to a complex earlier parameter. This might prevent some valid optimizations. However, given the complexity of handling all edge cases, the current conservative approach is reasonable.

Copilot uses AI. Check for mistakes.
}
_ => {}
}
}
report_change!(
"inline: Decided to inline function `{}{:?}` as it's very simple",
Expand Down
Loading
Loading