From b6deff8421a24a07092c9de856adb374eec40c85 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sat, 8 Mar 2025 08:59:05 +0000 Subject: [PATCH] feat(ecmascript): support integer index access for array and string in `may_have_side_effects` (#9603) --- crates/oxc_ecmascript/src/lib.rs | 4 ++ .../src/side_effects/may_have_side_effects.rs | 54 ++++++++++++++++++- crates/oxc_ecmascript/src/to_integer_index.rs | 31 +++++++++++ .../tests/ecmascript/may_have_side_effects.rs | 26 +++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 crates/oxc_ecmascript/src/to_integer_index.rs diff --git a/crates/oxc_ecmascript/src/lib.rs b/crates/oxc_ecmascript/src/lib.rs index bf8fd5badc5bd..34befde129997 100644 --- a/crates/oxc_ecmascript/src/lib.rs +++ b/crates/oxc_ecmascript/src/lib.rs @@ -24,6 +24,9 @@ mod to_numeric; mod to_primitive; mod to_string; +// other +mod to_integer_index; + pub mod constant_evaluation; pub mod is_global_reference; pub mod side_effects; @@ -44,6 +47,7 @@ pub use self::{ to_big_int::ToBigInt, to_boolean::ToBoolean, to_int_32::ToInt32, + to_integer_index::ToIntegerIndex, to_number::ToNumber, to_primitive::ToPrimitive, to_string::ToJsString, diff --git a/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs b/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs index 0636fa95878a3..6766ce2dc2814 100644 --- a/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs +++ b/crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs @@ -1,8 +1,8 @@ use oxc_ast::ast::*; use crate::{ - constant_evaluation::DetermineValueType, is_global_reference::IsGlobalReference, - to_numeric::ToNumeric, to_primitive::ToPrimitive, + ToBigInt, ToIntegerIndex, constant_evaluation::DetermineValueType, + is_global_reference::IsGlobalReference, to_numeric::ToNumeric, to_primitive::ToPrimitive, }; /// Returns true if subtree changes application state. @@ -448,6 +448,27 @@ impl MayHaveSideEffects for ComputedMemberExpression<'_> { is_global_reference, ) } + Expression::NumericLiteral(n) => !n.value.to_integer_index().is_some_and(|n| { + !integer_index_property_access_may_have_side_effects( + &self.object, + n, + is_global_reference, + ) + }), + Expression::BigIntLiteral(b) => { + if b.is_negative() { + return true; + } + !b.to_big_int(is_global_reference) + .and_then(ToIntegerIndex::to_integer_index) + .is_some_and(|b| { + !integer_index_property_access_may_have_side_effects( + &self.object, + b, + is_global_reference, + ) + }) + } _ => true, } } @@ -467,6 +488,35 @@ fn property_access_may_have_side_effects( } } +fn integer_index_property_access_may_have_side_effects( + object: &Expression, + property: u32, + is_global_reference: &impl IsGlobalReference, +) -> bool { + if object.may_have_side_effects(is_global_reference) { + return true; + } + match object { + Expression::StringLiteral(s) => property as usize >= s.value.encode_utf16().count(), + Expression::ArrayExpression(arr) => property as usize >= get_array_minimum_length(arr), + _ => true, + } +} + +fn get_array_minimum_length(arr: &ArrayExpression) -> usize { + arr.elements + .iter() + .map(|e| match e { + ArrayExpressionElement::SpreadElement(spread) => match &spread.argument { + Expression::ArrayExpression(arr) => get_array_minimum_length(arr), + Expression::StringLiteral(str) => str.value.chars().count(), + _ => 0, + }, + _ => 1, + }) + .sum() +} + impl MayHaveSideEffects for CallExpression<'_> { fn may_have_side_effects(&self, is_global_reference: &impl IsGlobalReference) -> bool { if self.pure { diff --git a/crates/oxc_ecmascript/src/to_integer_index.rs b/crates/oxc_ecmascript/src/to_integer_index.rs new file mode 100644 index 0000000000000..842e4d6a05269 --- /dev/null +++ b/crates/oxc_ecmascript/src/to_integer_index.rs @@ -0,0 +1,31 @@ +use num_bigint::BigInt; +use num_traits::{ToPrimitive, Zero}; + +/// Convert a value to u32 if the value can be an integer index and can be represented with u32. +/// +/// Note that the max value of u32 is `2^32 - 1`, which is smaller than the max value of +/// safe integer `2^53 - 1`. +/// +/// If the value is a non-negative safe integer, it can be an integer index. +/// +pub trait ToIntegerIndex { + fn to_integer_index(self) -> Option; +} + +impl ToIntegerIndex for f64 { + fn to_integer_index(self) -> Option { + if self.fract() != 0.0 || self < 0.0 { + return None; + } + self.to_u32() + } +} + +impl ToIntegerIndex for BigInt { + fn to_integer_index(self) -> Option { + if self < BigInt::zero() { + return None; + } + self.to_u32() + } +} diff --git a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs index 1ece0d6ef1c4a..a395afc262641 100644 --- a/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs +++ b/crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs @@ -641,6 +641,32 @@ fn test_property_access() { test("[][`length`]", false); test("a[0]", true); + test("''[-1]", true); // String.prototype[-1] may be overridden + test("''[0.3]", true); // String.prototype[0.3] may be overridden + test("''[0]", true); // String.prototype[0] may be overridden + test("'a'[0]", false); + test("'a'[0n]", false); + test("'a'[1]", true); // String.prototype[1] may be overridden + test("'あ'[0]", false); + test("'あ'[1]", true); // the length of "あ" is 1 + test("'😀'[0]", false); + test("'😀'[1]", false); + test("'😀'[2]", true); // the length of "😀" is 2 + + test("[][-1]", true); // Array.prototype[-1] may be overridden + test("[][0.3]", true); // Array.prototype[0.3] may be overridden + test("[][0]", true); // Array.prototype[0] may be overridden + test("[1][0]", false); + test("[1][0n]", false); + test("[1][1]", true); // Array.prototype[1] may be overridden + test("[,][0]", false); + test("[...[], 1][0]", false); + test("[...[1]][0]", false); + test("[...'a'][0]", false); + test("[...'a'][1]", true); + test("[...'😀'][0]", false); + test("[...'😀'][1]", true); + test("[...a, 1][0]", true); // "...a" may have a sideeffect } #[test]