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
feat(es/minfiier): Compute more with sequential inliner #6169
Changes from 25 commits
4f47679
7aa2c8e
679d49a
33848a7
dbe3e78
4d74cc6
6a0dfc3
5b10614
2e30b68
847f6a4
7e9e17b
c624d48
428bf1a
f249851
8a66e7a
9be6d08
be1e3fb
0e64a97
77c5563
72e8d3e
e56b714
ac4ea57
f6d1a46
bd69625
42f79cb
990da73
36a87b4
e557b79
b6935f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
//// [compoundAdditionAssignmentLHSCanBeAssigned.ts] | ||
var E, a, b, x1, x2, x3, x4, x6; | ||
var E; | ||
!function(E) { | ||
E[E.a = 0] = "a", E[E.b = 1] = "b"; | ||
}(E || (E = {})), x1 += a, x1 += b, x1 += !0, x1 += 0, x1 += "", x1 += E.a, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += b, x2 += !0, x2 += 0, x2 += "", x2 += E.a, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += 0, x3 += E.a, x3 += null, x3 += void 0, x4 += a, x4 += 0, x4 += E.a, x4 += null, x4 += void 0, x6 += a, x6 += ""; | ||
}(E || (E = {})), E.a, E.a, E.a, E.a; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
//// [compoundAdditionAssignmentWithInvalidOperands.ts] | ||
var E, a, x1, x2, x3, x4, x5; | ||
var E; | ||
!function(E) { | ||
E[E.a = 0] = "a", E[E.b = 1] = "b"; | ||
}(E || (E = {})), x1 += a, x1 += !0, x1 += 0, x1 += E.a, x1 += {}, x1 += null, x1 += void 0, x2 += a, x2 += !0, x2 += 0, x2 += E.a, x2 += {}, x2 += null, x2 += void 0, x3 += a, x3 += !0, x3 += 0, x3 += E.a, x3 += {}, x3 += null, x3 += void 0, x4 += a, x4 += !0, x4 += {}, x5 += a, x5 += !0, x5 += {}; | ||
}(E || (E = {})), E.a, E.a, E.a; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
//// [compoundArithmeticAssignmentLHSCanBeAssigned.ts] | ||
var E, a, b, c, x1, x2, x3; | ||
var E; | ||
!function(E) { | ||
E[E.a = 0] = "a", E[E.b = 1] = "b", E[E.c = 2] = "c"; | ||
}(E || (E = {})), x1 *= a, x1 *= b, x1 *= c, x1 *= null, x1 *= void 0, x2 *= a, x2 *= b, x2 *= c, x2 *= null, x2 *= void 0, x3 *= a, x3 *= b, x3 *= c, x3 *= null, x3 *= void 0; | ||
}(E || (E = {})); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
//// [compoundArithmeticAssignmentWithInvalidOperands.ts] | ||
var E, a, b, x1, x2, x3, x4, x5, x6; | ||
var E; | ||
!function(E) { | ||
E[E.a = 0] = "a", E[E.b = 1] = "b"; | ||
}(E || (E = {})), x1 *= a, x1 *= b, x1 *= !0, x1 *= 0, x1 *= "", x1 *= E.a, x1 *= {}, x1 *= null, x1 *= void 0, x2 *= a, x2 *= b, x2 *= !0, x2 *= 0, x2 *= "", x2 *= E.a, x2 *= {}, x2 *= null, x2 *= void 0, x3 *= a, x3 *= b, x3 *= !0, x3 *= 0, x3 *= "", x3 *= E.a, x3 *= {}, x3 *= null, x3 *= void 0, x4 *= a, x4 *= b, x4 *= !0, x4 *= 0, x4 *= "", x4 *= E.a, x4 *= {}, x4 *= null, x4 *= void 0, x5 *= b, x5 *= !0, x5 *= "", x5 *= {}, x6 *= b, x6 *= !0, x6 *= "", x6 *= {}; | ||
}(E || (E = {})), E.a, E.a, E.a, E.a; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
//// [destructuringControlFlow.ts] | ||
(0, [ | ||
[ | ||
"foo" | ||
][1]).toUpperCase(); | ||
][1].toUpperCase(); | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
//// [symbolType12.ts] | ||
var s = Symbol.for("assign"), str = ""; | ||
s *= s, s *= 0, s /= s, s /= 0, s %= s, s %= 0, s += s, s += 0, s += "", str += s, s -= s, s -= 0, s <<= s, s <<= 0, s >>= s, s >>= 0, s >>>= s, s >>>= 0, s &= s, s &= 0, s ^= s, s ^= 0, s |= s, s |= 0, str += s || str; | ||
s *= 0, s /= s, s /= 0, s %= s, s %= 0, str += s = s + 0 + "", s -= s, s -= 0, s <<= s, s <<= 0, s >>= s, s >>= 0, s >>>= s, s >>>= 0, s &= s, s &= 0, s ^= s, s ^= 0, s |= s, s |= 0, str += s || str; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1665,6 +1665,20 @@ where | |
}; | ||
|
||
if !self.is_skippable_for_seq(Some(a), &Expr::Ident(b_left.clone())) { | ||
// Let's be safe | ||
if IdentUsageFinder::find(&b_left.to_id(), &b_assign.right) { | ||
return Ok(false); | ||
} | ||
|
||
// As we are not *skipping* lhs, we can inline here | ||
if let Some(a_id) = a.id() { | ||
if a_id == b_left.to_id() { | ||
if self.replace_seq_assignment(a, b)? { | ||
return Ok(true); | ||
} | ||
} | ||
} | ||
|
||
return Ok(false); | ||
} | ||
|
||
|
@@ -2202,7 +2216,7 @@ where | |
} | ||
|
||
macro_rules! take_a { | ||
($force_drop:expr) => { | ||
($force_drop:expr, $drop_op:expr) => { | ||
match a { | ||
Mergable::Var(a) => { | ||
if self.options.unused { | ||
|
@@ -2224,15 +2238,17 @@ where | |
.unwrap_or_else(|| undefined(DUMMY_SP)) | ||
} | ||
Mergable::Expr(a) => { | ||
if can_remove { | ||
if can_remove || $force_drop { | ||
if let Expr::Assign(e) = a { | ||
report_change!( | ||
"sequences: Dropping assignment as we are going to drop the \ | ||
variable declaration. ({})", | ||
left_id | ||
); | ||
|
||
**a = *e.right.take(); | ||
if e.op == op!("=") || $drop_op { | ||
report_change!( | ||
"sequences: Dropping assignment as we are going to drop \ | ||
the variable declaration. ({})", | ||
left_id | ||
); | ||
|
||
**a = *e.right.take(); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -2256,7 +2272,10 @@ where | |
Expr::Assign(b @ AssignExpr { op: op!("="), .. }) => { | ||
if let Some(b_left) = b.left.as_ident() { | ||
if b_left.to_id() == left_id.to_id() { | ||
let mut a_expr = take_a!(true); | ||
report_change!("sequences: Merged assignment into another assignment"); | ||
self.changed = true; | ||
|
||
let mut a_expr = take_a!(true, false); | ||
let a_expr = self.ignore_return_value(&mut a_expr); | ||
|
||
if let Some(a) = a_expr { | ||
|
@@ -2271,19 +2290,35 @@ where | |
} | ||
Expr::Assign(b) => { | ||
if let Some(b_left) = b.left.as_ident() { | ||
if b_left.to_id() == left_id.to_id() { | ||
if let Some(bin_op) = b.op.to_update() { | ||
b.op = op!("="); | ||
let a_op = match a { | ||
Mergable::Var(_) => Some(op!("=")), | ||
Mergable::Expr(Expr::Assign(AssignExpr { op: a_op, .. })) => Some(*a_op), | ||
Mergable::FnDecl(_) => Some(op!("=")), | ||
_ => None, | ||
}; | ||
|
||
let to = take_a!(true); | ||
if let Some(a_op) = a_op { | ||
if can_drop_op_for(a_op, b.op) { | ||
if b_left.to_id() == left_id.to_id() { | ||
if let Some(bin_op) = b.op.to_update() { | ||
report_change!( | ||
"sequences: Merged assignment into another (op) assignment" | ||
); | ||
self.changed = true; | ||
|
||
b.right = Box::new(Expr::Bin(BinExpr { | ||
span: DUMMY_SP, | ||
op: bin_op, | ||
left: to, | ||
right: b.right.take(), | ||
})); | ||
return Ok(true); | ||
b.op = op!("="); | ||
|
||
let to = take_a!(true, true); | ||
|
||
b.right = Box::new(Expr::Bin(BinExpr { | ||
span: DUMMY_SP, | ||
op: bin_op, | ||
left: to, | ||
right: b.right.take(), | ||
})); | ||
return Ok(true); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -2319,7 +2354,7 @@ where | |
left_id.span.ctxt | ||
); | ||
|
||
let to = take_a!(false); | ||
let to = take_a!(false, false); | ||
|
||
replace_id_with_expr(b, left_id.to_id(), to); | ||
|
||
|
@@ -2449,3 +2484,16 @@ pub(crate) fn is_trivial_lit(e: &Expr) -> bool { | |
_ => false, | ||
} | ||
} | ||
|
||
/// This assumes `a.left.to_id() == b.left.to_id()` | ||
fn can_drop_op_for(a: AssignOp, b: AssignOp) -> bool { | ||
if a == op!("=") { | ||
return true; | ||
} | ||
|
||
if a == b { | ||
return matches!(a, op!("+=") | op!("*=")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just looking for associative property? We should be able to merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This first one should be prefixed with |
||
} | ||
|
||
false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it matters in this specific case, but this has changed the
this
context of the method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, I'll add a test case for it.