Skip to content

Commit 62a8c5e

Browse files
committed
chore(minifier): temporary remove try_fold_if_block_one and try_fold_if_one_child (#7536)
They are not idempotent.
1 parent d942a8d commit 62a8c5e

File tree

4 files changed

+17
-110
lines changed

4 files changed

+17
-110
lines changed

crates/oxc_minifier/examples/minifier.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ fn minify(
4747
) -> String {
4848
let ret = Parser::new(allocator, source_text, source_type).parse();
4949
let mut program = ret.program;
50-
let options = MinifierOptions { mangle, compress: CompressOptions::default() };
50+
let options = MinifierOptions { mangle, compress: CompressOptions::dead_code_elimination() };
5151
let ret = Minifier::new(options).build(allocator, &mut program);
5252
CodeGenerator::new()
5353
.with_options(CodegenOptions { minify: nospace, ..CodegenOptions::default() })

crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs

+2-95
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
use oxc_ast::ast::*;
2-
use oxc_ecmascript::ToBoolean;
3-
use oxc_span::SPAN;
42
use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx};
53

64
use crate::CompressorPass;
@@ -33,16 +31,6 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions {
3331
self.changed = true;
3432
};
3533
}
36-
37-
fn exit_statement(&mut self, node: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
38-
if let Statement::IfStatement(if_stmt) = node {
39-
self.try_fold_if_block_one(if_stmt, ctx);
40-
if let Some(new_stmt) = Self::try_fold_if_one_child(if_stmt, ctx) {
41-
*node = new_stmt;
42-
self.changed = true;
43-
}
44-
}
45-
}
4634
}
4735

4836
impl<'a> PeepholeMinimizeConditions {
@@ -64,89 +52,6 @@ impl<'a> PeepholeMinimizeConditions {
6452
}
6553
None
6654
}
67-
68-
/// Duplicate logic to DCE part.
69-
fn try_fold_if_block_one(&mut self, if_stmt: &mut IfStatement<'a>, ctx: &mut TraverseCtx<'a>) {
70-
if let Statement::BlockStatement(block) = &mut if_stmt.consequent {
71-
if block.body.len() == 1
72-
&& !matches!(&block.body[0], Statement::VariableDeclaration(decl) if !decl.kind.is_var())
73-
{
74-
self.changed = true;
75-
if_stmt.consequent = ctx.ast.move_statement(&mut block.body[0]);
76-
}
77-
}
78-
if let Some(Statement::BlockStatement(block)) = &mut if_stmt.alternate {
79-
if block.body.len() == 1
80-
&& !matches!(&block.body[0], Statement::VariableDeclaration(decl) if !decl.kind.is_var())
81-
{
82-
self.changed = true;
83-
if_stmt.alternate = Some(ctx.ast.move_statement(&mut block.body[0]));
84-
}
85-
}
86-
}
87-
88-
fn try_fold_if_one_child(
89-
if_stmt: &mut IfStatement<'a>,
90-
ctx: &mut TraverseCtx<'a>,
91-
) -> Option<Statement<'a>> {
92-
if let Statement::ExpressionStatement(expr) = &mut if_stmt.consequent {
93-
// The rest of things for known boolean are tasks for dce instead of here.
94-
if_stmt
95-
.test
96-
.to_boolean()
97-
.is_none()
98-
.then(|| {
99-
if !matches!(if_stmt.alternate, None | Some(Statement::ExpressionStatement(_)))
100-
{
101-
return None;
102-
}
103-
// Make if (x) y; => x && y;
104-
let (reverse, mut test) = match &mut if_stmt.test {
105-
Expression::UnaryExpression(unary) if unary.operator.is_not() => {
106-
let arg = ctx.ast.move_expression(&mut unary.argument);
107-
(true, arg)
108-
}
109-
_ => (false, ctx.ast.move_expression(&mut if_stmt.test)),
110-
};
111-
match &mut test {
112-
Expression::BinaryExpression(bin) if bin.operator.is_equality() => {
113-
if !bin.left.is_literal() && bin.right.is_literal() {
114-
test = ctx.ast.expression_binary(
115-
SPAN,
116-
ctx.ast.move_expression(&mut bin.right),
117-
bin.operator,
118-
ctx.ast.move_expression(&mut bin.left),
119-
);
120-
}
121-
}
122-
_ => {}
123-
}
124-
if let Some(Statement::ExpressionStatement(alt)) = &mut if_stmt.alternate {
125-
let left = ctx.ast.move_expression(&mut expr.expression);
126-
let right = ctx.ast.move_expression(&mut alt.expression);
127-
let cond = if reverse {
128-
ctx.ast.expression_conditional(SPAN, test, right, left)
129-
} else {
130-
ctx.ast.expression_conditional(SPAN, test, left, right)
131-
};
132-
Some(ctx.ast.statement_expression(SPAN, cond))
133-
} else if if_stmt.alternate.is_none() {
134-
let new_expr = ctx.ast.expression_logical(
135-
SPAN,
136-
test,
137-
if reverse { LogicalOperator::Or } else { LogicalOperator::And },
138-
ctx.ast.move_expression(&mut expr.expression),
139-
);
140-
Some(ctx.ast.statement_expression(SPAN, new_expr))
141-
} else {
142-
None
143-
}
144-
})
145-
.unwrap_or(None)
146-
} else {
147-
None
148-
}
149-
}
15055
}
15156

15257
/// <https://github.com/google/closure-compiler/blob/v20240609/test/com/google/javascript/jscomp/PeepholeMinimizeConditionsTest.java>
@@ -176,6 +81,7 @@ mod test {
17681

17782
/** Check that removing blocks with 1 child works */
17883
#[test]
84+
#[ignore]
17985
fn test_fold_one_child_blocks() {
18086
// late = false;
18187
fold("function f(){if(x)a();x=3}", "function f(){x&&a();x=3}");
@@ -402,6 +308,7 @@ mod test {
402308
}
403309

404310
#[test]
311+
#[ignore]
405312
fn test_not_cond() {
406313
fold("function f(){if(!x)foo()}", "function f(){x||foo()}");
407314
fold("function f(){if(!x)b=1}", "function f(){x||(b=1)}");

crates/oxc_minifier/tests/ast_passes/dead_code_elimination.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ fn dce_if_statement() {
4949
test("if (!false) { foo }", "foo");
5050
test("if (!true) { foo } else { bar }", "bar");
5151

52-
test("if (!false && xxx) { foo }", "xxx && foo");
52+
test("if (!false && xxx) { foo }", "if (xxx) foo");
5353
test("if (!true && yyy) { foo } else { bar }", "bar");
5454

5555
test("if (true || xxx) { foo }", "foo");
56-
test("if (false || xxx) { foo }", "xxx && foo");
56+
test("if (false || xxx) { foo }", "if (xxx) foo");
5757

5858
test("if ('production' == 'production') { foo } else { bar }", "foo");
5959
test("if ('development' == 'production') { foo } else { bar }", "bar");

tasks/minsize/minsize.snap

+12-12
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
11
Original | Minified | esbuild | Gzip | esbuild
22

3-
72.14 kB | 23.98 kB | 23.70 kB | 8.67 kB | 8.54 kB | react.development.js
3+
72.14 kB | 24.12 kB | 23.70 kB | 8.62 kB | 8.54 kB | react.development.js
44

5-
173.90 kB | 61.16 kB | 59.82 kB | 19.62 kB | 19.33 kB | moment.js
5+
173.90 kB | 61.67 kB | 59.82 kB | 19.54 kB | 19.33 kB | moment.js
66

7-
287.63 kB | 91.70 kB | 90.07 kB | 32.34 kB | 31.95 kB | jquery.js
7+
287.63 kB | 92.70 kB | 90.07 kB | 32.26 kB | 31.95 kB | jquery.js
88

9-
342.15 kB | 120.23 kB | 118.14 kB | 44.72 kB | 44.37 kB | vue.js
9+
342.15 kB | 121.90 kB | 118.14 kB | 44.59 kB | 44.37 kB | vue.js
1010

11-
544.10 kB | 73.03 kB | 72.48 kB | 26.16 kB | 26.20 kB | lodash.js
11+
544.10 kB | 73.48 kB | 72.48 kB | 26.12 kB | 26.20 kB | lodash.js
1212

13-
555.77 kB | 275.23 kB | 270.13 kB | 91.33 kB | 90.80 kB | d3.js
13+
555.77 kB | 276.48 kB | 270.13 kB | 91.15 kB | 90.80 kB | d3.js
1414

15-
1.01 MB | 464.89 kB | 458.89 kB | 127.04 kB | 126.71 kB | bundle.min.js
15+
1.01 MB | 467.59 kB | 458.89 kB | 126.73 kB | 126.71 kB | bundle.min.js
1616

17-
1.25 MB | 660.45 kB | 646.76 kB | 164.46 kB | 163.73 kB | three.js
17+
1.25 MB | 662.83 kB | 646.76 kB | 164.00 kB | 163.73 kB | three.js
1818

19-
2.14 MB | 739.98 kB | 724.14 kB | 181.63 kB | 181.07 kB | victory.js
19+
2.14 MB | 741.55 kB | 724.14 kB | 181.45 kB | 181.07 kB | victory.js
2020

21-
3.20 MB | 1.02 MB | 1.01 MB | 333.04 kB | 331.56 kB | echarts.js
21+
3.20 MB | 1.02 MB | 1.01 MB | 332.07 kB | 331.56 kB | echarts.js
2222

23-
6.69 MB | 2.39 MB | 2.31 MB | 496.74 kB | 488.28 kB | antd.js
23+
6.69 MB | 2.39 MB | 2.31 MB | 496.11 kB | 488.28 kB | antd.js
2424

25-
10.95 MB | 3.55 MB | 3.49 MB | 913.59 kB | 915.50 kB | typescript.js
25+
10.95 MB | 3.56 MB | 3.49 MB | 911.20 kB | 915.50 kB | typescript.js
2626

0 commit comments

Comments
 (0)