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 bundler: stack overflow and circular imports #1205

Merged
merged 18 commits into from Nov 6, 2020
2 changes: 1 addition & 1 deletion bundler/Cargo.toml
Expand Up @@ -8,7 +8,7 @@ edition = "2018"
license = "Apache-2.0/MIT"
name = "swc_bundler"
repository = "https://github.com/swc-project/swc.git"
version = "0.16.0"
version = "0.16.1"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[features]
Expand Down
79 changes: 76 additions & 3 deletions bundler/src/bundler/chunk/circular.rs
Expand Up @@ -179,16 +179,21 @@ fn dependency_index<T>(item: &ModuleItem, deps: &[T]) -> Option<usize>
where
T: Borrow<ModuleItem>,
{
let mut v = DepFinder { deps, idx: None };
let mut v = DepFinder {
deps,
idx: None,
last_usage_idx: None,
};
item.visit_with(&Invalid { span: DUMMY_SP }, &mut v);
v.idx
v.idx.or(v.last_usage_idx)
}

struct DepFinder<'a, T>
where
T: Borrow<ModuleItem>,
{
deps: &'a [T],
last_usage_idx: Option<usize>,
idx: Option<usize>,
}

Expand Down Expand Up @@ -224,7 +229,11 @@ where
}
}

_ => {}
dep => {
if DepUsageFinder::find(i, dep) {
self.last_usage_idx = Some(idx);
}
}
}
}
}
Expand Down Expand Up @@ -253,3 +262,67 @@ where
#[inline]
fn visit_block_stmt(&mut self, _: &BlockStmt, _: &dyn Node) {}
}

/// Finds usage of `ident`
struct DepUsageFinder<'a> {
ident: &'a Ident,
found: bool,
}

impl<'a> Visit for DepUsageFinder<'a> {
noop_visit_type!();

fn visit_call_expr(&mut self, e: &CallExpr, _: &dyn Node) {
if self.found {
return;
}

match &e.callee {
ExprOrSuper::Super(_) => {}
ExprOrSuper::Expr(callee) => match &**callee {
Expr::Ident(..) => {}
_ => {
callee.visit_with(e, self);
}
},
}

e.args.visit_with(e, self);
}

fn visit_ident(&mut self, i: &Ident, _: &dyn Node) {
if self.found {
return;
}

if i.span.ctxt() == self.ident.span.ctxt() && i.sym == self.ident.sym {
self.found = true;
}
}

fn visit_member_expr(&mut self, e: &MemberExpr, _: &dyn Node) {
if self.found {
return;
}

e.obj.visit_with(e as _, self);

if e.computed {
e.prop.visit_with(e as _, self);
}
}
}

impl<'a> DepUsageFinder<'a> {
pub fn find<N>(ident: &'a Ident, node: &N) -> bool
where
N: VisitWith<Self>,
{
let mut v = DepUsageFinder {
ident,
found: false,
};
node.visit_with(&Invalid { span: DUMMY_SP } as _, &mut v);
v.found
}
}
13 changes: 0 additions & 13 deletions bundler/src/bundler/chunk/export.rs
Expand Up @@ -184,19 +184,6 @@ where

if let Some(id) = id_of_export_namespace_from {
dep = self.wrap_esm_as_a_var(plan, dep, &imported, merged, id)?;

let module_plan;
let module_plan = match plan.normal.get(&info.id) {
Some(v) => v,
None => {
module_plan = Default::default();
&module_plan
}
};

dep = self
.merge_imports(plan, &module_plan, dep, &info, merged, false)
.context("failed to merge imports")?;
} else {
dep = self.remark_exports(dep, src.ctxt, None, false);
}
Expand Down
26 changes: 13 additions & 13 deletions bundler/src/bundler/chunk/merge.rs
Expand Up @@ -299,6 +299,19 @@ where
if dep_info.is_es6 {
// print_hygiene("entry: before injection", &self.cm, &entry);

if !is_direct {
prepend_stmts(&mut entry.body, take(&mut dep.body).into_iter());

log::debug!(
"Merged {} into {} as a transitive es module",
dep_info.fm.name,
info.fm.name,
);

// print_hygiene("ES6", &self.cm, &entry);
continue;
}

// Replace import statement / require with module body
let mut injector = Es6ModuleInjector {
imported: take(&mut dep.body),
Expand All @@ -323,19 +336,6 @@ where
continue;
}

if !is_direct {
prepend_stmts(&mut entry.body, injector.imported.into_iter());

log::debug!(
"Merged {} into {} as a transitive es module",
dep_info.fm.name,
info.fm.name,
);

// print_hygiene("ES6", &self.cm, &entry);
continue;
}

// print_hygiene("entry: failed to inject", &self.cm, &entry);

dep.body = take(&mut injector.imported);
Expand Down
2 changes: 1 addition & 1 deletion bundler/src/bundler/chunk/plan/lca.rs
Expand Up @@ -67,7 +67,7 @@ fn check_itself(b: &PlanBuilder, li: &[ModuleId], ri: &[ModuleId]) -> Option<Mod
for &r in ri {
// Root
if g.neighbors_directed(r, Incoming).count() == 0 {
return Some(l);
return Some(r);
}

if l == r {
Expand Down
42 changes: 26 additions & 16 deletions bundler/src/bundler/chunk/plan/mod.rs
Expand Up @@ -11,7 +11,6 @@ use petgraph::{
EdgeDirection::{Incoming, Outgoing},
};
use std::{
cmp::Ordering,
collections::{hash_map::Entry, HashMap, HashSet},
ops::{Deref, DerefMut},
};
Expand Down Expand Up @@ -386,9 +385,7 @@ where

// Sort transitive chunks topologically.
for (_, normal_plan) in &mut plans.normal {
normal_plan
.transitive_chunks
.sort_by(|a, b| toposort(&builder, *a, *b));
toposort(&builder, &mut normal_plan.transitive_chunks);
}

// Handle circular imports
Expand Down Expand Up @@ -523,7 +520,12 @@ where
}

// Prevent dejavu
for (src, _) in &m.imports.specifiers {
for (src, _) in m
.imports
.specifiers
.iter()
.chain(m.exports.reexports.iter())
{
if builder.all_deps.contains_key(&(src.module_id, module_id)) {
log::debug!("Circular dep: {:?} => {:?}", module_id, src.module_id);

Expand All @@ -548,16 +550,24 @@ where
}
}

/// Compare topology of `i` and `k`.
fn toposort(b: &PlanBuilder, i: ModuleId, j: ModuleId) -> Ordering {
//
let higher = least_common_ancestor(b, &[i, j]);

if higher == i {
Ordering::Greater
} else if higher == j {
Ordering::Less
} else {
Ordering::Equal
fn toposort(b: &PlanBuilder, module_ids: &mut Vec<ModuleId>) {
let len = module_ids.len();

if module_ids.len() <= 1 {
return;
}

for i in 0..len {
for j in i..len {
let mi = module_ids[i];
let mj = module_ids[j];
if mi == mj {
continue;
}

if b.direct_deps.contains_edge(mj, mi) {
module_ids.swap(i, j);
}
}
}
}
5 changes: 5 additions & 0 deletions bundler/tests/deno.rs
Expand Up @@ -73,6 +73,11 @@ fn deno_8211() {
run("https://unpkg.com/luxon@1.25.0/src/luxon.js", None);
}

#[test]
fn deno_8246() {
run("https://raw.githubusercontent.com/nats-io/nats.deno/v1.0.0-11/nats-base-client/internal_mod.ts",None);
}

fn run(url: &str, expeceted_bytes: Option<usize>) {
let dir = tempfile::tempdir().expect("failed to crate temp file");
let path = dir.path().join("main.js");
Expand Down
2 changes: 1 addition & 1 deletion spack/tests/pass/issue-1139/example-1/output/entry.js
@@ -1,8 +1,8 @@
function f2() {
console.log("f2");
}
f1();
export function f1() {
console.log("f1");
}
f1();
f2();
2 changes: 1 addition & 1 deletion spack/tests/pass/issue-1139/example-2/output/entry.js
@@ -1,8 +1,8 @@
function f2() {
console.log("f2");
}
f1();
function f1() {
console.log("f1");
}
f1();
f2();
4 changes: 2 additions & 2 deletions spack/tests/pass/transitive/import/simple-2/output/entry.js
@@ -1,9 +1,9 @@
const common1 = 1;
const common = 2;
const common2 = common;
console.log('a', common1, common2);
const common1 = 1;
const common3 = 3;
const common31 = common3;
console.log('a', common1, common2);
console.log('b', common31, common1);
var common4;
try {
Expand Down