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.

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

fix(minifier): fix remaining runtime bugs #6855

Merged
merged 1 commit into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 0 additions & 18 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,24 +271,6 @@ impl<'a> Expression<'a> {
}
}

#[allow(missing_docs)]
pub fn is_immutable_value(&self) -> bool {
match self {
Self::BooleanLiteral(_)
| Self::NullLiteral(_)
| Self::NumericLiteral(_)
| Self::BigIntLiteral(_)
| Self::RegExpLiteral(_)
| Self::StringLiteral(_) => true,
Self::TemplateLiteral(lit) if lit.is_no_substitution_template() => true,
Self::UnaryExpression(unary_expr) => unary_expr.argument.is_immutable_value(),
Self::Identifier(ident) => {
matches!(ident.name.as_str(), "undefined" | "Infinity" | "NaN")
}
_ => false,
}
}

#[allow(missing_docs)]
pub fn is_require_call(&self) -> bool {
if let Self::CallExpression(call_expr) = self {
Expand Down
18 changes: 17 additions & 1 deletion crates/oxc_ecmascript/src/constant_evaluation/is_litral_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@ pub trait IsLiteralValue<'a, 'b> {
fn is_literal_value(&self, include_functions: bool) -> bool;
}

pub fn is_immutable_value(expr: &Expression<'_>) -> bool {
match expr {
Expression::BooleanLiteral(_)
| Expression::NullLiteral(_)
| Expression::NumericLiteral(_)
| Expression::BigIntLiteral(_)
| Expression::RegExpLiteral(_)
| Expression::StringLiteral(_) => true,
Expression::TemplateLiteral(lit) if lit.is_no_substitution_template() => true,
Expression::Identifier(ident) => {
matches!(ident.name.as_str(), "undefined" | "Infinity" | "NaN")
}
_ => false,
}
}

impl<'a, 'b> IsLiteralValue<'a, 'b> for Expression<'a> {
fn is_literal_value(&self, include_functions: bool) -> bool {
match self {
Expand All @@ -27,7 +43,7 @@ impl<'a, 'b> IsLiteralValue<'a, 'b> for Expression<'a> {
Self::ObjectExpression(expr) => {
expr.properties.iter().all(|property| property.is_literal_value(include_functions))
}
_ => self.is_immutable_value(),
_ => is_immutable_value(self),
}
}
}
Expand Down
26 changes: 7 additions & 19 deletions crates/oxc_ecmascript/src/constant_evaluation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,19 +222,7 @@ pub trait ConstantEvaluation<'a> {
let rval = self.eval_to_number(right)?;
let val = match e.operator {
BinaryOperator::Subtraction => lval - rval,
BinaryOperator::Division => {
if rval.is_zero() {
if lval.is_zero() || lval.is_nan() || lval.is_infinite() {
f64::NAN
} else if lval.is_sign_positive() {
f64::INFINITY
} else {
f64::NEG_INFINITY
}
} else {
lval / rval
}
}
BinaryOperator::Division => lval / rval,
BinaryOperator::Remainder => {
if rval.is_zero() {
f64::NAN
Expand Down Expand Up @@ -332,17 +320,17 @@ pub trait ConstantEvaluation<'a> {
fn eval_unary_expression(&self, expr: &UnaryExpression<'a>) -> Option<ConstantValue<'a>> {
match expr.operator {
UnaryOperator::Typeof => {
if !expr.argument.is_literal_value(true) {
return None;
}
let s = match &expr.argument {
Expression::ObjectExpression(_) | Expression::ArrayExpression(_)
if expr.argument.is_literal_value(true) =>
{
"object"
}
Expression::FunctionExpression(_) => "function",
Expression::StringLiteral(_) => "string",
Expression::NumericLiteral(_) => "number",
Expression::BooleanLiteral(_) => "boolean",
Expression::NullLiteral(_)
| Expression::ObjectExpression(_)
| Expression::ArrayExpression(_) => "object",
Expression::NullLiteral(_) => "object",
Expression::UnaryExpression(e) if e.operator == UnaryOperator::Void => {
"undefined"
}
Expand Down
42 changes: 16 additions & 26 deletions crates/oxc_ecmascript/src/string_to_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,26 @@ pub trait StringToNumber {
/// <https://tc39.es/ecma262/#sec-stringtonumber>
impl StringToNumber for &str {
fn string_to_number(&self) -> f64 {
let s = self.trim_matches(is_trimmable_whitespace);

let s = *self;
match s {
"" => return 0.0,
"-Infinity" => return f64::NEG_INFINITY,
"Infinity" | "+Infinity" => return f64::INFINITY,
// Make sure that no further variants of "infinity" are parsed.
"inf" | "-inf" | "+inf" => return f64::NAN,
_ => {}
_ => {
// Make sure that no further variants of "infinity" are parsed by `f64::parse`.
// Note that alphabetical characters are not case-sensitive.
// <https://doc.rust-lang.org/std/primitive.f64.html#method.from_str>
let mut bytes = s.trim_start_matches(['-', '+']).bytes();
if bytes
.next()
.filter(|c| c.to_ascii_lowercase() == b'i')
.and_then(|_| bytes.next().filter(|c| c.to_ascii_lowercase() == b'n'))
.and_then(|_| bytes.next().filter(|c| c.to_ascii_lowercase() == b'f'))
.is_some()
{
return f64::NAN;
}
}
}

let mut bytes = s.bytes();
Expand Down Expand Up @@ -52,24 +63,3 @@ impl StringToNumber for &str {
s.parse::<f64>().unwrap_or(f64::NAN)
}
}

// <https://github.com/boa-dev/boa/blob/94d08fe4e68791ceca3c4b7d94ccc5f3588feeb3/core/string/src/lib.rs#L55>
/// Helper function to check if a `char` is trimmable.
pub(crate) const fn is_trimmable_whitespace(c: char) -> bool {
// The rust implementation of `trim` does not regard the same characters whitespace as ecma standard does
//
// Rust uses \p{White_Space} by default, which also includes:
// `\u{0085}' (next line)
// And does not include:
// '\u{FEFF}' (zero width non-breaking space)
// Explicit whitespace: https://tc39.es/ecma262/#sec-white-space
matches!(
c,
'\u{0009}' | '\u{000B}' | '\u{000C}' | '\u{0020}' | '\u{00A0}' | '\u{FEFF}' |
// Unicode Space_Separator category
'\u{1680}' | '\u{2000}'
..='\u{200A}' | '\u{202F}' | '\u{205F}' | '\u{3000}' |
// Line terminators: https://tc39.es/ecma262/#sec-line-terminators
'\u{000A}' | '\u{000D}' | '\u{2028}' | '\u{2029}'
)
}
8 changes: 6 additions & 2 deletions crates/oxc_linter/src/rules/eslint/prefer_numeric_literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,11 @@ impl Rule for PreferNumericLiterals {
}

fn is_string_type(arg: &Argument) -> bool {
matches!(arg, Argument::StringLiteral(_) | Argument::TemplateLiteral(_))
match arg {
Argument::StringLiteral(_) => true,
Argument::TemplateLiteral(t) => t.is_no_substitution_template(),
_ => false,
}
}

fn is_parse_int_call(
Expand All @@ -123,7 +127,7 @@ fn check_arguments<'a>(call_expr: &CallExpression<'a>, ctx: &LintContext<'a>) {
}

let string_arg = &call_expr.arguments[0];
if !is_string_type(string_arg) || !string_arg.to_expression().is_immutable_value() {
if !is_string_type(string_arg) {
return;
}

Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_linter/src/rules/import/no_dynamic_require.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ impl Rule for NoDynamicRequire {
}

fn is_static_value(expr: &Expression) -> bool {
expr.is_string_literal() && expr.is_immutable_value()
match expr {
Expression::StringLiteral(_) => true,
Expression::TemplateLiteral(t) => t.is_no_substitution_template(),
_ => false,
}
}

#[test]
Expand Down
15 changes: 14 additions & 1 deletion crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,20 @@ mod test {

test("x = Infinity / Infinity", "x = NaN");
test("x = Infinity % Infinity", "x = NaN");
test("x = Infinity / 0", "x = NaN");
test("x = Infinity / 0", "x = Infinity");
test("x = Infinity % 0", "x = NaN");
}

#[test]
fn test_to_number() {
test("x = +''", "x = 0");
test("x = +'+Infinity'", "x = Infinity");
test("x = +'-Infinity'", "x = -Infinity");

for op in ["", "+", "-"] {
for s in ["inf", "infinity", "INFINITY", "InFiNiTy"] {
test(&format!("x = +'{op}{s}'"), "x = NaN");
}
}
}
}
42 changes: 26 additions & 16 deletions crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,11 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
) -> Option<Statement<'a>> {
let test_boolean = for_stmt.test.as_ref().and_then(|test| ctx.get_boolean_value(test));
match test_boolean {
Some(false) => {
// Remove the entire `for` statement.
// Check vars in statement
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);

let mut var_decl = keep_var.get_variable_declaration();

if let Some(ForStatementInit::VariableDeclaration(var_init)) = &mut for_stmt.init {
Some(false) => match &mut for_stmt.init {
Some(ForStatementInit::VariableDeclaration(var_init)) => {
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);
let mut var_decl = keep_var.get_variable_declaration();
if var_init.kind.is_var() {
if let Some(var_decl) = &mut var_decl {
var_decl
Expand All @@ -222,14 +218,27 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
var_decl = Some(ctx.ast.move_variable_declaration(var_init));
}
}
Some(var_decl.map_or_else(
|| ctx.ast.statement_empty(SPAN),
|var_decl| {
ctx.ast
.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
},
))
}

var_decl
.map(|var_decl| {
ctx.ast.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
})
.or_else(|| Some(ctx.ast.statement_empty(SPAN)))
}
None => {
let mut keep_var = KeepVar::new(ctx.ast);
keep_var.visit_statement(&for_stmt.body);
Some(keep_var.get_variable_declaration().map_or_else(
|| ctx.ast.statement_empty(SPAN),
|var_decl| {
ctx.ast
.statement_declaration(ctx.ast.declaration_from_variable(var_decl))
},
))
}
_ => None,
},
Some(true) => {
// Remove the test expression.
for_stmt.test = None;
Expand Down Expand Up @@ -508,6 +517,7 @@ mod test {
fold("for (var se = [1, 2]; false;);", "var se = [1, 2];");
fold("for (var se = [1, 2]; false;) { var a = 0; }", "var se = [1, 2], a;");

fold("for (foo = bar; false;) {}", "for (foo = bar; false;);");
// fold("l1:for(;false;) { }", "");
}

Expand Down
44 changes: 1 addition & 43 deletions tasks/coverage/snapshots/runtime.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ commit: 06454619

runtime Summary:
AST Parsed : 18444/18444 (100.00%)
Positive Passed: 18273/18444 (99.07%)
Positive Passed: 18287/18444 (99.15%)
tasks/coverage/test262/test/annexB/language/function-code/if-decl-else-decl-a-func-existing-block-fn-no-init.js
minify error: Test262Error: Expected SameValue(«function f(){}», «undefined») to be true

Expand Down Expand Up @@ -318,9 +318,6 @@ minify error: SyntaxError: Identifier 'f' has already been declared
tasks/coverage/test262/test/annexB/language/global-code/if-stmt-else-decl-global-skip-early-err.js
minify error: SyntaxError: Identifier 'f' has already been declared

tasks/coverage/test262/test/language/expressions/addition/S11.6.1_A4_T5.js
minify error: Test262Error: #1.1: -0 + -0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/array/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValue(«true», «false») to be true

Expand All @@ -330,12 +327,6 @@ codegen error: Test262Error: descriptor value should be ; object value should be
tasks/coverage/test262/test/language/expressions/call/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValue(«true», «false») to be true

tasks/coverage/test262/test/language/expressions/conditional/in-branch-1.js
minify error: Test262Error: Expected SameValue(«0», «1») to be true

tasks/coverage/test262/test/language/expressions/division/S11.5.2_A4_T8.js
minify error: Test262Error: #1.2: -0 / 1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/does-not-equals/bigint-and-object.js
minify error: Test262Error: The result of (0n != {valueOf: function() {return 0n;}}) is false Expected SameValue(«true», «false») to be true

Expand All @@ -345,9 +336,6 @@ codegen error: Test262Error: p1 constructor is %Promise% Expected SameValue(«fu
tasks/coverage/test262/test/language/expressions/dynamic-import/eval-self-once-module.js
codegen error: Test262Error: global property was defined and incremented only once Expected SameValue(«2», «1») to be true

tasks/coverage/test262/test/language/expressions/dynamic-import/import-attributes/2nd-param-in.js
minify error: TypeError: Cannot read properties of undefined (reading 'then')

tasks/coverage/test262/test/language/expressions/dynamic-import/imported-self-update.js
codegen error: Test262Error: updated value, direct binding Expected SameValue(«0», «1») to be true

Expand Down Expand Up @@ -381,24 +369,6 @@ minify error: Test262Error: The result of (0n == {valueOf: function() {return 0n
tasks/coverage/test262/test/language/expressions/exponentiation/bigint-negative-exponent-throws.js
transform error: Test262Error: (-1n) ** -1n throws RangeError Expected a RangeError but got a TypeError

tasks/coverage/test262/test/language/expressions/logical-and/S11.11.1_A3_T2.js
minify error: Test262Error: #1.2: (-0 && -1) === -0

tasks/coverage/test262/test/language/expressions/logical-and/S11.11.1_A4_T2.js
minify error: Test262Error: #1.2: (-1 && -0) === -0

tasks/coverage/test262/test/language/expressions/logical-or/S11.11.2_A3_T2.js
minify error: Test262Error: #1.2: (0 || -0) === -0

tasks/coverage/test262/test/language/expressions/modulus/S11.5.3_A4_T2.js
minify error: Test262Error: #2.2: -1 % -1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/modulus/S11.5.3_A4_T6.js
minify error: Test262Error: #3.2: -0 % 1 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/multiplication/S11.5.1_A4_T2.js
minify error: Test262Error: #6.2: 0 * -0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/new/spread-obj-getter-init.js
transform error: Test262Error: Expected SameValue(«true», «false») to be true

Expand All @@ -414,21 +384,9 @@ transform error: TypeError: Cannot read properties of undefined (reading 'enumer
tasks/coverage/test262/test/language/expressions/object/object-spread-proxy-ownkeys-returned-keys-order.js
transform error: TypeError: Cannot read properties of undefined (reading 'enumerable')

tasks/coverage/test262/test/language/expressions/subtraction/S11.6.2_A4_T5.js
minify error: Test262Error: #3.2: -0 - 0 === - 0. Actual: +0

tasks/coverage/test262/test/language/expressions/super/call-spread-obj-getter-init.js
transform error: Test262Error: Expected SameValue(«true», «false») to be true

tasks/coverage/test262/test/language/expressions/unary-plus/S11.4.6_A3_T3.js
minify error: Test262Error: #4: +"INFINITY" === Not-a-Number. Actual: Infinity

tasks/coverage/test262/test/language/expressions/unary-plus/S9.3_A4.2_T2.js
minify error: Test262Error: #3.2: +(-0) === -0. Actual: +0

tasks/coverage/test262/test/language/expressions/unary-plus/bigint-throws.js
minify error: Test262Error: +0n throws TypeError Expected a TypeError to be thrown but no exception was thrown at all

tasks/coverage/test262/test/language/global-code/decl-func.js
codegen error: Test262Error: descriptor should not be configurable

Expand Down
Loading