Skip to content

Commit

Permalink
feat(minifier): constant fold unary expression (#5669)
Browse files Browse the repository at this point in the history
  • Loading branch information
Boshen committed Sep 10, 2024
1 parent 86256ea commit c6bbf94
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 45 deletions.
11 changes: 9 additions & 2 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,16 @@ impl<'a> Expression<'a> {
}
}

pub fn is_number(&self) -> bool {
matches!(self, Self::NumericLiteral(_))
}

/// Determines whether the given expr is a `0`
pub fn is_number_0(&self) -> bool {
matches!(self, Self::NumericLiteral(lit) if lit.value == 0.0)
}

pub fn is_number(&self, val: f64) -> bool {
pub fn is_number_value(&self, val: f64) -> bool {
matches!(self, Self::NumericLiteral(lit) if (lit.value - val).abs() < f64::EPSILON)
}

Expand Down Expand Up @@ -260,7 +264,10 @@ impl<'a> Expression<'a> {

/// Returns literal's value converted to the Boolean type
/// returns `true` when node is truthy, `false` when node is falsy, `None` when it cannot be determined.
pub fn get_boolean_value(&self) -> Option<bool> {
/// <https://tc39.es/ecma262/#sec-toboolean>
/// 1. If argument is a Boolean, return argument.
/// 2. If argument is one of undefined, null, +0𝔽, -0𝔽, NaN, 0ℤ, or the empty String, return false.
pub fn to_boolean(&self) -> Option<bool> {
match self {
Self::BooleanLiteral(lit) => Some(lit.value),
Self::NullLiteral(_) => Some(false),
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/ast_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn is_static_boolean<'a>(expr: &Expression<'a>, ctx: &LintContext<'a>) -> bo
fn is_logical_identity(op: LogicalOperator, expr: &Expression) -> bool {
match expr {
expr if expr.is_literal() => {
let boolean_value = expr.get_boolean_value();
let boolean_value = expr.to_boolean();
(op == LogicalOperator::Or && boolean_value == Some(true))
|| (op == LogicalOperator::And && boolean_value == Some(false))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,13 @@ pub fn check_statement(statement: &Statement) -> StatementReturnStatus {
let right =
stmt.alternate.as_ref().map_or(StatementReturnStatus::NotReturn, check_statement);

test.get_boolean_value()
.map_or_else(|| left.join(right), |val| if val { left } else { right })
test.to_boolean().map_or_else(|| left.join(right), |val| if val { left } else { right })
}

Statement::WhileStatement(stmt) => {
let test = &stmt.test;
let inner_return = check_statement(&stmt.body);
if test.get_boolean_value() == Some(true) {
if test.to_boolean() == Some(true) {
inner_return
} else {
inner_return.join(StatementReturnStatus::NotReturn)
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_linter/src/rules/eslint/no_magic_numbers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ fn is_array_index<'a>(ast_kind: &AstKind<'a>, parent_kind: &AstKind<'a>) -> bool
AstKind::MemberExpression(expression) => {
if let MemberExpression::ComputedMemberExpression(computed_expression) = expression
{
return computed_expression.expression.is_number(numeric.value)
return computed_expression.expression.is_number_value(numeric.value)
&& numeric.value >= 0.0
&& numeric.value.fract() == 0.0
&& numeric.value < f64::from(u32::MAX);
Expand Down Expand Up @@ -635,7 +635,7 @@ fn test() {
(
"
type Others = [['a'], ['b']];
type Foo = {
[K in keyof Others[0]]: Others[K];
};
Expand Down Expand Up @@ -703,7 +703,7 @@ fn test() {
type Other = {
[0]: 3;
};
type Foo = {
[K in keyof Other]: `${K & number}`;
};
Expand Down Expand Up @@ -881,7 +881,7 @@ fn test() {
(
"
type Others = [['a'], ['b']];
type Foo = {
[K in keyof Others[0]]: Others[K];
};
Expand All @@ -893,7 +893,7 @@ fn test() {
type Other = {
[0]: 3;
};
type Foo = {
[K in keyof Other]: `${K & number}`;
};
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/utils/react.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub fn is_hidden_from_screen_reader<'a>(
Some(JSXAttributeValue::StringLiteral(s)) if s.value == "true" => true,
Some(JSXAttributeValue::ExpressionContainer(container)) => {
if let Some(expr) = container.expression.as_expression() {
expr.get_boolean_value().unwrap_or(false)
expr.to_boolean().unwrap_or(false)
} else {
false
}
Expand Down
19 changes: 13 additions & 6 deletions crates/oxc_minifier/src/ast_passes/fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ impl<'a> FoldConstants<'a> {
}

// [optimizeSubtree](https://github.com/google/closure-compiler/blob/75335a5138dde05030747abfd3c852cd34ea7429/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L72)
// TODO: tryReduceOperandsForOp
pub fn fold_expression(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
if let Some(folded_expr) = match expr {
Expression::BinaryExpression(e) => self.try_fold_binary_operator(e, ctx),
Expand Down Expand Up @@ -166,6 +167,17 @@ impl<'a> FoldConstants<'a> {
) -> Option<Expression<'a>> {
match expr.operator {
UnaryOperator::Typeof => self.try_fold_type_of(expr, ctx),
UnaryOperator::LogicalNot => {
expr.argument.to_boolean().map(|b| self.ast.expression_boolean_literal(SPAN, !b))
}
// `-NaN` -> `NaN`
UnaryOperator::UnaryNegation if expr.argument.is_nan() => {
Some(ctx.ast.move_expression(&mut expr.argument))
}
// `+1` -> `1`
UnaryOperator::UnaryPlus if expr.argument.is_number() => {
Some(ctx.ast.move_expression(&mut expr.argument))
}
_ => None,
}
}
Expand All @@ -191,12 +203,7 @@ impl<'a> FoldConstants<'a> {
| Expression::ArrayExpression(_) => "object",
Expression::UnaryExpression(e) if e.operator == UnaryOperator::Void => "undefined",
Expression::BigIntLiteral(_) => "bigint",
Expression::Identifier(ident)
if ident.name == "undefined"
&& ctx.symbols().is_global_identifier_reference(ident) =>
{
"undefined"
}
Expression::Identifier(ident) if ctx.is_identifier_undefined(ident) => "undefined",
_ => return None,
};
Some(self.ast.expression_string_literal(SPAN, s))
Expand Down
16 changes: 6 additions & 10 deletions crates/oxc_minifier/src/ast_passes/substitute_alternate_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use oxc_syntax::{
};
use oxc_traverse::{Traverse, TraverseCtx};

use crate::{CompressOptions, CompressorPass};
use crate::{node_util::NodeUtil, CompressOptions, CompressorPass};

/// A peephole optimization that minimizes code by simplifying conditional
/// expressions, replacing IFs with HOOKs, replacing object constructors
Expand Down Expand Up @@ -67,8 +67,8 @@ impl<'a> Traverse<'a> for SubstituteAlternateSyntax<'a> {
self.in_define_export = false;
}

fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &mut TraverseCtx<'a>) {
if !self.compress_undefined(expr) {
fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
if !self.compress_undefined(expr, ctx) {
self.compress_boolean(expr);
}
}
Expand All @@ -90,15 +90,11 @@ impl<'a> SubstituteAlternateSyntax<'a> {
/* Utilities */

/// Transforms `undefined` => `void 0`
fn compress_undefined(&self, expr: &mut Expression<'a>) -> bool {
let Expression::Identifier(ident) = expr else { return false };
if ident.name == "undefined" {
// if let Some(reference_id) = ident.reference_id.get() {
// && self.semantic.symbols().is_global_reference(reference_id)
fn compress_undefined(&self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) -> bool {
if ctx.is_expression_undefined(expr) {
*expr = self.ast.void_0();
return true;
// }
}
};
false
}

Expand Down
16 changes: 15 additions & 1 deletion crates/oxc_minifier/src/node_util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@ pub trait NodeUtil {
#[allow(unused)]
fn scopes(&self) -> &ScopeTree;

fn is_expression_undefined(&self, expr: &Expression) -> bool {
if let Expression::Identifier(ident) = expr {
return self.is_identifier_undefined(ident);
};
false
}

fn is_identifier_undefined(&self, ident: &IdentifierReference) -> bool {
if ident.name == "undefined" && self.symbols().is_global_identifier_reference(ident) {
return true;
}
false
}

/// port from [closure compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/AbstractPeepholeOptimization.java#L104-L114)
/// Returns the number value of the node if it has one and it cannot have side effects.
fn get_side_free_number_value(&self, expr: &Expression) -> Option<NumberValue> {
Expand Down Expand Up @@ -101,7 +115,7 @@ pub trait NodeUtil {
Expression::Identifier(ident) => match ident.name.as_str() {
"NaN" => Some(false),
"Infinity" => Some(true),
"undefined" if self.symbols().is_global_identifier_reference(ident) => Some(false),
"undefined" if self.is_identifier_undefined(ident) => Some(false),
_ => None,
},
Expression::AssignmentExpression(assign_expr) => {
Expand Down
31 changes: 15 additions & 16 deletions crates/oxc_minifier/tests/ast_passes/fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ fn test_nan_comparison() {
}

#[test]
fn fold_typeof() {
fn js_typeof() {
test("x = typeof 1", "x = \"number\"");
test("x = typeof 'foo'", "x = \"string\"");
test("x = typeof true", "x = \"boolean\"");
Expand All @@ -494,7 +494,6 @@ fn fold_typeof() {
}

#[test]
#[ignore]
fn unary_ops() {
// TODO: need to port
// These cases are handled by PeepholeRemoveDeadCode in closure-compiler.
Expand All @@ -507,29 +506,29 @@ fn unary_ops() {
test("a=!10", "a=false");
test("a=!false", "a=true");
test_same("a=!foo()");
test("a=-0", "a=-0.0");
test("a=-(0)", "a=-0.0");
// test("a=-0", "a=-0.0");
// test("a=-(0)", "a=-0.0");
test_same("a=-Infinity");
test("a=-NaN", "a=NaN");
test_same("a=-foo()");
test("a=~~0", "a=0");
test("a=~~10", "a=10");
test("a=~-7", "a=6");
// test("a=~~0", "a=0");
// test("a=~~10", "a=10");
// test("a=~-7", "a=6");

test("a=+true", "a=1");
// test("a=+true", "a=1");
test("a=+10", "a=10");
test("a=+false", "a=0");
// test("a=+false", "a=0");
test_same("a=+foo()");
test_same("a=+f");
test("a=+(f?true:false)", "a=+(f?1:0)");
// test("a=+(f?true:false)", "a=+(f?1:0)");
test("a=+0", "a=0");
test("a=+Infinity", "a=Infinity");
test("a=+NaN", "a=NaN");
test("a=+-7", "a=-7");
test("a=+.5", "a=.5");
// test("a=+Infinity", "a=Infinity");
// test("a=+NaN", "a=NaN");
// test("a=+-7", "a=-7");
// test("a=+.5", "a=.5");

test("a=~0xffffffff", "a=0");
test("a=~~0xffffffff", "a=-1");
// test("a=~0xffffffff", "a=0");
// test("a=~~0xffffffff", "a=-1");
// test_same("a=~.5", PeepholeFoldConstants.FRACTIONAL_BITWISE_OPERAND);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,7 @@ fn undefined() {
test("for (undefined in {}) {}", "for(undefined in {}){}");
test("undefined++", "undefined++");
test("undefined += undefined", "undefined+=void 0");

// shadowd
test_same("(function(undefined) { let x = typeof undefined; })()");
}

0 comments on commit c6bbf94

Please sign in to comment.