Skip to content

Commit

Permalink
fix(es/minifier): Do not add vars if eval exists (#8888)
Browse files Browse the repository at this point in the history 
**Related issue:**

 - Closes #8886

---------

Co-authored-by: austaras <austaras@outlook.com>
  • Loading branch information
@kdy1 @Austaras
kdy1 and Austaras committed Apr 27, 2024
1 parent 9749bd2 commit be359fa
Show file tree
Hide file tree
Showing 17 changed files with 258 additions and 77 deletions.
80 changes: 80 additions & 0 deletions crates/swc/tests/fixture/issues-8xxx/8886/input/.swcrc
View file
@@ -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
View file
@@ -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
View file
@@ -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
View file
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
View file
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
View file
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
View file
@@ -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
View file
@@ -0,0 +1,5 @@
const bar = ((v) => v)(1);
const foo = ((v) => v)(2);

eval(bar);
eval(foo);
8 changes: 8 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/8886/mangle.json
View file
@@ -0,0 +1,8 @@
{
"toplevel": false,
"keep_classnames": false,
"keep_fnames": false,
"keep_private_props": false,
"ie8": false,
"safari10": false
}
2 changes: 2 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/8886/output.js
View file
@@ -0,0 +1,2 @@
const bar = 1, foo = 2;
eval(bar), eval(foo);
10 changes: 5 additions & 5 deletions crates/swc_ecma_minifier/tests/terser/compress/collapse_vars/issue_2298/output.js
View file
@@ -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");
}
})();
})();
}();

0 comments on commit be359fa

Please sign in to comment.