Skip to content

Commit

Permalink
fix(es/minifier): Avoid dropping statements which has side-effects (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
hyf0 committed Nov 20, 2022
1 parent e59ccfc commit 9154bbc
Show file tree
Hide file tree
Showing 13 changed files with 157 additions and 20 deletions.
@@ -1 +1,6 @@
//// [objectSpreadSetonlyAccessor.ts]
({
...{
set foo (_v){}
}
});
1 change: 1 addition & 0 deletions crates/swc_atoms/words.txt
Expand Up @@ -1369,3 +1369,4 @@ ychannelselector
yield
zoomAndPan
zoomandpan
__proto__
26 changes: 17 additions & 9 deletions crates/swc_ecma_minifier/src/compress/optimize/mod.rs
Expand Up @@ -1918,15 +1918,23 @@ where
#[cfg(feature = "debug")]
let start = dump(&n.expr, true);

let expr = self.ignore_return_value(&mut n.expr);
n.expr = expr.map(Box::new).unwrap_or_else(|| {
report_change!("visit_mut_expr_stmt: Dropped an expression statement");

#[cfg(feature = "debug")]
dump_change_detail!("Removed {}", start);

undefined(DUMMY_SP)
});
// Fix https://github.com/swc-project/swc/issues/6422
let is_object_lit_with_spread = n
.expr
.as_object()
.map(|object_lit| object_lit.props.iter().any(|prop| prop.is_spread()))
.unwrap_or(false);

if !is_object_lit_with_spread {
let expr = self.ignore_return_value(&mut n.expr);
n.expr = expr.map(Box::new).unwrap_or_else(|| {
report_change!("visit_mut_expr_stmt: Dropped an expression statement");
#[cfg(feature = "debug")]
dump_change_detail!("Removed {}", start);

undefined(DUMMY_SP)
});
}
} else {
match &mut *n.expr {
Expr::Seq(e) => {
Expand Down
@@ -0,0 +1,5 @@
{
"collapse_vars": true,
"unused": true,
"toplevel": true
}
21 changes: 21 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/1/input.js
@@ -0,0 +1,21 @@
let getter_effect = 'FAIL';
let setter_effect = 'FAIL';
let proto = {
get foo() {
getter_effect = 'PASS';
},
set bar(value) {
setter_effect = 'PASS';
}
};
let obj1 = {
__proto__: proto
};
let obj2 = {
__proto__: proto
};
let unused = obj1.foo;
obj2.bar = 0;

assert.strictEqual(getter_effect, 'PASS');
assert.strictEqual(setter_effect, 'PASS');
18 changes: 18 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/1/output.js
@@ -0,0 +1,18 @@
let getter_effect = 'FAIL';
let setter_effect = 'FAIL';
let proto = {
get foo () {
getter_effect = 'PASS';
},
set bar (value){
setter_effect = 'PASS';
}
};
({
__proto__: proto
}).foo;
({
__proto__: proto
}).bar = 0;
assert.strictEqual(getter_effect, 'PASS');
assert.strictEqual(setter_effect, 'PASS');
@@ -0,0 +1,4 @@
{
"toplevel": true,
"unused": true
}
10 changes: 10 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/2/input.js
@@ -0,0 +1,10 @@
import assert from 'assert'
let result = 'FAIL';
const unused = {
...{
get prop() {
result = 'PASS';
}
}
};
assert.strictEqual(result, 'PASS');
10 changes: 10 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/2/output.js
@@ -0,0 +1,10 @@
import assert from 'assert';
let result = 'FAIL';
({
...{
get prop () {
result = 'PASS';
}
}
});
assert.strictEqual(result, 'PASS');
@@ -0,0 +1,4 @@
{
"toplevel": true,
"unused": true
}
18 changes: 18 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/3/input.js
@@ -0,0 +1,18 @@
import assert from 'assert'
let result = 0;
const unused = {
...{
get prop() {
result = 1;
}
},
[assert.strictEqual(result, 1)]: null,
[result = 2]: null,
[assert.strictEqual(result, 2)]: null,
...{
get prop() {
result = 3;
}
}
};
assert.strictEqual(result, 3);
18 changes: 18 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6422/3/output.js
@@ -0,0 +1,18 @@
import assert from 'assert';
let result = 0;
({
...{
get prop () {
result = 1;
}
},
[assert.strictEqual(result, 1)]: null,
[result = 2]: null,
[assert.strictEqual(result, 2)]: null,
...{
get prop () {
result = 3;
}
}
});
assert.strictEqual(result, 3);
37 changes: 26 additions & 11 deletions crates/swc_ecma_utils/src/lib.rs
Expand Up @@ -1375,18 +1375,33 @@ pub trait ExprExt {
}
}
Expr::Object(obj) => {
let is_static_accessor = |prop: &PropOrSpread| {
if let PropOrSpread::Prop(prop) = prop {
if let Prop::Getter(_) | Prop::Setter(_) = &**prop {
true
} else {
false
}
} else {
false
}
let can_have_side_effect = |prop: &PropOrSpread| match prop {
PropOrSpread::Spread(_) => true,
PropOrSpread::Prop(prop) => match prop.as_ref() {
Prop::Getter(_)
| Prop::Setter(_)
| Prop::Method(_)
| Prop::Shorthand(Ident {
sym: js_word!("__proto__"),
..
})
| Prop::KeyValue(KeyValueProp {
key:
PropName::Ident(Ident {
sym: js_word!("__proto__"),
..
})
| PropName::Str(Str {
value: js_word!("__proto__"),
..
})
| PropName::Computed(_),
..
}) => true,
_ => false,
},
};
if obj.props.iter().any(is_static_accessor) {
if obj.props.iter().any(can_have_side_effect) {
return true;
}
}
Expand Down

1 comment on commit 9154bbc

@github-actions
Copy link

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 9154bbc Previous: 15ad2c2 Ratio
es/full/bugs-1 389841 ns/iter (± 22054) 346117 ns/iter (± 22474) 1.13
es/full/minify/libraries/antd 2094374257 ns/iter (± 82448426) 1957955351 ns/iter (± 56659346) 1.07
es/full/minify/libraries/d3 502630299 ns/iter (± 27607030) 444712150 ns/iter (± 38663654) 1.13
es/full/minify/libraries/echarts 1873070557 ns/iter (± 758763072) 1615751824 ns/iter (± 115715877) 1.16
es/full/minify/libraries/jquery 125668872 ns/iter (± 7586107) 110708257 ns/iter (± 5102303) 1.14
es/full/minify/libraries/lodash 161579804 ns/iter (± 21023829) 133833754 ns/iter (± 14042922) 1.21
es/full/minify/libraries/moment 88708545 ns/iter (± 29765105) 65244200 ns/iter (± 7197779) 1.36
es/full/minify/libraries/react 26280308 ns/iter (± 4046363) 25592873 ns/iter (± 2133927) 1.03
es/full/minify/libraries/terser 374533985 ns/iter (± 18064188) 368397583 ns/iter (± 23963737) 1.02
es/full/minify/libraries/three 652642002 ns/iter (± 31323109) 635544587 ns/iter (± 23540994) 1.03
es/full/minify/libraries/typescript 4136959214 ns/iter (± 252717032) 3664030287 ns/iter (± 95819033) 1.13
es/full/minify/libraries/victory 927628403 ns/iter (± 61741347) 863451822 ns/iter (± 31194415) 1.07
es/full/minify/libraries/vue 178428798 ns/iter (± 9735215) 165269128 ns/iter (± 11804335) 1.08
es/full/codegen/es3 35629 ns/iter (± 8609) 34763 ns/iter (± 1305) 1.02
es/full/codegen/es5 38103 ns/iter (± 6502) 34587 ns/iter (± 1230) 1.10
es/full/codegen/es2015 36890 ns/iter (± 5849) 34433 ns/iter (± 1937) 1.07
es/full/codegen/es2016 34100 ns/iter (± 6093) 34382 ns/iter (± 2122) 0.99
es/full/codegen/es2017 38119 ns/iter (± 7737) 34724 ns/iter (± 1535) 1.10
es/full/codegen/es2018 37150 ns/iter (± 6840) 34589 ns/iter (± 2727) 1.07
es/full/codegen/es2019 36291 ns/iter (± 6934) 34779 ns/iter (± 956) 1.04
es/full/codegen/es2020 36868 ns/iter (± 5595) 34435 ns/iter (± 1021) 1.07
es/full/all/es3 225387040 ns/iter (± 24353389) 208946945 ns/iter (± 17927599) 1.08
es/full/all/es5 213631035 ns/iter (± 17082855) 196987775 ns/iter (± 14151792) 1.08
es/full/all/es2015 169123795 ns/iter (± 9833735) 155192235 ns/iter (± 10338814) 1.09
es/full/all/es2016 171026993 ns/iter (± 15511849) 156039736 ns/iter (± 14142708) 1.10
es/full/all/es2017 169572869 ns/iter (± 12352684) 152429291 ns/iter (± 9867532) 1.11
es/full/all/es2018 164007453 ns/iter (± 10050642) 151556223 ns/iter (± 12155126) 1.08
es/full/all/es2019 164051983 ns/iter (± 11626394) 149367814 ns/iter (± 13828712) 1.10
es/full/all/es2020 161819590 ns/iter (± 14091917) 141622508 ns/iter (± 8817046) 1.14
es/full/parser 822178 ns/iter (± 111880) 735960 ns/iter (± 41686) 1.12
es/full/base/fixer 28915 ns/iter (± 4716) 26506 ns/iter (± 1048) 1.09
es/full/base/resolver_and_hygiene 97233 ns/iter (± 11134) 95434 ns/iter (± 18794) 1.02
serialization of ast node 244 ns/iter (± 32) 220 ns/iter (± 11) 1.11
serialization of serde 228 ns/iter (± 28) 221 ns/iter (± 3) 1.03

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.