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): Don't inline conditionally initialized vars #6751

Merged
merged 17 commits into from
Jan 11, 2023
2 changes: 1 addition & 1 deletion crates/swc_ecma_minifier/scripts/exec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ set -eu
export RUST_LOG=trace
# export SWC_CHECK=1

cargo test --features concurrent --features debug --test exec --test terser_exec
cargo test --features concurrent --features debug --test exec --test terser_exec $@
9 changes: 6 additions & 3 deletions crates/swc_ecma_minifier/src/compress/optimize/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ where
.vars
.get(&id.to_id())
.filter(|a| {
!a.reassigned() && a.declared && {
!a.reassigned() && a.declared && !a.cond_init && {
// Function declarations are hoisted
//
// As we copy expressions, this can cause a problem.
Expand Down Expand Up @@ -356,8 +356,11 @@ where
}

Expr::Ident(id) if !id.eq_ignore_span(ident) => {
if let Some(v_usage) = self.data.vars.get(&id.to_id()) {
if v_usage.reassigned() || !v_usage.declared {
if let Some(init_usage) = self.data.vars.get(&id.to_id()) {
if init_usage.reassigned()
|| !init_usage.declared
|| init_usage.cond_init
{
return;
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/swc_ecma_minifier/src/program_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ impl Storage for ProgramData {
e.get_mut().declared_as_fn_param |= var_info.declared_as_fn_param;
e.get_mut().declared_as_fn_decl |= var_info.declared_as_fn_decl;
e.get_mut().declared_as_fn_expr |= var_info.declared_as_fn_expr;
e.get_mut().declared_as_catch_param |= var_info.declared_as_catch_param;

// If a var is registered at a parent scope, it means that it's delcared before
// usages.
Expand All @@ -278,8 +279,6 @@ impl Storage for ProgramData {
e.get_mut().mutation_by_call_count += var_info.mutation_by_call_count;
e.get_mut().usage_count += var_info.usage_count;

e.get_mut().declared_as_catch_param |= var_info.declared_as_catch_param;

e.get_mut().infects.extend(var_info.infects);

e.get_mut().no_side_effect_for_member_access =
Expand Down
7 changes: 4 additions & 3 deletions crates/swc_ecma_minifier/tests/benches-full/d3.js
Original file line number Diff line number Diff line change
Expand Up @@ -5687,12 +5687,13 @@
return cache = cacheStream = null, projection;
}
return projection.stream = function(stream) {
return cache && cacheStream === stream ? cache : cache = transformRadians(transformer({
var rotate1;
return cache && cacheStream === stream ? cache : cache = transformRadians((rotate1 = rotate, transformer({
point: function(x, y) {
var r = rotate(x, y);
var r = rotate1(x, y);
return this.stream.point(r[0], r[1]);
}
})(preclip(projectResample(postclip(cacheStream = stream)))));
}))(preclip(projectResample(postclip(cacheStream = stream)))));
}, projection.preclip = function(_) {
return arguments.length ? (preclip = _, theta = void 0, reset()) : preclip;
}, projection.postclip = function(_) {
Expand Down
14 changes: 7 additions & 7 deletions crates/swc_ecma_minifier/tests/benches-full/echarts.js
Original file line number Diff line number Diff line change
Expand Up @@ -35218,22 +35218,22 @@
}, DataZoomFeature.prototype.dispose = function(ecModel, api) {
this._brushController && this._brushController.dispose();
}, DataZoomFeature.prototype._onBrush = function(eventParam) {
var storedSnapshots, areas = eventParam.areas;
var ecModel, newSnapshot, storedSnapshots, areas = eventParam.areas;
if (eventParam.isEnd && areas.length) {
var snapshot = {}, ecModel = this.ecModel;
this._brushController.updateCovers([]), new BrushTargetManager(makeAxisFinder(this.model), ecModel, {
var snapshot = {}, ecModel1 = this.ecModel;
this._brushController.updateCovers([]), new BrushTargetManager(makeAxisFinder(this.model), ecModel1, {
include: [
'grid'
]
}).matchOutputRanges(areas, ecModel, function(area, coordRange, coordSys) {
}).matchOutputRanges(areas, ecModel1, function(area, coordRange, coordSys) {
if ('cartesian2d' === coordSys.type) {
var brushType = area.brushType;
'rect' === brushType ? (setBatch('x', coordSys, coordRange[0]), setBatch('y', coordSys, coordRange[1])) : setBatch({
lineX: 'x',
lineY: 'y'
}[brushType], coordSys, coordRange);
}
}), storedSnapshots = getStoreSnapshots(ecModel), each(snapshot, function(batchItem, dataZoomId) {
}), ecModel = ecModel1, newSnapshot = snapshot, storedSnapshots = getStoreSnapshots(ecModel), each(newSnapshot, function(batchItem, dataZoomId) {
for(var i = storedSnapshots.length - 1; i >= 0 && !storedSnapshots[i][dataZoomId]; i--);
if (i < 0) {
var dataZoomModel = ecModel.queryComponents({
Expand All @@ -35250,10 +35250,10 @@
};
}
}
}), storedSnapshots.push(snapshot), this._dispatchZoomAction(snapshot);
}), storedSnapshots.push(newSnapshot), this._dispatchZoomAction(snapshot);
}
function setBatch(dimName, coordSys, minMax) {
var found, axis = coordSys.getAxis(dimName), axisModel = axis.model, dataZoomModel = (ecModel.eachComponent({
var found, axis = coordSys.getAxis(dimName), axisModel = axis.model, dataZoomModel = (ecModel1.eachComponent({
mainType: 'dataZoom',
subType: 'select'
}, function(dzModel) {
Expand Down
67 changes: 67 additions & 0 deletions crates/swc_ecma_minifier/tests/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10429,3 +10429,70 @@ fn issue_6728() {
"###,
)
}

#[test]
fn issue_6750_1() {
run_default_exec_test(
r###"
let current_component;

function set_current_component(component) {
current_component = component;
}

function f(component) {
const parent = current_component
set_current_component(component)
parent.m()
}

const obj = {
m() {
console.log("call m()")
}
}

try {
f(obj)
} catch (e) {
console.log('PASS')
}
"###,
)
}

#[test]
fn issue_6750_2() {
run_exec_test(
r###"
let current_component;

function set_current_component(component) {
current_component = component;
}

function f(component) {
const parent = current_component
set_current_component(component)
parent.m()
}

const obj = {
m() {
console.log("call m()")
}
}

try {
f(obj)
} catch (e) {
console.log('PASS')
}
"###,
r###"{
"defaults": true,
"sequences": false
}"###,
false,
)
}
23 changes: 23 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6751/1/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
let current_component;

function set_current_component(component) {
current_component = component;
}

function f(component) {
const parent = current_component
set_current_component(component)
parent.m()
}

const obj = {
m() {
console.log("call m()")
}
}

try {
f(obj)
} catch (e) {
console.log('PASS')
}
13 changes: 13 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/issues/6751/1/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
let current_component;
try {
!function(component) {
const parent = current_component;
current_component = component, parent.m();
}({
m () {
console.log("call m()");
}
});
} catch (e) {
console.log('PASS');
}

Large diffs are not rendered by default.

Large diffs are not rendered by default.