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

Skip unreachable code when early return can be statically analysed #8890

Merged
merged 15 commits into from
Aug 1, 2024
410 changes: 362 additions & 48 deletions crates/turbopack-ecmascript/src/analyzer/graph.rs

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions crates/turbopack-ecmascript/src/analyzer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3866,6 +3866,11 @@ mod tests {
queue
.extend(then.effects.into_iter().rev().map(|e| (i, e)));
}
ConditionalKind::Else { r#else } => {
queue.extend(
r#else.effects.into_iter().rev().map(|e| (i, e)),
);
}
ConditionalKind::IfElse { then, r#else }
| ConditionalKind::Ternary { then, r#else } => {
queue.extend(
Expand All @@ -3874,6 +3879,18 @@ mod tests {
queue
.extend(then.effects.into_iter().rev().map(|e| (i, e)));
}
ConditionalKind::IfElseMultiple { then, r#else } => {
for then in then {
queue.extend(
then.effects.into_iter().rev().map(|e| (i, e)),
);
}
for r#else in r#else {
queue.extend(
r#else.effects.into_iter().rev().map(|e| (i, e)),
);
}
}
ConditionalKind::And { expr }
| ConditionalKind::Or { expr }
| ConditionalKind::NullishCoalescing { expr } => {
Expand Down Expand Up @@ -3904,6 +3921,12 @@ mod tests {
JsValue::member_call(Box::new(obj), Box::new(prop), new_args),
));
}
Effect::Unreachable { .. } => {
resolved.push((
format!("{parent} -> {i} unreachable"),
JsValue::unknown_empty(true, "unreachable"),
));
}
_ => {}
}
let time = start.elapsed();
Expand Down
2 changes: 2 additions & 0 deletions crates/turbopack-ecmascript/src/path_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ impl VisitMutAstPath for ApplyVisitors<'_, '_> {
method!(visit_mut_call_expr, CallExpr);
method!(visit_mut_lit, Lit);
method!(visit_mut_str, Str);
method!(visit_mut_block_stmt, BlockStmt);
method!(visit_mut_switch_case, SwitchCase);
}

#[cfg(test)]
Expand Down
50 changes: 49 additions & 1 deletion crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ use crate::{
type_issue::SpecifiedModuleTypeIssue,
},
tree_shake::{find_turbopack_part_id_in_asserts, part_of_module, split},
utils::AstPathRange,
EcmascriptInputTransforms, EcmascriptModuleAsset, SpecifiedModuleType, TreeShakingMode,
};

Expand Down Expand Up @@ -861,6 +862,11 @@ pub(crate) async fn analyse_ecmascript_module_internal(
};

match effect {
Effect::Unreachable { start_ast_path } => {
analysis.add_code_gen(Unreachable::new(
AstPathRange::StartAfter(start_ast_path.to_vec()).cell(),
));
}
Effect::Conditional {
condition,
kind,
Expand All @@ -872,7 +878,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(

macro_rules! inactive {
($block:ident) => {
analysis.add_code_gen(Unreachable::new(Vc::cell($block.ast_path.to_vec())));
analysis.add_code_gen(Unreachable::new($block.range.clone().cell()));
};
}
macro_rules! condition {
Expand Down Expand Up @@ -904,6 +910,19 @@ pub(crate) async fn analyse_ecmascript_module_internal(
active!(then);
}
},
ConditionalKind::Else { r#else } => match condition.is_truthy() {
Some(true) => {
condition!(ConstantConditionValue::Truthy);
inactive!(r#else);
}
Some(false) => {
condition!(ConstantConditionValue::Falsy);
active!(r#else);
}
None => {
active!(r#else);
}
},
ConditionalKind::IfElse { then, r#else }
| ConditionalKind::Ternary { then, r#else } => match condition.is_truthy() {
Some(true) => {
Expand All @@ -921,6 +940,35 @@ pub(crate) async fn analyse_ecmascript_module_internal(
active!(r#else);
}
},
ConditionalKind::IfElseMultiple { then, r#else } => match condition.is_truthy()
{
Some(true) => {
condition!(ConstantConditionValue::Truthy);
for then in then {
active!(then);
}
for r#else in r#else {
inactive!(r#else);
}
}
Some(false) => {
condition!(ConstantConditionValue::Falsy);
for then in then {
inactive!(then);
}
for r#else in r#else {
active!(r#else);
}
}
None => {
for then in then {
active!(then);
}
for r#else in r#else {
active!(r#else);
}
}
},
ConditionalKind::And { expr } => match condition.is_truthy() {
Some(true) => {
condition!(ConstantConditionValue::Truthy);
Expand Down
231 changes: 213 additions & 18 deletions crates/turbopack-ecmascript/src/references/unreachable.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,40 @@
use std::mem::take;

use anyhow::Result;
use swc_core::quote;
use swc_core::{
common::{util::take::Take, Spanned},
ecma::{
ast::{
ArrayPat, ArrowExpr, AssignPat, AssignPatProp, BindingIdent, BlockStmt, ClassDecl,
Decl, FnDecl, FnExpr, Ident, KeyValuePatProp, ObjectPat, ObjectPatProp, Pat, RestPat,
Stmt, VarDecl, VarDeclKind, VarDeclarator,
},
visit::{
fields::{BlockStmtField, SwitchCaseField},
AstParentKind, VisitMut,
},
},
quote,
};
use turbo_tasks::Vc;
use turbopack_core::chunk::ChunkingContext;

use super::AstPath;
use crate::{
code_gen::{CodeGenerateable, CodeGeneration},
create_visitor,
utils::AstPathRange,
};

#[turbo_tasks::value]
pub struct Unreachable {
path: Vc<AstPath>,
range: Vc<AstPathRange>,
}

#[turbo_tasks::value_impl]
impl Unreachable {
#[turbo_tasks::function]
pub fn new(path: Vc<AstPath>) -> Vc<Self> {
Self::cell(Unreachable { path })
pub fn new(range: Vc<AstPathRange>) -> Vc<Self> {
Self::cell(Unreachable { range })
}
}

Expand All @@ -29,20 +45,199 @@ impl CodeGenerateable for Unreachable {
&self,
_context: Vc<Box<dyn ChunkingContext>>,
) -> Result<Vc<CodeGeneration>> {
let path = self.path.await?;
let visitors = [
// Unreachable might be used on Stmt or Expr
create_visitor!(exact path, visit_mut_expr(expr: &mut Expr) {
*expr = quote!("(\"TURBOPACK unreachable\", undefined)" as Expr);
}),
create_visitor!(exact path, visit_mut_stmt(stmt: &mut Stmt) {
// TODO(WEB-553) walk ast to find all `var` declarations and keep them
// since they hoist out of the scope
*stmt = quote!("{\"TURBOPACK unreachable\";}" as Stmt);
}),
]
.into();
let range = self.range.await?;
let visitors = match &*range {
AstPathRange::Exact(path) => {
[
// Unreachable might be used on Stmt or Expr
create_visitor!(exact path, visit_mut_expr(expr: &mut Expr) {
*expr = quote!("(\"TURBOPACK unreachable\", undefined)" as Expr);
}),
create_visitor!(exact path, visit_mut_stmt(stmt: &mut Stmt) {
// TODO(WEB-553) walk ast to find all `var` declarations and keep them
// since they hoist out of the scope
let mut replacement = Vec::new();
replacement.push(quote!("\"TURBOPACK unreachable\";" as Stmt));
ExtractDeclarations {
stmts: &mut replacement,
}.visit_mut_stmt(stmt);
*stmt = Stmt::Block(BlockStmt {
span: stmt.span(),
stmts: replacement,
});
}),
]
.into()
}
AstPathRange::StartAfter(path) => {
let mut parent = &path[..];
while !parent.is_empty()
&& !matches!(parent.last().unwrap(), AstParentKind::Stmt(_))
{
parent = &parent[0..parent.len() - 1];
}
if !parent.is_empty() {
parent = &parent[0..parent.len() - 1];
fn replace(stmts: &mut Vec<Stmt>, start_index: usize) {
if stmts.len() > start_index + 1 {
let unreachable = stmts
.splice(
start_index + 1..,
[quote!("\"TURBOPACK unreachable\";" as Stmt)].into_iter(),
)
.collect::<Vec<_>>();
for mut stmt in unreachable {
ExtractDeclarations { stmts }.visit_mut_stmt(&mut stmt);
sokra marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
let (parent, [last]) = parent.split_at(parent.len() - 1) else {
unreachable!();
};
if let &AstParentKind::BlockStmt(BlockStmtField::Stmts(start_index)) = last {
[
create_visitor!(exact parent, visit_mut_block_stmt(block: &mut BlockStmt) {
replace(&mut block.stmts, start_index);
}),
]
.into()
} else if let &AstParentKind::SwitchCase(SwitchCaseField::Cons(start_index)) =
last
{
[
create_visitor!(exact parent, visit_mut_switch_case(case: &mut SwitchCase) {
replace(&mut case.cons, start_index);
}),
]
.into()
} else {
Vec::new()
}
} else {
Vec::new()
}
}
};

Ok(CodeGeneration { visitors }.cell())
}
}

struct ExtractDeclarations<'a> {
stmts: &'a mut Vec<Stmt>,
}

impl<'a> VisitMut for ExtractDeclarations<'a> {
fn visit_mut_var_decl(&mut self, decl: &mut VarDecl) {
let VarDecl {
span,
kind,
declare,
decls,
} = decl;
let mut idents = Vec::new();
for decl in take(decls) {
collect_idents(&decl.name, &mut idents);
}
let decls = idents
.into_iter()
.map(|ident| VarDeclarator {
span: ident.span,
name: Pat::Ident(BindingIdent {
id: ident,
type_ann: None,
}),
init: if matches!(kind, VarDeclKind::Const) {
Some(quote!("undefined" as Box<Expr>))
} else {
None
},
definite: false,
})
.collect();
self.stmts.push(Stmt::Decl(Decl::Var(Box::new(VarDecl {
span: *span,
kind: *kind,
declare: *declare,
decls,
}))));
}

fn visit_mut_fn_decl(&mut self, decl: &mut FnDecl) {
let FnDecl {
declare,
ident,
function,
} = decl;
self.stmts.push(Stmt::Decl(Decl::Fn(FnDecl {
declare: *declare,
ident: ident.take(),
function: function.take(),
})));
}

fn visit_mut_fn_expr(&mut self, _: &mut FnExpr) {
// Do not walk into function expressions
sokra marked this conversation as resolved.
Show resolved Hide resolved
}

fn visit_mut_arrow_expr(&mut self, _: &mut ArrowExpr) {
// Do not walk into arrow expressions
}

fn visit_mut_class_decl(&mut self, decl: &mut ClassDecl) {
let ClassDecl { declare, ident, .. } = decl;
self.stmts.push(Stmt::Decl(Decl::Var(Box::new(VarDecl {
span: ident.span,
declare: *declare,
decls: vec![VarDeclarator {
span: ident.span,
name: Pat::Ident(BindingIdent {
type_ann: None,
id: ident.clone(),
}),
init: None,
definite: false,
}],
kind: VarDeclKind::Let,
}))));
}
}

fn collect_idents(pat: &Pat, idents: &mut Vec<Ident>) {
match pat {
Pat::Ident(ident) => {
idents.push(ident.id.clone());
}
Pat::Array(ArrayPat { elems, .. }) => {
for elem in elems.iter() {
if let Some(elem) = elem.as_ref() {
collect_idents(elem, idents);
}
}
}
Pat::Rest(RestPat { arg, .. }) => {
collect_idents(arg, idents);
}
Pat::Object(ObjectPat { props, .. }) => {
for prop in props.iter() {
match prop {
ObjectPatProp::KeyValue(KeyValuePatProp { value, .. }) => {
collect_idents(value, idents);
}
ObjectPatProp::Assign(AssignPatProp { key, .. }) => {
idents.push(key.id.clone());
}
ObjectPatProp::Rest(RestPat { arg, .. }) => {
collect_idents(arg, idents);
}
}
}
}
Pat::Assign(AssignPat { left, .. }) => {
collect_idents(left, idents);
}
Pat::Invalid(_) | Pat::Expr(_) => {
// ignore
}
}
}
Loading
Loading