Skip to content

Commit

Permalink
fix(es/minifier): Preserve flags while dropping elements of SeqExpr (
Browse files Browse the repository at this point in the history
…#8907)

Co-authored-by: 강동윤 (Donny) <kdy1997.dev@gmail.com>
  • Loading branch information
devongovett and kdy1 committed Jul 31, 2024
1 parent 81d7d9d commit 24e8798
Show file tree
Hide file tree
Showing 18 changed files with 126 additions and 88 deletions.
5 changes: 5 additions & 0 deletions .changeset/gorgeous-ties-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
swc_ecma_minifier: patch
---

dead_branch_remover removes PURE annotations on sequence expressions
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ resolver = "2"
copyless = "0.1.5"
crc = "2.1.0"
criterion = "0.5.1"
crossbeam-queue = "0.3.11"
dashmap = "5.5.3"
dialoguer = "0.10.2"
difference = "2"
Expand Down
1 change: 1 addition & 0 deletions crates/swc_ecma_minifier/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tracing = { workspace = true }


swc_allocator = { version = "0.1.7", path = "../swc_allocator", default-features = false }
swc_atoms = { version = "0.6.5", path = "../swc_atoms" }
swc_common = { version = "0.36.0", path = "../swc_common" }
Expand Down
1 change: 0 additions & 1 deletion crates/swc_ecma_minifier/src/compress/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,6 @@ impl Compressor<'_> {

let mut visitor = pure_optimizer(
self.options,
None,
self.marks,
PureOptimizerConfig {
enable_join_vars: self.pass > 1,
Expand Down
2 changes: 2 additions & 0 deletions crates/swc_ecma_minifier/src/compress/optimize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1817,7 +1817,9 @@ impl VisitMut for Optimizer<'_> {

match e {
Expr::Seq(seq) if seq.exprs.len() == 1 => {
let span = seq.span;
*e = *seq.exprs[0].take();
e.set_span(span);
}

Expr::Assign(AssignExpr {
Expand Down
72 changes: 49 additions & 23 deletions crates/swc_ecma_minifier/src/compress/pure/misc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,11 @@ impl Pure<'_> {
}
}

fn make_ignored_expr(&mut self, exprs: impl Iterator<Item = Box<Expr>>) -> Option<Expr> {
fn make_ignored_expr(
&mut self,
span: Span,
exprs: impl Iterator<Item = Box<Expr>>,
) -> Option<Expr> {
let mut exprs = exprs
.filter_map(|mut e| {
self.ignore_return_value(
Expand All @@ -894,7 +898,9 @@ impl Pure<'_> {
return None;
}
if exprs.len() == 1 {
return Some(*exprs.remove(0));
let mut new = *exprs.remove(0);
new.set_span(span);
return Some(new);
}

Some(
Expand Down Expand Up @@ -970,32 +976,41 @@ impl Pure<'_> {
}
}

Expr::Call(CallExpr { ctxt, args, .. }) if ctxt.has_mark(self.marks.pure) => {
Expr::Call(CallExpr {
span, ctxt, args, ..
}) if ctxt.has_mark(self.marks.pure) => {
report_change!("ignore_return_value: Dropping a pure call");
self.changed = true;

let new = self.make_ignored_expr(args.take().into_iter().map(|arg| arg.expr));
let new =
self.make_ignored_expr(*span, args.take().into_iter().map(|arg| arg.expr));

*e = new.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}

Expr::TaggedTpl(TaggedTpl { ctxt, tpl, .. }) if ctxt.has_mark(self.marks.pure) => {
Expr::TaggedTpl(TaggedTpl {
span, ctxt, tpl, ..
}) if ctxt.has_mark(self.marks.pure) => {
report_change!("ignore_return_value: Dropping a pure call");
self.changed = true;

let new = self.make_ignored_expr(tpl.exprs.take().into_iter());
let new = self.make_ignored_expr(*span, tpl.exprs.take().into_iter());

*e = new.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}

Expr::New(NewExpr { ctxt, args, .. }) if ctxt.has_mark(self.marks.pure) => {
Expr::New(NewExpr {
span, ctxt, args, ..
}) if ctxt.has_mark(self.marks.pure) => {
report_change!("ignore_return_value: Dropping a pure call");
self.changed = true;

let new =
self.make_ignored_expr(args.take().into_iter().flatten().map(|arg| arg.expr));
let new = self.make_ignored_expr(
*span,
args.take().into_iter().flatten().map(|arg| arg.expr),
);

*e = new.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
Expand All @@ -1006,6 +1021,7 @@ impl Pure<'_> {

match e {
Expr::Call(CallExpr {
span,
callee: Callee::Expr(callee),
args,
..
Expand All @@ -1014,20 +1030,23 @@ impl Pure<'_> {
self.changed = true;
report_change!("Dropping pure call as callee is pure");
*e = self
.make_ignored_expr(args.take().into_iter().map(|arg| arg.expr))
.make_ignored_expr(*span, args.take().into_iter().map(|arg| arg.expr))
.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}
}

Expr::TaggedTpl(TaggedTpl {
tag: callee, tpl, ..
span,
tag: callee,
tpl,
..
}) => {
if callee.is_pure_callee(&self.expr_ctx) {
self.changed = true;
report_change!("Dropping pure tag tpl as callee is pure");
*e = self
.make_ignored_expr(tpl.exprs.take().into_iter())
.make_ignored_expr(*span, tpl.exprs.take().into_iter())
.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}
Expand Down Expand Up @@ -1357,24 +1376,29 @@ impl Pure<'_> {

if self.options.side_effects && self.options.pristine_globals {
match e {
Expr::New(NewExpr { callee, args, .. })
if callee.is_one_of_global_ref_to(
&self.expr_ctx,
&[
"Map", "Set", "Array", "Object", "Boolean", "Number", "String",
],
) =>
Expr::New(NewExpr {
span, callee, args, ..
}) if callee.is_one_of_global_ref_to(
&self.expr_ctx,
&[
"Map", "Set", "Array", "Object", "Boolean", "Number", "String",
],
) =>
{
report_change!("Dropping a pure new expression");

self.changed = true;
*e = self
.make_ignored_expr(args.iter_mut().flatten().map(|arg| arg.expr.take()))
.make_ignored_expr(
*span,
args.iter_mut().flatten().map(|arg| arg.expr.take()),
)
.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}

Expr::Call(CallExpr {
span,
callee: Callee::Expr(callee),
args,
..
Expand All @@ -1387,7 +1411,7 @@ impl Pure<'_> {

self.changed = true;
*e = self
.make_ignored_expr(args.iter_mut().map(|arg| arg.expr.take()))
.make_ignored_expr(*span, args.iter_mut().map(|arg| arg.expr.take()))
.unwrap_or(Invalid { span: DUMMY_SP }.into());
return;
}
Expand Down Expand Up @@ -1427,7 +1451,7 @@ impl Pure<'_> {
}

*e = self
.make_ignored_expr(exprs.into_iter())
.make_ignored_expr(obj.span, exprs.into_iter())
.unwrap_or(Invalid { span: DUMMY_SP }.into());
report_change!("Ignored an object literal");
self.changed = true;
Expand Down Expand Up @@ -1499,7 +1523,7 @@ impl Pure<'_> {
}

*e = self
.make_ignored_expr(exprs.into_iter())
.make_ignored_expr(arr.span, exprs.into_iter())
.unwrap_or(Invalid { span: DUMMY_SP }.into());
report_change!("Ignored an array literal");
self.changed = true;
Expand All @@ -1514,6 +1538,7 @@ impl Pure<'_> {
//
// foo(),basr(),foo;
Expr::Member(MemberExpr {
span,
obj,
prop: MemberProp::Computed(prop),
..
Expand All @@ -1526,6 +1551,7 @@ impl Pure<'_> {
_ => {
*e = self
.make_ignored_expr(
*span,
vec![obj.take(), prop.expr.take()].into_iter(),
)
.unwrap_or(Invalid { span: DUMMY_SP }.into());
Expand Down
9 changes: 1 addition & 8 deletions crates/swc_ecma_minifier/src/compress/pure/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@ use self::{ctx::Ctx, misc::DropOpts};
use super::util::is_pure_undefined_or_null;
#[cfg(feature = "debug")]
use crate::debug::dump;
use crate::{
debug::AssertValid, maybe_par, option::CompressOptions, program_data::ProgramData,
util::ModuleItemExt,
};
use crate::{debug::AssertValid, maybe_par, option::CompressOptions, util::ModuleItemExt};

mod arrows;
mod bools;
Expand Down Expand Up @@ -52,7 +49,6 @@ pub(crate) struct PureOptimizerConfig {
#[allow(clippy::needless_lifetimes)]
pub(crate) fn pure_optimizer<'a>(
options: &'a CompressOptions,
data: Option<&'a ProgramData>,
marks: Marks,
config: PureOptimizerConfig,
) -> impl 'a + VisitMut + Repeated {
Expand All @@ -64,7 +60,6 @@ pub(crate) fn pure_optimizer<'a>(
unresolved_ctxt: SyntaxContext::empty().apply_mark(marks.unresolved_mark),
is_unresolved_ref_safe: false,
},
data,
ctx: Default::default(),
changed: Default::default(),
}
Expand All @@ -76,8 +71,6 @@ struct Pure<'a> {
marks: Marks,
expr_ctx: ExprCtx,

#[allow(unused)]
data: Option<&'a ProgramData>,
ctx: Ctx,
changed: bool,
}
Expand Down
1 change: 0 additions & 1 deletion crates/swc_ecma_minifier/src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ impl Evaluator {
{
e.visit_mut_with(&mut pure_optimizer(
&Default::default(),
None,
self.marks,
PureOptimizerConfig {
enable_join_vars: false,
Expand Down
1 change: 0 additions & 1 deletion crates/swc_ecma_minifier/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ pub fn optimize(

let mut v = pure_optimizer(
c,
None,
marks,
PureOptimizerConfig {
force_str_for_tpl: Minification.force_str_for_tpl(),
Expand Down
3 changes: 1 addition & 2 deletions crates/swc_ecma_minifier/tests/benches-full/terser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4437,8 +4437,7 @@
let printed_comments = new Set();
var to_utf8 = options.ascii_only ? function(str, identifier = !1, regexp = !1) {
return !(options.ecma >= 2015) || options.safari10 || regexp || (str = str.replace(/[\ud800-\udbff][\udc00-\udfff]/g, function(ch) {
return "\\u{" + // https://en.wikipedia.org/wiki/Universal_Character_Set_characters#Surrogates
(is_surrogate_pair_head(ch.charCodeAt(0)) ? 0x10000 + (ch.charCodeAt(0) - 0xd800 << 10) + ch.charCodeAt(1) - 0xdc00 : ch.charCodeAt(0)).toString(16) + "}";
return "\\u{" + (is_surrogate_pair_head(ch.charCodeAt(0)) ? 0x10000 + (ch.charCodeAt(0) - 0xd800 << 10) + ch.charCodeAt(1) - 0xdc00 : ch.charCodeAt(0)).toString(16) + "}";
})), str.replace(/[\u0000-\u001f\u007f-\uffff]/g, function(ch) {
var code = ch.charCodeAt(0).toString(16);
if (code.length <= 2 && !identifier) {
Expand Down
3 changes: 1 addition & 2 deletions crates/swc_ecma_minifier/tests/benches-full/victory.js
Original file line number Diff line number Diff line change
Expand Up @@ -11730,8 +11730,7 @@ object-assign
// Equivalent of `typeof` but with special handling for array and regexp.
function getPropType(propValue) {
var propType = typeof propValue;
return Array.isArray(propValue) ? 'array' : propValue instanceof RegExp ? 'object' : // Native Symbol.
'symbol' === propType || propValue && ('Symbol' === propValue['@@toStringTag'] || 'function' == typeof Symbol && propValue instanceof Symbol) ? 'symbol' : propType;
return Array.isArray(propValue) ? 'array' : propValue instanceof RegExp ? 'object' : 'symbol' === propType || propValue && ('Symbol' === propValue['@@toStringTag'] || 'function' == typeof Symbol && propValue instanceof Symbol) ? 'symbol' : propType;
}
// This handles more types than `getPropType`. Only used for error messages.
// See `createPrimitiveTypeChecker`.
Expand Down
Loading

0 comments on commit 24e8798

Please sign in to comment.