mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(ecmascript): skip array length evaluation if there are any spread elements (#13162)
https://github.com/oxc-project/oxc/blob/4d0a05e1e7c06598fc92b41bbf785eb6dc6e36fa/crates/oxc_ecmascript/src/constant_evaluation/mod.rs#L501-L502 If there are spread elements in an array, say `[...[1, 2, 3]]`, then `arr.elements.len()` no longer represents the value of `.length` property of the array, which will cause mis-optimization. <details> <summary>PoC of the bug</summary> `test.js`: ``` js let k = [...[1, 2, 3]].length; console.log(k); ``` ``` $ node test.js 3 $ cargo run --example minifier -- test.js Finished `dev` profile [unoptimized] target(s) in 0.08s Running `target/debug/examples/minifier` console.log(1); ``` </details> This bug was originally discovered in https://github.com/vitejs/rolldown-vite/issues/375. Maybe sometimes the `.length` can be evaluated recursively, but here we just skip the evaluation as a quick fix. What's more, I think it would be better to add some tests like `test_eval("[1, 2, 3].length", Some(Number(3.0)))` and `test_eval("[...a].length", None)`, but I cannot find a suitable place and found it's not trivial to implement one `Ctx` for `evaluate_value` in test files. Please let me know if you have any comments. Thanks. --------- Co-authored-by: GZTime <Time.GZ@outlook.com> Co-authored-by: Boshen <boshenc@gmail.com>
This commit is contained in:
2 files changed
+20
-16
No files matched your search
@@ -495,15 +495,7 @@ impl<'a> ConstantEvaluation<'a> for StaticMemberExpression<'a> {
|
||||
_target_ty: Option<ValueType>,
|
||||
) -> Option<ConstantValue<'a>> {
|
||||
match self.property.name.as_str() {
|
||||
"length" => {
|
||||
if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
|
||||
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
|
||||
} else if let Expression::ArrayExpression(arr) = &self.object {
|
||||
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
"length" => evaluate_value_length(&self.object, ctx),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -517,19 +509,30 @@ impl<'a> ConstantEvaluation<'a> for ComputedMemberExpression<'a> {
|
||||
) -> Option<ConstantValue<'a>> {
|
||||
match &self.expression {
|
||||
Expression::StringLiteral(s) if s.value == "length" => {
|
||||
if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
|
||||
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
|
||||
} else if let Expression::ArrayExpression(arr) = &self.object {
|
||||
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
evaluate_value_length(&self.object, ctx)
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn evaluate_value_length<'a>(
|
||||
object: &Expression<'a>,
|
||||
ctx: &impl ConstantEvaluationCtx<'a>,
|
||||
) -> Option<ConstantValue<'a>> {
|
||||
if let Some(ConstantValue::String(s)) = object.evaluate_value(ctx) {
|
||||
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
|
||||
} else if let Expression::ArrayExpression(arr) = object {
|
||||
if arr.elements.iter().any(|e| matches!(e, ArrayExpressionElement::SpreadElement(_))) {
|
||||
None
|
||||
} else {
|
||||
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
|
||||
}
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> ConstantEvaluation<'a> for CallExpression<'a> {
|
||||
fn evaluate_value_to(
|
||||
&self,
|
||||
|
||||
@@ -1778,6 +1778,7 @@ mod test {
|
||||
// Cannot fold
|
||||
fold("x = [foo(), 0].length", "x = [foo(),0].length");
|
||||
fold_same("x = y.length");
|
||||
fold_same("[...[1, 2, 3]].length");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in new issue
Block a user