Skip to content

Commit

Permalink
dce: Separate phase for dropping imports (#764)
Browse files Browse the repository at this point in the history
  • Loading branch information
kdy1 authored May 9, 2020
1 parent 14f5212 commit ddc5ace
Show file tree
Hide file tree
Showing 12 changed files with 481 additions and 12 deletions.
30 changes: 27 additions & 3 deletions ecmascript/transforms/src/optimization/simplify/dce/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use self::side_effect::SideEffectVisitor;
use self::side_effect::{ImportDetector, SideEffectVisitor};
use crate::pass::RepeatedJsPass;
use fxhash::FxHashSet;
use std::borrow::Cow;
Expand Down Expand Up @@ -64,6 +64,7 @@ pub fn dce<'a>(config: Config<'a>) -> impl RepeatedJsPass + 'a {
included: Default::default(),
changed: false,
marking_phase: false,
import_dropping_phase: false,
},
UsedMarkRemover { used_mark }
)
Expand Down Expand Up @@ -111,6 +112,11 @@ struct Dce<'a> {
/// If true, idents are added to [included].
marking_phase: bool,

/// If false, the pass **ignores** imports.
///
/// It means, imports are not marked (as used) nor removed.
import_dropping_phase: bool,

dropped: bool,
}

Expand All @@ -134,7 +140,7 @@ impl Repeated for Dce<'_> {
impl<T> Fold<Vec<T>> for Dce<'_>
where
T: StmtLike + FoldWith<Self> + Spanned,
T: for<'any> VisitWith<SideEffectVisitor<'any>>,
T: for<'any> VisitWith<SideEffectVisitor<'any>> + VisitWith<ImportDetector>,
{
fn fold(&mut self, mut items: Vec<T>) -> Vec<T> {
let old = self.changed;
Expand Down Expand Up @@ -168,6 +174,7 @@ where
{
let mut idx = 0;
items = items.move_flat_map(|item| {
let item = self.drop_imports(item);
let item = match item.try_into_stmt() {
Ok(stmt) => match stmt {
Stmt::Empty(..) => {
Expand All @@ -187,7 +194,12 @@ where
}

idx += 1;
Some(item)
// Drop unused imports
if self.is_marked(item.span()) {
Some(item)
} else {
None
}
});
}

Expand Down Expand Up @@ -260,6 +272,18 @@ impl Dce<'_> {

node
}

pub fn drop_imports<T>(&mut self, node: T) -> T
where
T: FoldWith<Self>,
{
let old = self.import_dropping_phase;
self.import_dropping_phase = true;
let node = node.fold_with(self);
self.import_dropping_phase = old;

node
}
}

impl Fold<Ident> for Dce<'_> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ use swc_ecma_ast::*;

impl Fold<ImportDecl> for Dce<'_> {
fn fold(&mut self, mut import: ImportDecl) -> ImportDecl {
// Do not mark import as used while ignoring imports
if !self.import_dropping_phase {
return import;
}

if self.is_marked(import.span) {
return import;
}
Expand All @@ -14,11 +19,6 @@ impl Fold<ImportDecl> for Dce<'_> {
return import;
}

// First run.
if self.included.is_empty() {
return import;
}

// Drop unused imports.
import.specifiers.retain(|s| self.should_include(&s));

Expand Down
27 changes: 25 additions & 2 deletions ecmascript/transforms/src/optimization/simplify/dce/side_effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,34 @@ use swc_common::{Visit, VisitWith};
use swc_ecma_ast::*;
use swc_ecma_utils::{ident::IdentLike, ExprExt, Id};

pub(super) struct ImportDetector {
found: bool,
}

noop_visit_type!(ImportDetector);

impl Visit<ImportDecl> for ImportDetector {
fn visit(&mut self, _: &ImportDecl) {
self.found = true;
}
}

impl Dce<'_> {
pub fn should_include<T>(&mut self, node: &T) -> bool
pub(super) fn should_include<T>(&mut self, node: &T) -> bool
where
T: for<'any> VisitWith<SideEffectVisitor<'any>>,
T: for<'any> VisitWith<SideEffectVisitor<'any>> + VisitWith<ImportDetector>,
{
// Preserve imports if we are not in import dropping phase
if !self.import_dropping_phase {
let mut v = ImportDetector { found: false };

node.visit_with(&mut v);

if v.found {
return true;
}
}

let mut v = SideEffectVisitor {
included: &mut self.included,
exports: self.config.used.as_ref().map(|v| &**v),
Expand Down
3 changes: 2 additions & 1 deletion ecmascript/transforms/src/optimization/simplify/dce/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,8 @@ impl Fold<ForStmt> for Dce<'_> {

node = node.fold_children(self);

if self.is_marked(node.init.span())
if node.test.is_none()
|| self.is_marked(node.init.span())
|| self.is_marked(node.test.span())
|| self.is_marked(node.update.span())
|| self.is_marked(node.body.span())
Expand Down
14 changes: 13 additions & 1 deletion ecmascript/transforms/src/optimization/simplify/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,10 +807,22 @@ impl SimplifyExpr {

op!("~") => {
if let Known(value) = arg.as_number() {
println!(
"Value: {} -> {} -> {} -> {}",
value,
value as u32,
!(value as u32),
!(value as u32) as i32
);
println!("{}", value as i32 as u32);
if value.fract() == 0.0 {
return Expr::Lit(Lit::Num(Number {
span,
value: !(value as u32) as i32 as f64,
value: if value < 0.0 {
!(value as i32 as u32) as i32 as f64
} else {
!(value as u32) as i32 as f64
},
}));
}
// TODO: Report error
Expand Down
47 changes: 47 additions & 0 deletions ecmascript/transforms/tests/modules_common_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4108,3 +4108,50 @@ test!(
var _mongodb = require('mongodb');
require('foo');"
);

test!(
syntax(),
|_| tr(Config {
strict: false,
strict_mode: true,
..Default::default()
}),
issue_763,
"import {
INSTAGRAM_CHECK_PATTERN,
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
]",
"'use strict';
var _consts = require('../../../../consts');
const resources = [
{
value: _consts.RESOURCE_WEBSITE,
label: 'Webové stránky'
},
{
value: _consts.RESOURCE_FACEBOOK,
label: 'Facebook'
},
{
value: _consts.RESOURCE_INSTAGRAM,
label: 'Instagram'
}
];"
);
168 changes: 168 additions & 0 deletions ecmascript/transforms/tests/optimization_simplify_dce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,171 @@ optimized_out!(
function reducer(state = initialState, action = {}) {
}"
);

to!(
issue_763_1,
"import {
INSTAGRAM_CHECK_PATTERN,
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
export const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
]",
"import {
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
export const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
]"
);

to!(
issue_763_2,
"import {
INSTAGRAM_CHECK_PATTERN,
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
]
resources.map(console.log.bind(console));",
"import {
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
];
resources.map(console.log.bind(console));"
);

noop!(
issue_763_3,
"import {
RESOURCE_FACEBOOK,
RESOURCE_INSTAGRAM,
RESOURCE_WEBSITE,
} from '../../../../consts'
const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
];
resources.map(v => v)"
);

noop!(
issue_763_4,
"import { RESOURCE_FACEBOOK, RESOURCE_INSTAGRAM, RESOURCE_WEBSITE } from './consts';
const resources = [
{
value: RESOURCE_WEBSITE,
label: 'Webové stránky',
},
{
value: RESOURCE_FACEBOOK,
label: 'Facebook',
},
{
value: RESOURCE_INSTAGRAM,
label: 'Instagram',
},
];
export function foo(websites) {
const a = resources.map((resource) => (
{
value: resource.value,
}
));
const b = website.type_id === RESOURCE_INSTAGRAM ? 'text' : 'url';
return a + b;
}"
);

noop!(
issue_763_5_1,
"import { A, B } from './consts';
const resources = [A, B];
use(B)
resources.map(v => v)
"
);

noop!(
issue_763_5_2,
"import { A, B } from './consts';
const resources = {A, B};
use(B)
resources.map(v => v)
"
);
Loading

0 comments on commit ddc5ace

Please sign in to comment.