Skip to content

Commit

Permalink
fix constant folding of bigint literal equality
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Oct 17, 2023
1 parent 126ca5a commit 7862937
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 5 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
25 changes: 22 additions & 3 deletions internal/js_ast/js_ast_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions internal/js_parser/js_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7862937

Please sign in to comment.