Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor(minifier): align code with closure compiler #5717

Merged
merged 1 commit into from
Sep 12, 2024
Merged

refactor(minifier): align code with closure compiler #5717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 38 additions & 29 deletions crates/oxc_minifier/src/ast_passes/fold_constants.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ impl<'a> FoldConstants<'a> {
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),
Expression::LogicalExpression(e)
if matches!(e.operator, LogicalOperator::And | LogicalOperator::Or) =>
{
self.try_fold_and_or(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),
Expand Down Expand Up @@ -111,7 +115,7 @@ impl<'a> FoldConstants<'a> {
ctx: &mut TraverseCtx<'a>,
) -> Option<bool> {
self.fold_expression(expr, ctx);
ctx.get_boolean_value(expr)
ctx.get_boolean_value(expr).to_option()
}

fn fold_if_statement(&self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
Expand Down Expand Up @@ -150,11 +154,15 @@ impl<'a> FoldConstants<'a> {
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
if ctx.ancestry.parent().is_tagged_template_expression() {
return None;
}
match self.fold_expression_and_get_boolean_value(&mut expr.test, ctx) {
Some(true) => Some(self.ast.move_expression(&mut expr.consequent)),
Some(true) => {
// Bail `let o = { f() { assert.ok(this !== o); } }; (true ? o.f : false)(); (true ? o.f : false)``;`
let parent = ctx.ancestry.parent();
if parent.is_tagged_template_expression() || parent.is_call_expression() {
return None;
}
Some(self.ast.move_expression(&mut expr.consequent))
}
Some(false) => Some(self.ast.move_expression(&mut expr.alternate)),
_ => None,
}
Expand Down Expand Up @@ -384,7 +392,7 @@ impl<'a> FoldConstants<'a> {
let right_bigint = ctx.get_side_free_bigint_value(right_expr);

if let (Some(l_big), Some(r_big)) = (left_bigint, right_bigint) {
return Tri::for_boolean(l_big.eq(&r_big));
return Tri::from(l_big.eq(&r_big));
}
}

Expand Down Expand Up @@ -421,7 +429,7 @@ impl<'a> FoldConstants<'a> {
return Tri::Unknown;
}

return Tri::for_boolean(left_string.cmp(&right_string) == Ordering::Less);
return Tri::from(left_string.cmp(&right_string) == Ordering::Less);
}

if let (Expression::UnaryExpression(left), Expression::UnaryExpression(right)) =
Expand Down Expand Up @@ -450,14 +458,14 @@ impl<'a> FoldConstants<'a> {
match (left_bigint, right_bigint, left_num, right_num) {
// Next, try to evaluate based on the value of the node. Try comparing as BigInts first.
(Some(l_big), Some(r_big), _, _) => {
return Tri::for_boolean(l_big < r_big);
return Tri::from(l_big < r_big);
}
// try comparing as Numbers.
(_, _, Some(l_num), Some(r_num)) => match (l_num, r_num) {
(NumberValue::NaN, _) | (_, NumberValue::NaN) => {
return Tri::for_boolean(will_negative);
return Tri::from(will_negative);
}
(NumberValue::Number(l), NumberValue::Number(r)) => return Tri::for_boolean(l < r),
(NumberValue::Number(l), NumberValue::Number(r)) => return Tri::from(l < r),
_ => {}
},
// Finally, try comparisons between BigInt and Number.
Expand All @@ -484,7 +492,7 @@ impl<'a> FoldConstants<'a> {
// if invert is false, then the number is on the right in tryAbstractRelationalComparison
// if it's true, then the number is on the left
match number_value {
NumberValue::NaN => Tri::for_boolean(will_negative),
NumberValue::NaN => Tri::from(will_negative),
NumberValue::PositiveInfinity => Tri::True.xor(invert),
NumberValue::NegativeInfinity => Tri::False.xor(invert),
NumberValue::Number(num) => {
Expand All @@ -502,7 +510,7 @@ impl<'a> FoldConstants<'a> {
if is_exact_int64(*num) {
Tri::False
} else {
Tri::for_boolean(num.is_sign_positive()).xor(invert)
Tri::from(num.is_sign_positive()).xor(invert)
}
}
}
Expand Down Expand Up @@ -534,7 +542,7 @@ impl<'a> FoldConstants<'a> {
return Tri::False;
}

return Tri::for_boolean(l_num == r_num);
return Tri::from(l_num == r_num);
}

Tri::Unknown
Expand All @@ -548,7 +556,7 @@ impl<'a> FoldConstants<'a> {
return Tri::Unknown;
}

return Tri::for_boolean(left_string == right_string);
return Tri::from(left_string == right_string);
}

if let (Expression::UnaryExpression(left), Expression::UnaryExpression(right)) =
Expand Down Expand Up @@ -640,22 +648,20 @@ impl<'a> FoldConstants<'a> {
/// Try to fold a AND / OR node.
///
/// 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(
pub fn try_fold_and_or(
&self,
logical_expr: &mut LogicalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
if ctx.ancestry.parent().is_tagged_template_expression() {
return None;
}
let op = logical_expr.operator;
if !matches!(op, LogicalOperator::And | LogicalOperator::Or) {
return None;
}
debug_assert!(matches!(op, LogicalOperator::And | LogicalOperator::Or));

if let Some(boolean_value) = ctx.get_boolean_value(&logical_expr.left) {
let left = &logical_expr.left;
let left_val = ctx.get_boolean_value(left).to_option();

if let Some(lval) = left_val {
// Bail `0 && (module.exports = {})` for `cjs-module-lexer`.
if !boolean_value {
if !lval {
if let Expression::AssignmentExpression(assign_expr) = &logical_expr.right {
if let Some(member_expr) = assign_expr.left.as_member_expression() {
if member_expr.is_specific_member_access("module", "exports") {
Expand All @@ -667,11 +673,14 @@ impl<'a> FoldConstants<'a> {

// (TRUE || x) => TRUE (also, (3 || x) => 3)
// (FALSE && x) => FALSE
if (boolean_value && op == LogicalOperator::Or)
|| (!boolean_value && op == LogicalOperator::And)
{
if if lval { op == LogicalOperator::Or } else { op == LogicalOperator::And } {
return Some(self.ast.move_expression(&mut logical_expr.left));
} else if !logical_expr.left.may_have_side_effects() {
} else if !left.may_have_side_effects() {
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() || parent.is_call_expression() {
return None;
}
// (FALSE || x) => x
// (TRUE && x) => x
return Some(self.ast.move_expression(&mut logical_expr.right));
Expand All @@ -688,7 +697,7 @@ impl<'a> FoldConstants<'a> {
return Some(sequence_expr);
} else if let Expression::LogicalExpression(left_child) = &mut logical_expr.left {
if left_child.operator == logical_expr.operator {
let left_child_right_boolean = ctx.get_boolean_value(&left_child.right);
let left_child_right_boolean = ctx.get_boolean_value(&left_child.right).to_option();
let left_child_op = left_child.operator;
if let Some(right_boolean) = left_child_right_boolean {
if !left_child.right.may_have_side_effects() {
Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_minifier/src/node_util/check_for_state_change.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ fn is_simple_unary_operator(operator: UnaryOperator) -> bool {
operator != UnaryOperator::Delete
}

/// port from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L241)
/// Returns true if some node in n's subtree changes application state. If
/// `check_for_new_objects` is true, we assume that newly created mutable objects (like object
/// literals) change state. Otherwise, we assume that they have no side effects.
///
/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L241)
pub trait CheckForStateChange<'a, 'b> {
fn check_for_state_change(&self, check_for_new_objects: bool) -> bool;
}
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_minifier/src/node_util/may_have_side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@ use oxc_ast::ast::*;

use super::check_for_state_change::CheckForStateChange;

/// port from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94)
/// Returns true if the node which may have side effects when executed.
/// This version default to the "safe" assumptions when the compiler object
/// is not provided (RegExp have side-effects, etc).
///
/// Ported from [closure-compiler](https://github.com/google/closure-compiler/blob/f3ce5ed8b630428e311fe9aa2e20d36560d975e2/src/com/google/javascript/jscomp/AstAnalyzer.java#L94)
pub trait MayHaveSideEffects<'a, 'b>
where
Self: CheckForStateChange<'a, 'b>,
{
fn may_have_side_effects(&self) -> bool {
self.check_for_state_change(false)
self.check_for_state_change(/* check_for_new_objects */ false)
}
}

Expand Down
66 changes: 36 additions & 30 deletions crates/oxc_minifier/src/node_util/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use oxc_ast::ast::*;
use oxc_semantic::{IsGlobalReference, ScopeTree, SymbolTable};
use oxc_syntax::operator::{AssignmentOperator, LogicalOperator, UnaryOperator};

use crate::tri::Tri;

pub use self::{
is_literal_value::IsLiteralValue, may_have_side_effects::MayHaveSideEffects,
number_value::NumberValue,
Expand Down Expand Up @@ -89,20 +91,22 @@ pub trait NodeUtil {
/// Gets the boolean value of a node that represents an expression, or `None` if no
/// such value can be determined by static analysis.
/// This method does not consider whether the node may have side-effects.
fn get_boolean_value(&self, expr: &Expression) -> Option<bool> {
fn get_boolean_value(&self, expr: &Expression) -> Tri {
match expr {
Expression::RegExpLiteral(_)
| Expression::ArrayExpression(_)
| Expression::ArrowFunctionExpression(_)
| Expression::ClassExpression(_)
| Expression::FunctionExpression(_)
| Expression::NewExpression(_)
| Expression::ObjectExpression(_) => Some(true),
Expression::NullLiteral(_) => Some(false),
Expression::BooleanLiteral(boolean_literal) => Some(boolean_literal.value),
Expression::NumericLiteral(number_literal) => Some(number_literal.value != 0.0),
Expression::BigIntLiteral(big_int_literal) => Some(!big_int_literal.is_zero()),
Expression::StringLiteral(string_literal) => Some(!string_literal.value.is_empty()),
| Expression::ObjectExpression(_) => Tri::True,
Expression::NullLiteral(_) => Tri::False,
Expression::BooleanLiteral(boolean_literal) => Tri::from(boolean_literal.value),
Expression::NumericLiteral(number_literal) => Tri::from(number_literal.value != 0.0),
Expression::BigIntLiteral(big_int_literal) => Tri::from(!big_int_literal.is_zero()),
Expression::StringLiteral(string_literal) => {
Tri::from(!string_literal.value.is_empty())
}
Expression::TemplateLiteral(template_literal) => {
// only for ``
template_literal
Expand All @@ -111,16 +115,17 @@ pub trait NodeUtil {
.filter(|quasi| quasi.tail)
.and_then(|quasi| quasi.value.cooked.as_ref())
.map(|cooked| !cooked.is_empty())
.into()
}
Expression::Identifier(ident) => match ident.name.as_str() {
"NaN" => Some(false),
"Infinity" => Some(true),
"undefined" if self.is_identifier_undefined(ident) => Some(false),
_ => None,
"NaN" => Tri::False,
"Infinity" => Tri::True,
"undefined" if self.is_identifier_undefined(ident) => Tri::False,
_ => Tri::Unknown,
},
Expression::AssignmentExpression(assign_expr) => {
match assign_expr.operator {
AssignmentOperator::LogicalAnd | AssignmentOperator::LogicalOr => None,
AssignmentOperator::LogicalAnd | AssignmentOperator::LogicalOr => Tri::Unknown,
// For ASSIGN, the value is the value of the RHS.
_ => self.get_boolean_value(&assign_expr.right),
}
Expand All @@ -133,36 +138,35 @@ pub trait NodeUtil {
LogicalOperator::And => {
let left = self.get_boolean_value(&logical_expr.left);
let right = self.get_boolean_value(&logical_expr.right);

match (left, right) {
(Some(true), Some(true)) => Some(true),
(Some(false), _) | (_, Some(false)) => Some(false),
(None, _) | (_, None) => None,
(Tri::True, Tri::True) => Tri::True,
(Tri::False, _) | (_, Tri::False) => Tri::False,
(Tri::Unknown, _) | (_, Tri::Unknown) => Tri::Unknown,
}
}
// true || false -> true
// false || false -> false
// a || b -> None
// a || b -> Tri::Unknown
LogicalOperator::Or => {
let left = self.get_boolean_value(&logical_expr.left);
let right = self.get_boolean_value(&logical_expr.right);

match (left, right) {
(Some(true), _) | (_, Some(true)) => Some(true),
(Some(false), Some(false)) => Some(false),
(None, _) | (_, None) => None,
(Tri::True, _) | (_, Tri::True) => Tri::True,
(Tri::False, Tri::False) => Tri::False,
(Tri::Unknown, _) | (_, Tri::Unknown) => Tri::Unknown,
}
}
LogicalOperator::Coalesce => None,
LogicalOperator::Coalesce => Tri::Unknown,
}
}
Expression::SequenceExpression(sequence_expr) => {
// For sequence expression, the value is the value of the RHS.
sequence_expr.expressions.last().and_then(|e| self.get_boolean_value(e))
sequence_expr.expressions.last().map_or(Tri::Unknown, |e| self.get_boolean_value(e))
}
Expression::UnaryExpression(unary_expr) => {
if unary_expr.operator == UnaryOperator::Void {
Some(false)
Tri::False
} else if matches!(
unary_expr.operator,
UnaryOperator::BitwiseNot
Expand All @@ -173,15 +177,17 @@ pub trait NodeUtil {
// +1 -> true
// +0 -> false
// -0 -> false
self.get_number_value(expr).map(|value| value != NumberValue::Number(0_f64))
self.get_number_value(expr)
.map(|value| value != NumberValue::Number(0_f64))
.into()
} else if unary_expr.operator == UnaryOperator::LogicalNot {
// !true -> false
self.get_boolean_value(&unary_expr.argument).map(|boolean| !boolean)
self.get_boolean_value(&unary_expr.argument).not()
} else {
None
Tri::Unknown
}
}
_ => None,
_ => Tri::Unknown,
}
}

Expand Down Expand Up @@ -213,7 +219,7 @@ pub trait NodeUtil {
}
UnaryOperator::LogicalNot => self
.get_boolean_value(expr)
.map(|boolean| if boolean { 1_f64 } else { 0_f64 })
.map(|tri| if tri.is_true() { 1_f64 } else { 0_f64 })
.map(NumberValue::Number),
UnaryOperator::Void => Some(NumberValue::NaN),
_ => None,
Expand Down Expand Up @@ -264,7 +270,7 @@ pub trait NodeUtil {
}
Expression::UnaryExpression(unary_expr) => match unary_expr.operator {
UnaryOperator::LogicalNot => self.get_boolean_value(expr).map(|boolean| {
if boolean {
if boolean.is_true() {
BigInt::one()
} else {
BigInt::zero()
Expand Down Expand Up @@ -336,7 +342,7 @@ pub trait NodeUtil {
UnaryOperator::LogicalNot => {
self.get_boolean_value(&unary_expr.argument).map(|boolean| {
// need reversed.
if boolean {
if boolean.is_true() {
Cow::Borrowed("false")
} else {
Cow::Borrowed("true")
Expand Down
Loading
Loading