From 7862937551f904cd6ed2af97fbf4f7caf13dac1b Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Tue, 17 Oct 2023 19:31:09 -0400 Subject: [PATCH] fix constant folding of bigint literal equality --- CHANGELOG.md | 18 ++++++++++++++++++ internal/js_ast/js_ast_helpers.go | 25 ++++++++++++++++++++++--- internal/js_parser/js_parser.go | 6 ++++-- internal/js_parser/js_parser_test.go | 18 ++++++++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cf4398e66c..eab383e3ac4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ ## Unreleased +* Fix a constant folding bug with bigint equality + + This release fixes a bug where esbuild incorrectly checked for bigint equality by checking the equality of the bigint literal text. This is correct if the bigint doesn't have a radix because bigint literals without a radix are always in canonical form (since leading zeros are not allowed). However, this is incorrect if the bigint has a radix (e.g. `0x123n`) because the canonical form is not enforced when a radix is present. + + ```js + // Original code + console.log(!!0n, !!1n, 123n === 123n) + console.log(!!0x0n, !!0x1n, 123n === 0x7Bn) + + // Old output + console.log(false, true, true); + console.log(true, true, false); + + // New output + console.log(false, true, true); + console.log(!!0x0n, !!0x1n, 123n === 0x7Bn); + ``` + * Add some improvements to the JavaScript minifier This release adds more cases to the JavaScript minifier, including support for inlining `String.fromCharCode` and `String.prototype.charCodeAt` when possible: diff --git a/internal/js_ast/js_ast_helpers.go b/internal/js_ast/js_ast_helpers.go index eef11b935ce..8cc79eb9f2d 100644 --- a/internal/js_ast/js_ast_helpers.go +++ b/internal/js_ast/js_ast_helpers.go @@ -79,7 +79,9 @@ func MaybeSimplifyNot(expr Expr) (Expr, bool) { return Expr{Loc: expr.Loc, Data: &EBoolean{Value: e.Value == 0 || math.IsNaN(e.Value)}}, true case *EBigInt: - return Expr{Loc: expr.Loc, Data: &EBoolean{Value: e.Value == "0"}}, true + if equal, ok := CheckEqualityBigInt(e.Value, "0"); ok { + return Expr{Loc: expr.Loc, Data: &EBoolean{Value: equal}}, true + } case *EString: return Expr{Loc: expr.Loc, Data: &EBoolean{Value: len(e.Value) == 0}}, true @@ -1168,6 +1170,22 @@ func IsBinaryNullAndUndefined(left Expr, right Expr, op OpCode) (Expr, Expr, boo return Expr{}, Expr{}, false } +func CheckEqualityBigInt(a string, b string) (equal bool, ok bool) { + // Equal literals are always equal + if a == b { + return true, true + } + + // Unequal literals are unequal if neither has a radix. Leading zeros are + // disallowed in bigint literals without a radix, so in this case we know + // each value is in canonical form. + if (len(a) < 2 || a[0] != '0') && (len(b) < 2 || b[0] != '0') { + return false, true + } + + return false, false +} + type EqualityKind uint8 const ( @@ -1282,7 +1300,7 @@ func CheckEqualityIfNoSideEffects(left E, right E, kind EqualityKind) (equal boo case *EBigInt: // "0n === 0n" is true // "0n === 1n" is false - return l.Value == r.Value, true + return CheckEqualityBigInt(l.Value, r.Value) case *ENull, *EUndefined: // "(not null or undefined) == undefined" is false @@ -1760,7 +1778,8 @@ func ToBooleanWithSideEffects(data E) (boolean bool, sideEffects SideEffects, ok return e.Value != 0 && !math.IsNaN(e.Value), NoSideEffects, true case *EBigInt: - return e.Value != "0", NoSideEffects, true + equal, ok := CheckEqualityBigInt(e.Value, "0") + return !equal, NoSideEffects, ok case *EString: return len(e.Value) > 0, NoSideEffects, true diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index b56d6059f4a..463dde447b6 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -891,8 +891,10 @@ func duplicateCaseEquals(left js_ast.Expr, right js_ast.Expr) (equals bool, coul return ok && helpers.UTF16EqualsUTF16(a.Value, b.Value), false case *js_ast.EBigInt: - b, ok := right.Data.(*js_ast.EBigInt) - return ok && a.Value == b.Value, false + if b, ok := right.Data.(*js_ast.EBigInt); ok { + equal, ok := js_ast.CheckEqualityBigInt(a.Value, b.Value) + return ok && equal, false + } case *js_ast.EIdentifier: b, ok := right.Data.(*js_ast.EIdentifier) diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index 99f8c9f34b3..6aeb8e83cd4 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -2636,6 +2636,12 @@ func TestConstantFolding(t *testing.T) { expectPrinted(t, "x = !!(() => {})", "x = true;\n") expectPrinted(t, "x = !!0n", "x = false;\n") expectPrinted(t, "x = !!1n", "x = true;\n") + expectPrinted(t, "x = !!0b0n", "x = !!0b0n;\n") + expectPrinted(t, "x = !!0b1n", "x = !!0b1n;\n") + expectPrinted(t, "x = !!0o0n", "x = !!0o0n;\n") + expectPrinted(t, "x = !!0o1n", "x = !!0o1n;\n") + expectPrinted(t, "x = !!0x0n", "x = !!0x0n;\n") + expectPrinted(t, "x = !!0x1n", "x = !!0x1n;\n") expectPrinted(t, "x = 1 ? a : b", "x = 1 ? a : b;\n") expectPrinted(t, "x = 0 ? a : b", "x = 0 ? a : b;\n") @@ -2762,8 +2768,20 @@ func TestConstantFolding(t *testing.T) { expectPrinted(t, "x = Infinity === Infinity", "x = true;\n") expectPrinted(t, "x = Infinity === -Infinity", "x = false;\n") + expectPrinted(t, "x = 0n === 0n", "x = true;\n") + expectPrinted(t, "x = 1n === 1n", "x = true;\n") + expectPrinted(t, "x = 0n === 1n", "x = false;\n") + expectPrinted(t, "x = 0n !== 1n", "x = true;\n") + expectPrinted(t, "x = 0n !== 0n", "x = false;\n") expectPrinted(t, "x = 123n === 1_2_3n", "x = true;\n") + expectPrinted(t, "x = 0n === 0b0n", "x = 0n === 0b0n;\n") + expectPrinted(t, "x = 0n === 0o0n", "x = 0n === 0o0n;\n") + expectPrinted(t, "x = 0n === 0x0n", "x = 0n === 0x0n;\n") + expectPrinted(t, "x = 0b0n === 0b0n", "x = true;\n") + expectPrinted(t, "x = 0o0n === 0o0n", "x = true;\n") + expectPrinted(t, "x = 0x0n === 0x0n", "x = true;\n") + // We support folding strings from sibling AST nodes since that ends up being // equivalent with string addition. For example, "(x + 'a') + 'b'" is the // same as "x + 'ab'". However, this is not true for numbers. We can't turn