Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(es/minifier): Do not add vars if eval exists #8888

Merged
merged 18 commits into from
Apr 27, 2024
80 changes: 80 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8886/input/.swcrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
{
"jsc": {
"parser": {
"syntax": "ecmascript",
"jsx": false
},
"target": "es2022",
"externalHelpers": true,
"transform": {
"react": {
"development": true,
"runtime": "automatic"
}
},
"minify": {
"compress": {
"arguments": false,
"arrows": true,
"booleans": true,
"booleans_as_integers": false,
"collapse_vars": true,
"comparisons": true,
"computed_props": true,
"conditionals": true,
"dead_code": true,
"directives": true,
"drop_console": false,
"drop_debugger": true,
"evaluate": true,
"expression": false,
"hoist_funs": false,
"hoist_props": true,
"hoist_vars": false,
"if_return": true,
"join_vars": true,
"keep_classnames": false,
"keep_fargs": true,
"keep_fnames": false,
"keep_infinity": false,
"loops": true,
"negate_iife": true,
"properties": true,
"reduce_funcs": false,
"reduce_vars": false,
"side_effects": true,
"switches": true,
"typeofs": true,
"unsafe": false,
"unsafe_arrows": false,
"unsafe_comps": false,
"unsafe_Function": false,
"unsafe_math": false,
"unsafe_symbols": false,
"unsafe_methods": false,
"unsafe_proto": false,
"unsafe_regexp": false,
"unsafe_undefined": false,
"unused": true,
"const_to_let": false,
"pristine_globals": true,
"passes": 99
},
"mangle": {
"toplevel": false,
"keep_classnames": false,
"keep_fnames": false,
"keep_private_props": false,
"ie8": false,
"safari10": false
},
"inlineSourcesContent": false
},
"loose": false
},
"minify": false,
"isModule": true,
"module": {
"type": "es6"
},
}
5 changes: 5 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8886/input/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const bar = ((v) => v)(1);
const foo = ((v) => v)(2);

eval(bar);
eval(foo);
2 changes: 2 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8886/output/1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const bar = 1, foo = 2;
eval(bar), eval(foo);
115 changes: 63 additions & 52 deletions crates/swc_ecma_minifier/src/compress/optimize/iife.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,14 @@ impl Optimizer<'_> {

fn clean_params(callee: &mut Expr) {
match callee {
Expr::Arrow(callee) => callee.params.retain(|p| !p.is_invalid()),
Expr::Fn(callee) => callee.function.params.retain(|p| !p.pat.is_invalid()),
Expr::Arrow(callee) => {
// Drop invalid nodes
callee.params.retain(|p| !p.is_invalid())
}
Expr::Fn(callee) => {
// Drop invalid nodes
callee.function.params.retain(|p| !p.pat.is_invalid())
}
_ => {}
}
}
Expand Down Expand Up @@ -301,9 +307,9 @@ impl Optimizer<'_> {
unreachable!("find_body and find_params should match")
}
}
}

clean_params(callee);
clean_params(callee);
}
}

/// If a parameter is not used, we can ignore return value of the
Expand Down Expand Up @@ -519,6 +525,10 @@ impl Optimizer<'_> {
}
}
BlockStmtOrExpr::Expr(body) => {
if !self.can_extract_param(&param_ids) {
return;
}

if let Expr::Lit(Lit::Num(..)) = &**body {
if self.ctx.in_obj_of_non_computed_member {
return;
Expand All @@ -528,15 +538,9 @@ impl Optimizer<'_> {
self.changed = true;
report_change!("inline: Inlining a function call (arrow)");

let vars = param_ids
.iter()
.map(|name| VarDeclarator {
span: DUMMY_SP,
name: name.clone().into(),
init: Default::default(),
definite: Default::default(),
})
.collect::<Vec<_>>();
let mut exprs = vec![Box::new(make_number(DUMMY_SP, 0.0))];

let vars = self.inline_fn_param(&param_ids, &mut call.args, &mut exprs);

if !vars.is_empty() {
self.prepend_stmts.push(
Expand All @@ -550,18 +554,6 @@ impl Optimizer<'_> {
)
}

let mut exprs = vec![Box::new(make_number(DUMMY_SP, 0.0))];
for (idx, param) in param_ids.iter().enumerate() {
if let Some(arg) = call.args.get_mut(idx) {
exprs.push(Box::new(Expr::Assign(AssignExpr {
span: DUMMY_SP,
op: op!("="),
left: param.clone().into(),
right: arg.expr.take(),
})));
}
}

if call.args.len() > f.params.len() {
for arg in &mut call.args[f.params.len()..] {
exprs.push(arg.expr.take());
Expand Down Expand Up @@ -697,6 +689,22 @@ impl Optimizer<'_> {
}
}

fn can_extract_param(&self, param_ids: &[Ident]) -> bool {
// Don't create top-level variables.
if !param_ids.is_empty() && !self.may_add_ident() {
for pid in param_ids {
if let Some(usage) = self.data.vars.get(&pid.to_id()) {
if usage.ref_count > 1 || usage.assign_count > 0 || usage.inline_prevented {
log_abort!("iife: [x] Cannot inline because of usage of `{}`", pid);
return false;
}
}
}
}

true
}

fn can_inline_fn_like(&self, param_ids: &[Ident], body: &BlockStmt) -> bool {
trace_op!("can_inline_fn_like");

Expand All @@ -712,16 +720,8 @@ impl Optimizer<'_> {
}
}

// Don't create top-level variables.
if !param_ids.is_empty() && !self.may_add_ident() {
for pid in param_ids {
if let Some(usage) = self.data.vars.get(&pid.to_id()) {
if usage.ref_count > 1 || usage.assign_count > 0 || usage.inline_prevented {
log_abort!("iife: [x] Cannot inline because of usage of `{}`", pid);
return false;
}
}
}
if !self.can_extract_param(param_ids) {
return false;
}

// Abort on eval.
Expand Down Expand Up @@ -835,27 +835,13 @@ impl Optimizer<'_> {
true
}

fn inline_fn_like(
fn inline_fn_param(
&mut self,
params: &[Ident],
body: &mut BlockStmt,
args: &mut [ExprOrSpread],
) -> Option<Expr> {
if !self.can_inline_fn_like(params, &*body) {
return None;
}

if args.iter().any(|arg| arg.spread.is_some()) {
return None;
}

if self.vars.inline_with_multi_replacer(body) {
self.changed = true;
}

exprs: &mut Vec<Box<Expr>>,
) -> Vec<VarDeclarator> {
let mut vars = Vec::new();
let mut exprs = Vec::new();
let param_len = params.len();

for (idx, param) in params.iter().enumerate() {
let arg = args.get_mut(idx).map(|arg| arg.expr.take());
Expand Down Expand Up @@ -903,6 +889,31 @@ impl Optimizer<'_> {
});
}

vars
}

fn inline_fn_like(
&mut self,
params: &[Ident],
body: &mut BlockStmt,
args: &mut [ExprOrSpread],
) -> Option<Expr> {
if !self.can_inline_fn_like(params, &*body) {
return None;
}

if args.iter().any(|arg| arg.spread.is_some()) {
return None;
}

if self.vars.inline_with_multi_replacer(body) {
self.changed = true;
}

let param_len = params.len();
let mut exprs = Vec::new();
let vars = self.inline_fn_param(params, args, &mut exprs);

if args.len() > param_len {
for arg in &mut args[param_len..] {
exprs.push(arg.expr.take());
Expand Down
8 changes: 8 additions & 0 deletions crates/swc_ecma_minifier/src/compress/optimize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,14 @@ impl Optimizer<'_> {
}

fn may_add_ident(&self) -> bool {
if self.ctx.in_top_level() && self.data.top.has_eval_call {
return false;
}

if self.data.scopes.get(&self.ctx.scope).unwrap().has_eval_call {
return false;
}

if !self.ctx.in_top_level() {
return true;
}
Expand Down
11 changes: 11 additions & 0 deletions crates/swc_ecma_minifier/tests/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11212,3 +11212,14 @@ fn issue_8864_1() {
",
)
}

#[test]
fn issue_8886() {
run_default_exec_test(
"const bar = ((v) => v)(1);
const foo = ((v) => v)(2);

console.log(eval(bar));
console.log(eval(foo));",
);
}
48 changes: 48 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/8886/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
{
"defaults": true,
"arguments": false,
"arrows": true,
"booleans": true,
"booleans_as_integers": false,
"collapse_vars": true,
"comparisons": true,
"computed_props": true,
"conditionals": true,
"dead_code": true,
"directives": true,
"drop_console": false,
"drop_debugger": true,
"evaluate": true,
"expression": false,
"hoist_funs": false,
"hoist_props": true,
"hoist_vars": false,
"if_return": true,
"join_vars": true,
"keep_classnames": false,
"keep_fargs": true,
"keep_fnames": false,
"keep_infinity": false,
"loops": true,
"negate_iife": true,
"properties": true,
"reduce_funcs": false,
"reduce_vars": false,
"side_effects": true,
"switches": true,
"typeofs": true,
"unsafe": false,
"unsafe_arrows": false,
"unsafe_comps": false,
"unsafe_Function": false,
"unsafe_math": false,
"unsafe_symbols": false,
"unsafe_methods": false,
"unsafe_proto": false,
"unsafe_regexp": false,
"unsafe_undefined": false,
"unused": true,
"const_to_let": false,
"pristine_globals": true,
"passes": 99
}
5 changes: 5 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/8886/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const bar = ((v) => v)(1);
const foo = ((v) => v)(2);

eval(bar);
eval(foo);
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"toplevel": false,
"keep_classnames": false,
"keep_fnames": false,
"keep_private_props": false,
"ie8": false,
"safari10": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const bar = 1, foo = 2;
eval(bar), eval(foo);
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
!(function () {
(function () {
!function() {
(function() {
var a = undefined;
var undefined = a++;
try {
!(function (b) {
!function(b) {
(void 0)[1] = "foo";
})();
}();
console.log("FAIL");
} catch (e) {
console.log("PASS");
}
})();
})();
}();
Loading
Loading