Skip to content

Commit d27a04b

Browse files
YanWQ-monadGZTimeWalkerBoshen
authored
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 vitejs/rolldown-vite#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>
1 parent 2674634 commit d27a04b

File tree

2 files changed

+20
-16
lines changed

2 files changed

+20
-16
lines changed

crates/oxc_ecmascript/src/constant_evaluation/mod.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,7 @@ impl<'a> ConstantEvaluation<'a> for StaticMemberExpression<'a> {
495495
_target_ty: Option<ValueType>,
496496
) -> Option<ConstantValue<'a>> {
497497
match self.property.name.as_str() {
498-
"length" => {
499-
if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
500-
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
501-
} else if let Expression::ArrayExpression(arr) = &self.object {
502-
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
503-
} else {
504-
None
505-
}
506-
}
498+
"length" => evaluate_value_length(&self.object, ctx),
507499
_ => None,
508500
}
509501
}
@@ -517,19 +509,30 @@ impl<'a> ConstantEvaluation<'a> for ComputedMemberExpression<'a> {
517509
) -> Option<ConstantValue<'a>> {
518510
match &self.expression {
519511
Expression::StringLiteral(s) if s.value == "length" => {
520-
if let Some(ConstantValue::String(s)) = self.object.evaluate_value(ctx) {
521-
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
522-
} else if let Expression::ArrayExpression(arr) = &self.object {
523-
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
524-
} else {
525-
None
526-
}
512+
evaluate_value_length(&self.object, ctx)
527513
}
528514
_ => None,
529515
}
530516
}
531517
}
532518

519+
fn evaluate_value_length<'a>(
520+
object: &Expression<'a>,
521+
ctx: &impl ConstantEvaluationCtx<'a>,
522+
) -> Option<ConstantValue<'a>> {
523+
if let Some(ConstantValue::String(s)) = object.evaluate_value(ctx) {
524+
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
525+
} else if let Expression::ArrayExpression(arr) = object {
526+
if arr.elements.iter().any(|e| matches!(e, ArrayExpressionElement::SpreadElement(_))) {
527+
None
528+
} else {
529+
Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap()))
530+
}
531+
} else {
532+
None
533+
}
534+
}
535+
533536
impl<'a> ConstantEvaluation<'a> for CallExpression<'a> {
534537
fn evaluate_value_to(
535538
&self,

crates/oxc_minifier/src/peephole/fold_constants.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,6 +1778,7 @@ mod test {
17781778
// Cannot fold
17791779
fold("x = [foo(), 0].length", "x = [foo(),0].length");
17801780
fold_same("x = y.length");
1781+
fold_same("[...[1, 2, 3]].length");
17811782
}
17821783

17831784
#[test]

0 commit comments

Comments
 (0)