nprasad2077/oxc
1
0 Fork 0
mirror of https://github.com/oxc-project/oxc.git synced 2026-11-04 10:29:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(minifier): avoid minifying +void unknown to NaN and fix typo (#8784)

Browse Source 
fixes #8782
This bug appears when we arbitrarily eliminate potential side-effective elements in unary expression.
This commit is contained in:
7086cmd committed 2025-01-30 15:01:22 +00:00
1 parent b3a57696a1
commit ae7f670024
5 files changed
+50 -44
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/oxc_ecmascript/src/constant_evaluation/mod.rs
+21 -19
View File
@@ -32,7 +32,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
// and there are only a very few cases where we can compute a number value, but there could
// also be side effects. e.g. `void doSomething()` has value NaN, regardless of the behavior
// of `doSomething()`
if value.is_some() && self.expression_may_have_side_efffects(expr) {
if value.is_some() && self.expression_may_have_side_effects(expr) {
None
} else {
value
@@ -41,7 +41,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
fn get_side_free_string_value(&self, expr: &Expression<'a>) -> Option<Cow<'a, str>> {
let value = expr.to_js_string();
if value.is_some() && !self.expression_may_have_side_efffects(expr) {
if value.is_some() && !self.expression_may_have_side_effects(expr) {
return value;
}
None
@@ -49,7 +49,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
fn get_side_free_boolean_value(&self, expr: &Expression<'a>) -> Option<bool> {
let value = self.get_boolean_value(expr);
if value.is_some() && !self.expression_may_have_side_efffects(expr) {
if value.is_some() && !self.expression_may_have_side_effects(expr) {
return value;
}
None
@@ -57,7 +57,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
fn get_side_free_bigint_value(&self, expr: &Expression<'a>) -> Option<BigInt> {
let value = expr.to_big_int();
if value.is_some() && self.expression_may_have_side_efffects(expr) {
if value.is_some() && self.expression_may_have_side_effects(expr) {
None
} else {
value
@@ -201,8 +201,8 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
) -> Option<ConstantValue<'a>> {
match operator {
BinaryOperator::Addition => {
if self.expression_may_have_side_efffects(left)
|| self.expression_may_have_side_efffects(right)
if self.expression_may_have_side_effects(left)
|| self.expression_may_have_side_effects(right)
{
return None;
}
@@ -319,7 +319,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
None
}
BinaryOperator::Instanceof => {
if self.expression_may_have_side_efffects(left) {
if self.expression_may_have_side_effects(left) {
return None;
}
if let Expression::Identifier(right_ident) = right {
@@ -379,21 +379,22 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
Some(ConstantValue::String(Cow::Borrowed(s)))
}
UnaryOperator::Void => (expr.argument.is_literal()
|| !self.expression_may_have_side_efffects(&expr.argument))
|| !self.expression_may_have_side_effects(&expr.argument))
.then_some(ConstantValue::Undefined),
UnaryOperator::LogicalNot => self
.get_side_free_boolean_value(&expr.argument)
.map(|b| !b)
.map(ConstantValue::Boolean),
UnaryOperator::UnaryPlus => {
self.eval_to_number(&expr.argument).map(ConstantValue::Number)
self.get_side_free_number_value(&expr.argument).map(ConstantValue::Number)
}
UnaryOperator::UnaryNegation => match ValueType::from(&expr.argument) {
ValueType::BigInt => {
self.eval_to_big_int(&expr.argument).map(|v| -v).map(ConstantValue::BigInt)
}
ValueType::BigInt => self
.get_side_free_bigint_value(&expr.argument)
.map(|v| -v)
.map(ConstantValue::BigInt),
ValueType::Number => self
.eval_to_number(&expr.argument)
.get_side_free_number_value(&expr.argument)
.map(|v| if v.is_nan() { v } else { -v })
.map(ConstantValue::Number),
ValueType::Undefined => Some(ConstantValue::Number(f64::NAN)),
@@ -401,12 +402,13 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
_ => None,
},
UnaryOperator::BitwiseNot => match ValueType::from(&expr.argument) {
ValueType::BigInt => {
self.eval_to_big_int(&expr.argument).map(|v| !v).map(ConstantValue::BigInt)
}
ValueType::BigInt => self
.get_side_free_bigint_value(&expr.argument)
.map(|v| !v)
.map(ConstantValue::BigInt),
#[expect(clippy::cast_lossless)]
_ => self
.eval_to_number(&expr.argument)
.get_side_free_number_value(&expr.argument)
.map(|v| (!v.to_int_32()) as f64)
.map(ConstantValue::Number),
},
@@ -423,7 +425,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) {
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
} else {
if self.expression_may_have_side_efffects(&expr.object) {
if self.expression_may_have_side_effects(&expr.object) {
return None;
}
if let Expression::ArrayExpression(arr) = &expr.object {
@@ -446,7 +448,7 @@ pub trait ConstantEvaluation<'a>: MayHaveSideEffects {
if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) {
Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap()))
} else {
if self.expression_may_have_side_efffects(&expr.object) {
if self.expression_may_have_side_effects(&expr.object) {
return None;
}
if let Expression::ArrayExpression(arr) = &expr.object {
crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs
+15 -15
View File
@@ -6,7 +6,7 @@ use oxc_ast::ast::*;
pub trait MayHaveSideEffects {
fn is_global_reference(&self, ident: &IdentifierReference<'_>) -> bool;
fn expression_may_have_side_efffects(&self, e: &Expression<'_>) -> bool {
fn expression_may_have_side_effects(&self, e: &Expression<'_>) -> bool {
match e {
// Reference read can have a side effect.
Expression::Identifier(ident) => match ident.name.as_str() {
@@ -24,19 +24,19 @@ pub trait MayHaveSideEffects {
| Expression::ArrowFunctionExpression(_)
| Expression::FunctionExpression(_) => false,
Expression::TemplateLiteral(template) => {
template.expressions.iter().any(|e| self.expression_may_have_side_efffects(e))
template.expressions.iter().any(|e| self.expression_may_have_side_effects(e))
}
Expression::UnaryExpression(e) => self.unary_expression_may_have_side_effects(e),
Expression::ParenthesizedExpression(e) => {
self.expression_may_have_side_efffects(&e.expression)
self.expression_may_have_side_effects(&e.expression)
}
Expression::ConditionalExpression(e) => {
self.expression_may_have_side_efffects(&e.test)
|| self.expression_may_have_side_efffects(&e.consequent)
|| self.expression_may_have_side_efffects(&e.alternate)
self.expression_may_have_side_effects(&e.test)
|| self.expression_may_have_side_effects(&e.consequent)
|| self.expression_may_have_side_effects(&e.alternate)
}
Expression::SequenceExpression(e) => {
e.expressions.iter().any(|e| self.expression_may_have_side_efffects(e))
e.expressions.iter().any(|e| self.expression_may_have_side_effects(e))
}
Expression::BinaryExpression(e) => self.binary_expression_may_have_side_effects(e),
Expression::ObjectExpression(object_expr) => object_expr
@@ -57,7 +57,7 @@ pub trait MayHaveSideEffects {
operator != UnaryOperator::Delete
}
if is_simple_unary_operator(e.operator) {
return self.expression_may_have_side_efffects(&e.argument);
return self.expression_may_have_side_effects(&e.argument);
}
true
}
@@ -67,8 +67,8 @@ pub trait MayHaveSideEffects {
if matches!(e.operator, BinaryOperator::In | BinaryOperator::Instanceof) {
return true;
}
self.expression_may_have_side_efffects(&e.left)
|| self.expression_may_have_side_efffects(&e.right)
self.expression_may_have_side_effects(&e.left)
|| self.expression_may_have_side_effects(&e.right)
}
fn array_expression_element_may_have_side_effects(
@@ -77,10 +77,10 @@ pub trait MayHaveSideEffects {
) -> bool {
match e {
ArrayExpressionElement::SpreadElement(e) => {
self.expression_may_have_side_efffects(&e.argument)
self.expression_may_have_side_effects(&e.argument)
}
match_expression!(ArrayExpressionElement) => {
self.expression_may_have_side_efffects(e.to_expression())
self.expression_may_have_side_effects(e.to_expression())
}
ArrayExpressionElement::Elision(_) => false,
}
@@ -90,21 +90,21 @@ pub trait MayHaveSideEffects {
match e {
ObjectPropertyKind::ObjectProperty(o) => self.object_property_may_have_side_effects(o),
ObjectPropertyKind::SpreadProperty(e) => {
self.expression_may_have_side_efffects(&e.argument)
self.expression_may_have_side_effects(&e.argument)
}
}
}
fn object_property_may_have_side_effects(&self, e: &ObjectProperty<'_>) -> bool {
self.property_key_may_have_side_effects(&e.key)
|| self.expression_may_have_side_efffects(&e.value)
|| self.expression_may_have_side_effects(&e.value)
}
fn property_key_may_have_side_effects(&self, key: &PropertyKey<'_>) -> bool {
match key {
PropertyKey::StaticIdentifier(_) | PropertyKey::PrivateIdentifier(_) => false,
match_expression!(PropertyKey) => {
self.expression_may_have_side_efffects(key.to_expression())
self.expression_may_have_side_effects(key.to_expression())
}
}
}
crates/oxc_minifier/src/peephole/fold_constants.rs
+9 -5
View File
@@ -120,7 +120,7 @@ impl<'a, 'b> PeepholeOptimizations {
// (FALSE && x) => FALSE
if if lval { op == LogicalOperator::Or } else { op == LogicalOperator::And } {
return Some(ctx.ast.move_expression(&mut logical_expr.left));
} else if !ctx.expression_may_have_side_efffects(left) {
} else if !ctx.expression_may_have_side_effects(left) {
let parent = ctx.ancestry.parent();
// Bail `let o = { f() { assert.ok(this !== o); } }; (true && o.f)(); (true && o.f)``;`
if parent.is_tagged_template_expression()
@@ -145,7 +145,7 @@ impl<'a, 'b> PeepholeOptimizations {
let left_child_right_boolean = ctx.get_boolean_value(&left_child.right);
let left_child_op = left_child.operator;
if let Some(right_boolean) = left_child_right_boolean {
if !ctx.expression_may_have_side_efffects(&left_child.right) {
if !ctx.expression_may_have_side_effects(&left_child.right) {
// a || false || b => a || b
// a && true && b => a && b
if !right_boolean && left_child_op == LogicalOperator::Or
@@ -178,7 +178,7 @@ impl<'a, 'b> PeepholeOptimizations {
let left_val = ValueType::from(left);
match left_val {
ValueType::Null | ValueType::Undefined => {
Some(if ctx.expression_may_have_side_efffects(left) {
Some(if ctx.expression_may_have_side_effects(left) {
// e.g. `(a(), null) ?? 1` => `(a(), null, 1)`
let expressions = ctx.ast.vec_from_array([
ctx.ast.move_expression(&mut logical_expr.left),
@@ -381,8 +381,7 @@ impl<'a, 'b> PeepholeOptimizations {
let left = &e.left;
let right = &e.right;
let op = e.operator;
if ctx.expression_may_have_side_efffects(left)
|| ctx.expression_may_have_side_efffects(right)
if ctx.expression_may_have_side_effects(left) || ctx.expression_may_have_side_effects(right)
{
return None;
}
@@ -1828,6 +1827,11 @@ mod test {
fold("typeof foo === 'number'", "typeof foo == 'number'");
}
#[test]
fn test_issue_8782() {
fold("+(void unknown())", "+void unknown()");
}
// TODO: All big ints are rare and difficult to handle.
mod bigint {
use super::{
crates/oxc_minifier/src/peephole/remove_dead_code.rs
+4 -4
View File
@@ -389,7 +389,7 @@ impl<'a, 'b> PeepholeOptimizations {
| ValueType::Boolean
| ValueType::Number
| ValueType::String
) && !ctx.expression_may_have_side_efffects(arg)
) && !ctx.expression_may_have_side_effects(arg)
}
_ => false,
},
@@ -526,7 +526,7 @@ impl<'a, 'b> PeepholeOptimizations {
match ctx.get_boolean_value(&expr.test) {
Some(v) => {
if ctx.expression_may_have_side_efffects(&expr.test) {
if ctx.expression_may_have_side_effects(&expr.test) {
let mut exprs = ctx.ast.vec_with_capacity(2);
exprs.push(ctx.ast.move_expression(&mut expr.test));
exprs.push(ctx.ast.move_expression(if v {
@@ -564,7 +564,7 @@ impl<'a, 'b> PeepholeOptimizations {
(false, 0),
|(mut should_fold, mut new_len), (i, expr)| {
if i == sequence_expr.expressions.len() - 1
|| ctx.expression_may_have_side_efffects(expr)
|| ctx.expression_may_have_side_effects(expr)
{
new_len += 1;
} else {
@@ -582,7 +582,7 @@ impl<'a, 'b> PeepholeOptimizations {
let mut new_exprs = ctx.ast.vec_with_capacity(new_len);
let len = sequence_expr.expressions.len();
for (i, expr) in sequence_expr.expressions.iter_mut().enumerate() {
if i == len - 1 || ctx.expression_may_have_side_efffects(expr) {
if i == len - 1 || ctx.expression_may_have_side_effects(expr) {
new_exprs.push(ctx.ast.move_expression(expr));
}
}
crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs
+1 -1
View File
@@ -433,7 +433,7 @@ impl<'a> PeepholeOptimizations {
if !match argument {
Expression::Identifier(ident) => ctx.is_identifier_undefined(ident),
Expression::UnaryExpression(e) => {
e.operator.is_void() && !ctx.expression_may_have_side_efffects(argument)
e.operator.is_void() && !ctx.expression_may_have_side_effects(argument)
}
_ => false,
} {