Skip to content

Commit

Permalink
feat(minifier): constant fold typeof (#5666)
Browse files Browse the repository at this point in the history
closes #5628
  • Loading branch information
Boshen committed Sep 10, 2024
1 parent 4f70fe5 commit 86256ea
Show file tree
Hide file tree
Showing 5 changed files with 150 additions and 104 deletions.
193 changes: 115 additions & 78 deletions crates/oxc_minifier/src/ast_passes/fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use oxc_traverse::{Traverse, TraverseCtx};

use crate::{
keep_var::KeepVar,
node_util::{is_exact_int64, MayHaveSideEffects, NodeUtil, NumberValue},
node_util::{is_exact_int64, IsLiteralValue, MayHaveSideEffects, NodeUtil, NumberValue},
tri::Tri,
ty::Ty,
CompressorPass,
Expand All @@ -35,7 +35,6 @@ impl<'a> Traverse<'a> for FoldConstants<'a> {

fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
self.fold_expression(expr, ctx);
self.fold_conditional_expression(expr, ctx);
}
}

Expand All @@ -49,16 +48,72 @@ impl<'a> FoldConstants<'a> {
self
}

// [optimizeSubtree](https://github.com/google/closure-compiler/blob/75335a5138dde05030747abfd3c852cd34ea7429/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L72)
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),
Expression::LogicalExpression(e) => self.try_fold_logical_expression(e, ctx),
// TODO: move to `PeepholeMinimizeConditions`
Expression::ConditionalExpression(e) => self.try_fold_conditional_expression(e, ctx),
Expression::UnaryExpression(e) => self.try_fold_unary_expression(e, ctx),
_ => None,
} {
*expr = folded_expr;
};
}

fn try_fold_binary_operator(
&self,
binary_expr: &mut BinaryExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
match binary_expr.operator {
BinaryOperator::Equality
| BinaryOperator::Inequality
| BinaryOperator::StrictEquality
| BinaryOperator::StrictInequality
| BinaryOperator::LessThan
| BinaryOperator::LessEqualThan
| BinaryOperator::GreaterThan
| BinaryOperator::GreaterEqualThan => self.try_fold_comparison(
binary_expr.span,
binary_expr.operator,
&binary_expr.left,
&binary_expr.right,
ctx,
),
BinaryOperator::ShiftLeft
| BinaryOperator::ShiftRight
| BinaryOperator::ShiftRightZeroFill => self.try_fold_shift(
binary_expr.span,
binary_expr.operator,
&binary_expr.left,
&binary_expr.right,
ctx,
),
// NOTE: string concat folding breaks our current evaluation of Test262 tests. The
// minifier is tested by comparing output of running the minifier once and twice,
// respectively. Since Test262Error messages include string concats, the outputs
// don't match (even though the produced code is valid). Additionally, We'll likely
// want to add `evaluate` checks for all constant folding, not just additions, but
// we're adding this here until a decision is made.
BinaryOperator::Addition if self.evaluate => {
self.try_fold_addition(binary_expr.span, &binary_expr.left, &binary_expr.right, ctx)
}
_ => None,
}
}

fn fold_expression_and_get_boolean_value(
&mut self,
&self,
expr: &mut Expression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<bool> {
self.fold_expression(expr, ctx);
ctx.get_boolean_value(expr)
}

fn fold_if_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
let Statement::IfStatement(if_stmt) = stmt else { return };

// Descend and remove `else` blocks first.
Expand Down Expand Up @@ -89,80 +144,66 @@ impl<'a> FoldConstants<'a> {
}
}

fn fold_conditional_expression(
&mut self,
expr: &mut Expression<'a>,
fn try_fold_conditional_expression(
&self,
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) {
let Expression::ConditionalExpression(conditional_expr) = expr else {
return;
};
) -> Option<Expression<'a>> {
if ctx.ancestry.parent().is_tagged_template_expression() {
return;
return None;
}
match self.fold_expression_and_get_boolean_value(&mut conditional_expr.test, ctx) {
Some(true) => {
*expr = self.ast.move_expression(&mut conditional_expr.consequent);
}
Some(false) => {
*expr = self.ast.move_expression(&mut conditional_expr.alternate);
}
_ => {}
match self.fold_expression_and_get_boolean_value(&mut expr.test, ctx) {
Some(true) => Some(self.ast.move_expression(&mut expr.consequent)),
Some(false) => Some(self.ast.move_expression(&mut expr.alternate)),
_ => None,
}
}

pub fn fold_expression<'b>(&mut self, expr: &'b mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
let folded_expr = match expr {
Expression::BinaryExpression(binary_expr) => match binary_expr.operator {
BinaryOperator::Equality
| BinaryOperator::Inequality
| BinaryOperator::StrictEquality
| BinaryOperator::StrictInequality
| BinaryOperator::LessThan
| BinaryOperator::LessEqualThan
| BinaryOperator::GreaterThan
| BinaryOperator::GreaterEqualThan => self.try_fold_comparison(
binary_expr.span,
binary_expr.operator,
&binary_expr.left,
&binary_expr.right,
ctx,
),
BinaryOperator::ShiftLeft
| BinaryOperator::ShiftRight
| BinaryOperator::ShiftRightZeroFill => self.try_fold_shift(
binary_expr.span,
binary_expr.operator,
&binary_expr.left,
&binary_expr.right,
ctx,
),
// NOTE: string concat folding breaks our current evaluation of Test262 tests. The
// minifier is tested by comparing output of running the minifier once and twice,
// respectively. Since Test262Error messages include string concats, the outputs
// don't match (even though the produced code is valid). Additionally, We'll likely
// want to add `evaluate` checks for all constant folding, not just additions, but
// we're adding this here until a decision is made.
BinaryOperator::Addition if self.evaluate => self.try_fold_addition(
binary_expr.span,
&binary_expr.left,
&binary_expr.right,
ctx,
),
_ => None,
},
Expression::LogicalExpression(logic_expr) => {
self.try_fold_logical_expression(logic_expr, ctx)
}
fn try_fold_unary_expression(
&self,
expr: &mut UnaryExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
match expr.operator {
UnaryOperator::Typeof => self.try_fold_type_of(expr, ctx),
_ => None,
};
if let Some(folded_expr) = folded_expr {
*expr = folded_expr;
}
}

/// Folds 'typeof(foo)' if foo is a literal, e.g.
/// `typeof("bar") --> "string"`
/// `typeof(6) --> "number"`
fn try_fold_type_of(
&self,
expr: &mut UnaryExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
if !expr.argument.is_literal_value(/* include_function */ true) {
return None;
}
let s = match &mut expr.argument {
Expression::FunctionExpression(_) => "function",
Expression::StringLiteral(_) => "string",
Expression::NumericLiteral(_) => "number",
Expression::BooleanLiteral(_) => "boolean",
Expression::NullLiteral(_)
| Expression::ObjectExpression(_)
| 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"
}
_ => return None,
};
Some(self.ast.expression_string_literal(SPAN, s))
}

fn try_fold_addition<'b>(
&mut self,
&self,
span: Span,
left: &'b Expression<'a>,
right: &'b Expression<'a>,
Expand Down Expand Up @@ -206,7 +247,7 @@ impl<'a> FoldConstants<'a> {
}

fn try_fold_comparison<'b>(
&mut self,
&self,
span: Span,
op: BinaryOperator,
left: &'b Expression<'a>,
Expand All @@ -222,7 +263,7 @@ impl<'a> FoldConstants<'a> {
}

fn evaluate_comparison<'b>(
&mut self,
&self,
op: BinaryOperator,
left: &'b Expression<'a>,
right: &'b Expression<'a>,
Expand All @@ -231,7 +272,6 @@ impl<'a> FoldConstants<'a> {
if left.may_have_side_effects() || right.may_have_side_effects() {
return Tri::Unknown;
}

match op {
BinaryOperator::Equality => self.try_abstract_equality_comparison(left, right, ctx),
BinaryOperator::Inequality => {
Expand Down Expand Up @@ -261,7 +301,7 @@ impl<'a> FoldConstants<'a> {

/// <https://tc39.es/ecma262/#sec-abstract-equality-comparison>
fn try_abstract_equality_comparison<'b>(
&mut self,
&self,
left_expr: &'b Expression<'a>,
right_expr: &'b Expression<'a>,
ctx: &mut TraverseCtx<'a>,
Expand Down Expand Up @@ -526,7 +566,7 @@ impl<'a> FoldConstants<'a> {
/// ported from [closure-compiler](https://github.com/google/closure-compiler/blob/a4c880032fba961f7a6c06ef99daa3641810bfdd/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L1114-L1162)
#[allow(clippy::cast_possible_truncation)]
fn try_fold_shift<'b>(
&mut self,
&self,
span: Span,
op: BinaryOperator,
left: &'b Expression<'a>,
Expand Down Expand Up @@ -580,7 +620,7 @@ impl<'a> FoldConstants<'a> {
///
/// port from [closure-compiler](https://github.com/google/closure-compiler/blob/09094b551915a6487a980a783831cba58b5739d1/src/com/google/javascript/jscomp/PeepholeFoldConstants.java#L587)
pub fn try_fold_logical_expression(
&mut self,
&self,
logical_expr: &mut LogicalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
Expand Down Expand Up @@ -654,7 +694,7 @@ impl<'a> FoldConstants<'a> {
}

pub(crate) fn fold_condition<'b>(
&mut self,
&self,
stmt: &'b mut Statement<'a>,
ctx: &mut TraverseCtx<'a>,
) {
Expand Down Expand Up @@ -684,10 +724,7 @@ impl<'a> FoldConstants<'a> {
};
}

fn fold_expression_in_condition(
&mut self,
expr: &mut Expression<'a>,
) -> Option<Expression<'a>> {
fn fold_expression_in_condition(&self, expr: &mut Expression<'a>) -> Option<Expression<'a>> {
let folded_expr = match expr {
Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
UnaryOperator::LogicalNot => {
Expand Down
14 changes: 5 additions & 9 deletions crates/oxc_minifier/src/node_util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use oxc_ast::ast::*;
use oxc_semantic::{ScopeTree, SymbolTable};
use oxc_syntax::operator::{AssignmentOperator, LogicalOperator, UnaryOperator};

pub use self::{may_have_side_effects::MayHaveSideEffects, number_value::NumberValue};
pub use self::{
is_literal_value::IsLiteralValue, may_have_side_effects::MayHaveSideEffects,
number_value::NumberValue,
};

pub fn is_exact_int64(num: f64) -> bool {
num.fract() == 0.0
Expand Down Expand Up @@ -98,14 +101,7 @@ pub trait NodeUtil {
Expression::Identifier(ident) => match ident.name.as_str() {
"NaN" => Some(false),
"Infinity" => Some(true),
"undefined"
if ident
.reference_id
.get()
.is_some_and(|id| self.symbols().is_global_reference(id)) =>
{
Some(false)
}
"undefined" if self.symbols().is_global_identifier_reference(ident) => Some(false),
_ => None,
},
Expression::AssignmentExpression(assign_expr) => {
Expand Down
33 changes: 16 additions & 17 deletions crates/oxc_minifier/tests/ast_passes/fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,23 +475,22 @@ fn test_nan_comparison() {
}

#[test]
#[ignore]
fn js_typeof() {
test("x = typeof 1", "x='number'");
test("x = typeof 'foo'", "x='string'");
test("x = typeof true", "x='boolean'");
test("x = typeof false", "x='boolean'");
test("x = typeof null", "x='object'");
test("x = typeof undefined", "x='undefined'");
test("x = typeof void 0", "x='undefined'");
test("x = typeof []", "x='object'");
test("x = typeof [1]", "x='object'");
test("x = typeof [1,[]]", "x='object'");
test("x = typeof {}", "x='object'");
test("x = typeof function() {}", "x='function'");

test_same("x=typeof [1,[foo()]]");
test_same("x=typeof {bathwater:baby()}");
fn fold_typeof() {
test("x = typeof 1", "x = \"number\"");
test("x = typeof 'foo'", "x = \"string\"");
test("x = typeof true", "x = \"boolean\"");
test("x = typeof false", "x = \"boolean\"");
test("x = typeof null", "x = \"object\"");
test("x = typeof undefined", "x = \"undefined\"");
test("x = typeof void 0", "x = \"undefined\"");
test("x = typeof []", "x = \"object\"");
test("x = typeof [1]", "x = \"object\"");
test("x = typeof [1,[]]", "x = \"object\"");
test("x = typeof {}", "x = \"object\"");
test("x = typeof function() {}", "x = 'function'");

test_same("x = typeof[1,[foo()]]");
test_same("x = typeof{bathwater:baby()}");
}

#[test]
Expand Down
9 changes: 9 additions & 0 deletions crates/oxc_minifier/tests/ast_passes/remove_dead_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ fn dce_if_statement() {

// parenthesized
test("if (!!(false)) { REMOVE; } else { KEEP; }", "{ KEEP }");

// typeof
test("if (typeof 1 !== 'number') { REMOVE; }", "");
test("if (typeof false !== 'boolean') { REMOVE; }", "");
test(
"if (typeof 1 === 'string') { REMOVE;
}",
"",
);
}

#[test]
Expand Down
5 changes: 5 additions & 0 deletions crates/oxc_semantic/src/symbol.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(non_snake_case)] // Silence erroneous warnings from Rust Analyser for `#[derive(Tsify)]`

use oxc_ast::ast::IdentifierReference;
use oxc_index::IndexVec;
use oxc_span::{CompactStr, Span};
pub use oxc_syntax::{
Expand Down Expand Up @@ -176,6 +177,10 @@ impl SymbolTable {
self.references[reference_id].symbol_id().is_none()
}

pub fn is_global_identifier_reference(&self, ident: &IdentifierReference<'_>) -> bool {
ident.reference_id.get().is_some_and(|reference_id| self.is_global_reference(reference_id))
}

#[inline]
pub fn get_resolved_reference_ids(&self, symbol_id: SymbolId) -> &Vec<ReferenceId> {
&self.resolved_references[symbol_id]
Expand Down

0 comments on commit 86256ea

Please sign in to comment.