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 drop inlined parameter as duplicate #6293

Merged
merged 15 commits into from Nov 2, 2022
2 changes: 1 addition & 1 deletion crates/swc_ecma_codegen/benches/bench.rs
Expand Up @@ -114,7 +114,7 @@ fn bench_emitter(b: &mut Bencher, s: &str) {
let _ = emitter.emit_module(&module);
}
black_box(buf);
let srcmap = cm.build_source_map(&mut src_map_buf);
let srcmap = cm.build_source_map(&src_map_buf);
kdy1 marked this conversation as resolved.
Show resolved Hide resolved
black_box(srcmap);
});
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/swc_ecma_codegen/benches/with_parse.rs
Expand Up @@ -113,7 +113,7 @@ fn bench_emitter(b: &mut Bencher, s: &str) {
let _ = emitter.emit_module(&module);
}
black_box(buf);
let srcmap = cm.build_source_map(&mut src_map_buf);
let srcmap = cm.build_source_map(&src_map_buf);
black_box(srcmap);
});
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/swc_ecma_codegen/examples/sourcemap.rs
Expand Up @@ -45,7 +45,7 @@ fn parse_and_gen(entry: &Path) {
emitter.emit_module(&m).unwrap();
}

let srcmap = cm.build_source_map(&mut srcmap);
let srcmap = cm.build_source_map(&srcmap);

fs::write("output.js", &code).unwrap();

Expand Down
2 changes: 1 addition & 1 deletion crates/swc_ecma_codegen/tests/sourcemap.rs
Expand Up @@ -352,7 +352,7 @@ fn identity(entry: PathBuf) {
}

let actual_code = String::from_utf8(wr).unwrap();
let actual_map = cm.build_source_map_with_config(&mut src_map, None, SourceMapConfigImpl);
let actual_map = cm.build_source_map_with_config(&src_map, None, SourceMapConfigImpl);

let visualizer_url_for_actual = visualizer_url(&actual_code, &actual_map);

Expand Down
53 changes: 10 additions & 43 deletions crates/swc_ecma_minifier/src/compress/optimize/mod.rs
@@ -1,4 +1,4 @@
use std::{iter::once, mem::take};
use std::iter::once;

use rustc_hash::{FxHashMap, FxHashSet};
use swc_atoms::{js_word, JsWord};
Expand All @@ -19,7 +19,7 @@ use Value::Known;

use self::{
unused::PropertyAccessOpts,
util::{extract_class_side_effect, Finalizer, NormalMultiReplacer},
util::{extract_class_side_effect, Finalizer, NormalMultiReplacer, SynthesizedStmts},
};
use super::util::{drop_invalid_stmts, is_fine_for_if_cons};
#[cfg(feature = "debug")]
Expand Down Expand Up @@ -1383,8 +1383,8 @@ where
let mut old_prepend_stmts = self.prepend_stmts.take();
let old_append_stmts = self.append_stmts.take();
n.visit_mut_with(self);
old_prepend_stmts.append(&mut *self.prepend_stmts);
old_prepend_stmts.append(&mut *self.append_stmts);
old_prepend_stmts.append(&mut self.prepend_stmts);
old_prepend_stmts.append(&mut self.append_stmts);

self.prepend_stmts = old_prepend_stmts;
self.append_stmts = old_append_stmts;
Expand Down Expand Up @@ -2423,11 +2423,15 @@ where
// We use var decl with no declarator to indicate we dropped an decl.
Stmt::Decl(Decl::Var(v)) if v.decls.is_empty() => {
*s = Stmt::Empty(EmptyStmt { span: DUMMY_SP });
self.prepend_stmts = old_prepend;
self.append_stmts = old_append;
return;
}
Stmt::Expr(es) => {
if es.expr.is_invalid() {
*s = Stmt::Empty(EmptyStmt { span: DUMMY_SP });
self.prepend_stmts = old_prepend;
self.append_stmts = old_append;
return;
}
}
Expand Down Expand Up @@ -2459,6 +2463,8 @@ where
Stmt::Decl(Decl::Var(v)) if v.decls.is_empty() => {
s.take();
if self.prepend_stmts.is_empty() && self.append_stmts.is_empty() {
self.prepend_stmts = old_prepend;
self.append_stmts = old_append;
return;
}
}
Expand Down Expand Up @@ -3062,42 +3068,3 @@ fn is_left_access_to_arguments(l: &PatOrExpr) -> bool {
},
}
}

#[derive(Debug, Default, PartialEq, Eq)]
struct SynthesizedStmts(Vec<Stmt>);

impl SynthesizedStmts {
fn take_stmts(&mut self) -> Vec<Stmt> {
take(&mut self.0)
}
}

impl std::ops::Deref for SynthesizedStmts {
type Target = Vec<Stmt>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl std::ops::DerefMut for SynthesizedStmts {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl Take for SynthesizedStmts {
fn dummy() -> Self {
Self(Take::dummy())
}
}

impl Drop for SynthesizedStmts {
fn drop(&mut self) {
if !self.0.is_empty() {
if !std::thread::panicking() {
panic!("We should not drop synthesized stmts");
}
}
}
}
Expand Up @@ -492,7 +492,7 @@ where
prepend_stmts(stmts, self.prepend_stmts.drain(..).map(T::from_stmt));
}

if self.append_stmts.is_empty() {
if !self.append_stmts.is_empty() {
stmts.extend(self.append_stmts.drain(..).map(T::from_stmt));
}

Expand Down
9 changes: 7 additions & 2 deletions crates/swc_ecma_minifier/src/compress/optimize/unused.rs
Expand Up @@ -759,9 +759,14 @@ where
for d in var.decls.iter_mut() {
if d.init.is_none() {
if let Pat::Ident(name) = &d.name {
if let Some(usage) = self.data.vars.get(&name.to_id()) {
if usage.is_fn_local && usage.declared_as_fn_param {
if let Some(usage) = self.data.vars.get_mut(&name.to_id()) {
if usage.is_fn_local
&& usage.declared_as_fn_param
&& usage.declared_count >= 2
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix

{
d.name.take();
usage.declared_count -= 1;

report_change!(
"Removing a variable statement because it's a function parameter"
);
Expand Down
58 changes: 57 additions & 1 deletion crates/swc_ecma_minifier/src/compress/optimize/util.rs
@@ -1,4 +1,7 @@
use std::ops::{Deref, DerefMut};
use std::{
mem::take,
ops::{Deref, DerefMut},
};

use rustc_hash::{FxHashMap, FxHashSet};
use swc_atoms::js_word;
Expand Down Expand Up @@ -532,3 +535,56 @@ impl VisitMut for ExprReplacer {
}
}
}

#[derive(Debug, Default, PartialEq, Eq)]
pub(super) struct SynthesizedStmts(Vec<Stmt>);

impl SynthesizedStmts {
pub fn take_stmts(&mut self) -> Vec<Stmt> {
take(&mut self.0)
}
}

impl std::ops::Deref for SynthesizedStmts {
type Target = Vec<Stmt>;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl SynthesizedStmts {
pub fn push(&mut self, stmt: Stmt) {
self.0.push(stmt);
}

pub fn extend(&mut self, stmts: impl IntoIterator<Item = Stmt>) {
self.0.extend(stmts);
}

pub fn append(&mut self, other: &mut SynthesizedStmts) {
self.0.append(&mut other.0);
}
}

impl std::ops::DerefMut for SynthesizedStmts {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl Take for SynthesizedStmts {
fn dummy() -> Self {
Self(Take::dummy())
}
}

impl Drop for SynthesizedStmts {
fn drop(&mut self) {
if !self.0.is_empty() {
if !std::thread::panicking() {
panic!("We should not drop synthesized stmts");
}
}
}
}
14 changes: 14 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/next/tinymce/1/input.js
@@ -0,0 +1,14 @@
Yp.revoke, (a => Yp.config((b => ({
aria: {
mode: "checked"
},
...ge(b, ((e, t) => "exclusive" !== t)),
onToggled: (c, d) => {
p(b.onToggled) && b.onToggled(c, d), ((e, f) => {
Or(e, th, {
item: e,
state: f
})
})(c, d)
}
}))(a)))
12 changes: 12 additions & 0 deletions crates/swc_ecma_minifier/tests/fixture/next/tinymce/1/output.js
@@ -0,0 +1,12 @@
Yp.revoke, (a)=>Yp.config({
aria: {
mode: "checked"
},
...ge(a, (e, t)=>"exclusive" !== t),
onToggled (c, d) {
p(a.onToggled) && a.onToggled(c, d), Or(c, th, {
item: c,
state: d
});
}
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous output:

"use strict";
Yp.revoke, (o)=>Yp.config({
        aria: {
            mode: "checked"
        },
        ...ge(o, (o, g)=>"exclusive" !== g),
        onToggled (g, t) {
            p(o.onToggled) && o.onToggled(g, t), e = g, f = t, Or(e, th, {
                item: e,
                state: f
            });
        }
    });