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.

Sign up for GitHub

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

refactor(minifier): remove the buggy minimize_exit_points implementation #8349

Merged
merged 1 commit into from
Jan 9, 2025
Merged

refactor(minifier): remove the buggy minimize_exit_points implementation #8349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
98 changes: 3 additions & 95 deletions crates/oxc_minifier/src/ast_passes/minimize_exit_points.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use oxc_allocator::Vec;
use oxc_ast::ast::*;
use oxc_semantic::ScopeFlags;
use oxc_span::{GetSpan, SPAN};
use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx};
use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse};

use crate::CompressorPass;

Expand All @@ -21,68 +18,12 @@ impl<'a> CompressorPass<'a> for MinimizeExitPoints {
}
}

impl<'a> Traverse<'a> for MinimizeExitPoints {
fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, _ctx: &mut TraverseCtx<'a>) {
self.remove_last_return(&mut body.statements);
}

fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
self.fold_if_return(stmts, ctx);
}
}
impl Traverse<'_> for MinimizeExitPoints {}

impl<'a> MinimizeExitPoints {
impl MinimizeExitPoints {
pub fn new() -> Self {
Self { changed: false }
}

// `function foo() { return }` -> `function foo() {}`
fn remove_last_return(&mut self, stmts: &mut Vec<'a, Statement<'a>>) {
if let Some(last) = stmts.last() {
if matches!(last, Statement::ReturnStatement(ret) if ret.argument.is_none()) {
stmts.pop();
self.changed = true;
}
}
}

// `if(x)return;foo` -> `if(!x)foo;`
fn fold_if_return(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) {
if stmts.len() <= 1 {
return;
}
let Some(index) = stmts.iter().position(|stmt| {
if let Statement::IfStatement(if_stmt) = stmt {
if if_stmt.alternate.is_none()
&& matches!(
if_stmt.consequent.get_one_child(),
Some(Statement::ReturnStatement(s)) if s.argument.is_none()
)
{
return true;
}
}
false
}) else {
return;
};
let Some(stmts_rest) = stmts.get_mut(index + 1..) else { return };
let body = ctx.ast.vec_from_iter(stmts_rest.iter_mut().map(|s| ctx.ast.move_statement(s)));
let Statement::IfStatement(if_stmt) = &mut stmts[index] else { unreachable!() };
let scope_id = ctx.create_child_scope_of_current(ScopeFlags::empty());
if_stmt.test = match ctx.ast.move_expression(&mut if_stmt.test) {
Expression::UnaryExpression(unary_expr) if unary_expr.operator.is_not() => {
unary_expr.unbox().argument
}
e => ctx.ast.expression_unary(e.span(), UnaryOperator::LogicalNot, e),
};
if_stmt.alternate = None;
if_stmt.consequent = Statement::BlockStatement(
ctx.ast.alloc_block_statement_with_scope_id(SPAN, body, scope_id),
);
stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_)));
self.changed = true;
}
}

#[cfg(test)]
Expand All @@ -101,39 +42,6 @@ mod test {
fold(source_text, source_text);
}

// oxc

#[test]
fn simple() {
fold(
"function foo() { if (foo) return; bar; quaz; }",
"function foo() { if (!foo) { bar; quaz; } }",
);
fold(
"function foo() { if (!foo) return; bar; quaz; }",
"function foo() { if (foo) { bar; quaz; } }",
);
fold(
"function foo() { x; if (foo) return; bar; quaz; }",
"function foo() { x; if (!foo) { bar; quaz; } }",
);
fold(
"function foo() { x; if (!foo) return; bar; quaz; }",
"function foo() { x; if (foo) { bar; quaz; } }",
);
fold_same("function foo() { if (foo) return }");
fold_same("function foo() { if (foo) return bar; baz }");
}

#[test]
fn remove_last_return() {
fold("function () {return}", "function () {}");
fold("function () {a;b;return}", "function () {a;b;}");
fold_same("function () { if(foo) { return } }");
}

// closure

#[test]
#[ignore]
fn test_break_optimization() {
Expand Down
13 changes: 8 additions & 5 deletions crates/oxc_minifier/tests/ast_passes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,16 @@ fn cjs() {
});"#,
"
Object.keys(_index6).forEach(function(key) {
!(key === 'default' || key === '__esModule') && !Object.prototype.hasOwnProperty.call(_exportNames, key) && (key in exports && exports[key] === _index6[key] || Object.defineProperty(exports, key, {
enumerable: !0,
if (key === 'default' || key === '__esModule') return;
if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
if (key in exports && exports[key] === _index6[key]) return;
Object.defineProperty(exports, key, {
enumerable: true,
get: function() {
return _index6[key];
return _index6[key];
}
}));
});"
});
});",
);
}

Expand Down
24 changes: 12 additions & 12 deletions tasks/minsize/minsize.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
| Oxc | ESBuild | Oxc | ESBuild |
Original | minified | minified | gzip | gzip | Fixture
-------------------------------------------------------------------------------------
72.14 kB | 23.68 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js
72.14 kB | 23.71 kB | 23.70 kB | 8.62 kB | 8.54 kB | react.development.js

173.90 kB | 59.77 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js
173.90 kB | 59.80 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js

287.63 kB | 90.02 kB | 90.07 kB | 32.05 kB | 31.95 kB | jquery.js
287.63 kB | 90.14 kB | 90.07 kB | 32.07 kB | 31.95 kB | jquery.js

342.15 kB | 118.12 kB | 118.14 kB | 44.51 kB | 44.37 kB | vue.js
342.15 kB | 118.38 kB | 118.14 kB | 44.53 kB | 44.37 kB | vue.js

544.10 kB | 71.72 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js
544.10 kB | 71.75 kB | 72.48 kB | 26.16 kB | 26.20 kB | lodash.js

555.77 kB | 272.81 kB | 270.13 kB | 90.92 kB | 90.80 kB | d3.js
555.77 kB | 273.20 kB | 270.13 kB | 90.93 kB | 90.80 kB | d3.js

1.01 MB | 460.21 kB | 458.89 kB | 126.83 kB | 126.71 kB | bundle.min.js
1.01 MB | 460.62 kB | 458.89 kB | 126.89 kB | 126.71 kB | bundle.min.js

1.25 MB | 652.53 kB | 646.76 kB | 163.51 kB | 163.73 kB | three.js
1.25 MB | 653.02 kB | 646.76 kB | 163.55 kB | 163.73 kB | three.js

2.14 MB | 726 kB | 724.14 kB | 180.13 kB | 181.07 kB | victory.js
2.14 MB | 726.50 kB | 724.14 kB | 180.19 kB | 181.07 kB | victory.js

3.20 MB | 1.01 MB | 1.01 MB | 331.78 kB | 331.56 kB | echarts.js
3.20 MB | 1.01 MB | 1.01 MB | 331.98 kB | 331.56 kB | echarts.js

6.69 MB | 2.32 MB | 2.31 MB | 492.61 kB | 488.28 kB | antd.js
6.69 MB | 2.32 MB | 2.31 MB | 492.75 kB | 488.28 kB | antd.js

10.95 MB | 3.50 MB | 3.49 MB | 908.24 kB | 915.50 kB | typescript.js
10.95 MB | 3.50 MB | 3.49 MB | 909.08 kB | 915.50 kB | typescript.js

Loading