Skip to content

Commit

Permalink
analyse side effects (vercel/turborepo#6563)
Browse files Browse the repository at this point in the history 
### Description

track side effects and might opt-out to unknown value to avoid wrong
replacements.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->


Closes PACK-2033
  • Loading branch information
@sokra
sokra authored Nov 24, 2023
1 parent 2b24d20 commit e7887fe
Show file tree
Hide file tree
Showing 77 changed files with 20,701 additions and 7,266 deletions.
122 changes: 79 additions & 43 deletions crates/turbopack-ecmascript/src/analyzer/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,81 @@ use super::{ConstantNumber, ConstantValue, JsValue, LogicalOperator, ObjectPart}
pub fn early_replace_builtin(value: &mut JsValue) -> bool {
match value {
// matching calls like `callee(arg1, arg2, ...)`
JsValue::Call(_, box ref mut callee, _) => match callee {
// We don't know what the callee is, so we can early return
JsValue::Unknown(_, _) => {
value.make_unknown("unknown callee");
true
}
// We known that these callee will lead to an error at runtime, so we can skip
// processing them
JsValue::Constant(_)
| JsValue::Url(_)
| JsValue::WellKnownObject(_)
| JsValue::Array { .. }
| JsValue::Object { .. }
| JsValue::Alternatives(_, _)
| JsValue::Concat(_, _)
| JsValue::Add(_, _)
| JsValue::Not(_, _) => {
value.make_unknown("non-function callee");
true
JsValue::Call(_, box ref mut callee, args) => {
let args_have_side_effects = || args.iter().any(|arg| arg.has_side_effects());
match callee {
// We don't know what the callee is, so we can early return
&mut JsValue::Unknown {
original_value: _,
reason: _,
has_side_effects,
} => {
let has_side_effects = has_side_effects || args_have_side_effects();
value.make_unknown(has_side_effects, "unknown callee");
true
}
// We known that these callee will lead to an error at runtime, so we can skip
// processing them
JsValue::Constant(_)
| JsValue::Url(_)
| JsValue::WellKnownObject(_)
| JsValue::Array { .. }
| JsValue::Object { .. }
| JsValue::Alternatives(_, _)
| JsValue::Concat(_, _)
| JsValue::Add(_, _)
| JsValue::Not(_, _) => {
let has_side_effects = args_have_side_effects();
value.make_unknown(has_side_effects, "non-function callee");
true
}
_ => false,
}
_ => false,
},
}
// matching calls with this context like `obj.prop(arg1, arg2, ...)`
JsValue::MemberCall(_, box ref mut obj, box ref mut prop, _) => match obj {
// We don't know what the callee is, so we can early return
JsValue::Unknown(_, _) => {
value.make_unknown("unknown callee object");
true
}
// otherwise we need to look at the property
_ => match prop {
// We don't know what the property is, so we can early return
JsValue::Unknown(_, _) => {
value.make_unknown("unknown callee property");
JsValue::MemberCall(_, box ref mut obj, box ref mut prop, args) => {
let args_have_side_effects = || args.iter().any(|arg| arg.has_side_effects());
match obj {
// We don't know what the callee is, so we can early return
&mut JsValue::Unknown {
original_value: _,
reason: _,
has_side_effects,
} => {
let side_effects =
has_side_effects || prop.has_side_effects() || args_have_side_effects();
value.make_unknown(side_effects, "unknown callee object");
true
}
_ => false,
},
},
// otherwise we need to look at the property
_ => match prop {
// We don't know what the property is, so we can early return
&mut JsValue::Unknown {
original_value: _,
reason: _,
has_side_effects,
} => {
let side_effects = has_side_effects || args_have_side_effects();
value.make_unknown(side_effects, "unknown callee property");
true
}
_ => false,
},
}
}
// matching property access like `obj.prop` when we don't know what the obj is.
// We can early return here
JsValue::Member(_, box JsValue::Unknown(_, _), _) => {
value.make_unknown("unknown object");
&mut JsValue::Member(
_,
box JsValue::Unknown {
original_value: _,
reason: _,
has_side_effects,
},
box ref mut prop,
) => {
let side_effects = has_side_effects || prop.has_side_effects();
value.make_unknown(side_effects, "unknown object");
true
}
_ => false,
Expand Down Expand Up @@ -88,6 +120,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
fn items_to_alternatives(items: &mut Vec<JsValue>, prop: &mut JsValue) -> JsValue {
items.push(JsValue::unknown(
JsValue::member(Box::new(JsValue::array(Vec::new())), Box::new(take(prop))),
false,
"unknown array prototype methods or values",
));
JsValue::alternatives(take(items))
Expand All @@ -100,25 +133,26 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
if index < items.len() {
*value = items.swap_remove(index);
if mutable {
value.add_unknown_mutations();
value.add_unknown_mutations(true);
}
true
} else {
*value = JsValue::unknown(
JsValue::member(Box::new(take(obj)), Box::new(take(prop))),
false,
"invalid index",
);
true
}
} else {
value.make_unknown("non-num constant property on array");
value.make_unknown(false, "non-num constant property on array");
true
}
}
// accessing a non-numeric property on an array like `[1,2,3].length`
// We don't know what happens here
JsValue::Constant(_) => {
value.make_unknown("non-num constant property on array");
value.make_unknown(false, "non-num constant property on array");
true
}
// accessing multiple alternative properties on an array like `[1,2,3][(1 | 2 |
Expand Down Expand Up @@ -163,6 +197,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
Box::new(JsValue::object(vec![take(part)])),
prop.clone(),
),
true,
"spreaded object",
));
}
Expand All @@ -174,6 +209,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
Box::new(JsValue::object(Vec::new())),
Box::new(take(prop)),
),
true,
"unknown object prototype methods or values",
));
}
Expand Down Expand Up @@ -230,7 +266,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
);
}
if mutable {
value.add_unknown_mutations();
value.add_unknown_mutations(true);
}
return true;
}
Expand All @@ -239,7 +275,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
}
}
ObjectPart::Spread(_) => {
value.make_unknown("spread object");
value.make_unknown(true, "spread object");
return true;
}
}
Expand All @@ -255,7 +291,7 @@ pub fn replace_builtin(value: &mut JsValue) -> bool {
);
}
if mutable {
value.add_unknown_mutations();
value.add_unknown_mutations(true);
}
true
}
Expand Down
48 changes: 33 additions & 15 deletions crates/turbopack-ecmascript/src/analyzer/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ impl EvalContext {
values.push(JsValue::from(v.clone()));
}
// This is actually unreachable
None => return JsValue::unknown_empty(""),
None => return JsValue::unknown_empty(true, ""),
}
}
} else {
Expand Down Expand Up @@ -459,7 +459,7 @@ impl EvalContext {
{
self.eval_tpl(tpl, true)
} else {
JsValue::unknown_empty("tagged template literal is not supported yet")
JsValue::unknown_empty(true, "tagged template literal is not supported yet")
}
}

Expand All @@ -480,14 +480,20 @@ impl EvalContext {

Expr::Await(AwaitExpr { arg, .. }) => self.eval(arg),

Expr::New(..) => JsValue::unknown_empty("unknown new expression"),
Expr::New(..) => JsValue::unknown_empty(true, "unknown new expression"),

Expr::Seq(e) => {
if let Some(e) = e.exprs.last() {
self.eval(e)
} else {
unreachable!()
let mut seq = e.exprs.iter().map(|e| self.eval(e)).peekable();
let mut side_effects = false;
let mut last = seq.next().unwrap();
for e in seq {
side_effects |= last.has_side_effects();
last = e;
}
if side_effects {
last.make_unknown(true, "sequence with side effects");
}
last
}

Expr::Member(MemberExpr {
Expand Down Expand Up @@ -516,7 +522,10 @@ impl EvalContext {
}) => {
// We currently do not handle spreads.
if args.iter().any(|arg| arg.spread.is_some()) {
return JsValue::unknown_empty("spread in function calls is not supported");
return JsValue::unknown_empty(
true,
"spread in function calls is not supported",
);
}

let args = args.iter().map(|arg| self.eval(&arg.expr)).collect();
Expand All @@ -527,6 +536,7 @@ impl EvalContext {
MemberProp::Ident(i) => i.sym.clone().into(),
MemberProp::PrivateName(_) => {
return JsValue::unknown_empty(
false,
"private names in function calls is not supported",
);
}
Expand All @@ -547,7 +557,10 @@ impl EvalContext {
}) => {
// We currently do not handle spreads.
if args.iter().any(|arg| arg.spread.is_some()) {
return JsValue::unknown_empty("spread in function calls is not supported");
return JsValue::unknown_empty(
true,
"spread in function calls is not supported",
);
}

let args = args.iter().map(|arg| self.eval(&arg.expr)).collect();
Expand All @@ -562,7 +575,7 @@ impl EvalContext {
}) => {
// We currently do not handle spreads.
if args.iter().any(|arg| arg.spread.is_some()) {
return JsValue::unknown_empty("spread in import() is not supported");
return JsValue::unknown_empty(true, "spread in import() is not supported");
}
let args = args.iter().map(|arg| self.eval(&arg.expr)).collect();

Expand All @@ -573,7 +586,7 @@ impl EvalContext {

Expr::Array(arr) => {
if arr.elems.iter().flatten().any(|v| v.spread.is_some()) {
return JsValue::unknown_empty("spread is not supported");
return JsValue::unknown_empty(true, "spread is not supported");
}

let arr = arr
Expand Down Expand Up @@ -604,14 +617,15 @@ impl EvalContext {
self.eval(&Expr::Ident(ident.clone())),
),
_ => ObjectPart::Spread(JsValue::unknown_empty(
true,
"unsupported object part",
)),
})
.collect(),
)
}

_ => JsValue::unknown_empty("unsupported expression"),
_ => JsValue::unknown_empty(true, "unsupported expression"),
}
}
}
Expand Down Expand Up @@ -1006,7 +1020,7 @@ impl VisitAstPath for Analyzer<'_> {
let right = self.eval_context.eval(&n.right);
Some(JsValue::add(vec![left, right]))
}
_ => Some(JsValue::unknown_empty("unsupported assign operation")),
_ => Some(JsValue::unknown_empty(true, "unsupported assign operation")),
};
if let Some(value) = value {
self.add_value(key.to_id(), value);
Expand Down Expand Up @@ -1043,7 +1057,7 @@ impl VisitAstPath for Analyzer<'_> {
if let Some(key) = n.arg.as_ident() {
self.add_value(
key.to_id(),
JsValue::unknown_empty("updated with update expression"),
JsValue::unknown_empty(true, "updated with update expression"),
);
}

Expand Down Expand Up @@ -1408,7 +1422,11 @@ impl VisitAstPath for Analyzer<'_> {
self.add_value(
i.to_id(),
value.unwrap_or_else(|| {
JsValue::unknown(JsValue::Variable(i.to_id()), "pattern without value")
JsValue::unknown(
JsValue::Variable(i.to_id()),
false,
"pattern without value",
)
}),
);
}
Expand Down
Loading

0 comments on commit e7887fe

Please # to comment.