Skip to content

Commit

Permalink
fix #4065: bitwise operators can return bigints
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 7, 2025
1 parent f4e9d19 commit da1de1b
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 34 deletions.
15 changes: 15 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,21 @@

Due to a copy+paste typo, the binary published to `@esbuild/netbsd-arm64` was not actually for `arm64`, and didn't run in that environment. This release should fix running esbuild in that environment (NetBSD on 64-bit ARM). Sorry about the mistake.

* Fix a minification bug with bitwise operators and bigints ([#4065](https://github.com/evanw/esbuild/issues/4065))

This change removes an incorrect assumption in esbuild that all bitwise operators result in a numeric integer. That assumption was correct up until the introduction of bigints in ES2020, but is no longer correct because almost all bitwise operators now operate on both numbers and bigints. Here's an example of the incorrect minification:

```js
// Original code
if ((a & b) !== 0) found = true

// Old output (with --minify)
a&b&&(found=!0);

// New output (with --minify)
(a&b)!==0&&(found=!0);
```

* Fix esbuild incorrectly rejecting valid TypeScript edge case ([#4027](https://github.com/evanw/esbuild/issues/4027))

The following TypeScript code is valid:
Expand Down
10 changes: 4 additions & 6 deletions internal/js_ast/js_ast_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -946,14 +946,12 @@ func ToUint32(f float64) uint32 {
return uint32(ToInt32(f))
}

// If this returns true, we know the result can't be NaN
func isInt32OrUint32(data E) bool {
switch e := data.(type) {
case *EUnary:
return e.Op == UnOpCpl

case *EBinary:
switch e.Op {
case BinOpBitwiseAnd, BinOpBitwiseOr, BinOpBitwiseXor, BinOpShl, BinOpShr, BinOpUShr:
case BinOpUShr: // This is the only bitwise operator that can't return a bigint (because it throws instead)
return true

case BinOpLogicalOr, BinOpLogicalAnd:
Expand Down Expand Up @@ -2105,10 +2103,10 @@ func (ctx HelperContext) SimplifyBooleanExpr(expr Expr) Expr {
// in a boolean context is unnecessary because the value is
// only truthy if it's not zero.
if e.Op == BinOpStrictNe || e.Op == BinOpLooseNe {
// "if ((a | b) !== 0)" => "if (a | b)"
// "if ((a >>> b) !== 0)" => "if (a >>> b)"
return left
} else {
// "if ((a | b) === 0)" => "if (!(a | b))"
// "if ((a >>> b) === 0)" => "if (!(a >>> b))"
return Not(left)
}
}
Expand Down
56 changes: 30 additions & 26 deletions internal/js_parser/js_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3793,8 +3793,12 @@ func TestMangleBooleanConstructor(t *testing.T) {
expectPrintedNormalAndMangle(t, "a = Boolean(b ? c > 0 : c < 0)", "a = Boolean(b ? c > 0 : c < 0);\n", "a = b ? c > 0 : c < 0;\n")

// Check for calling "SimplifyBooleanExpr" on the argument
expectPrintedNormalAndMangle(t, "a = Boolean((b | c) !== 0)", "a = Boolean((b | c) !== 0);\n", "a = !!(b | c);\n")
expectPrintedNormalAndMangle(t, "a = Boolean(b ? (c | d) !== 0 : (d | e) !== 0)", "a = Boolean(b ? (c | d) !== 0 : (d | e) !== 0);\n", "a = !!(b ? c | d : d | e);\n")
expectPrintedNormalAndMangle(t, "a = Boolean((b | c) !== 0)", "a = Boolean((b | c) !== 0);\n", "a = (b | c) !== 0;\n")
expectPrintedNormalAndMangle(t, "a = Boolean((b >>> c) !== 0)", "a = Boolean(b >>> c !== 0);\n", "a = !!(b >>> c);\n")
expectPrintedNormalAndMangle(t, "a = Boolean(b ? (c | d) !== 0 : (d | e) !== 0)",
"a = Boolean(b ? (c | d) !== 0 : (d | e) !== 0);\n", "a = b ? (c | d) !== 0 : (d | e) !== 0;\n")
expectPrintedNormalAndMangle(t, "a = Boolean(b ? (c >>> d) !== 0 : (d >>> e) !== 0)",
"a = Boolean(b ? c >>> d !== 0 : d >>> e !== 0);\n", "a = !!(b ? c >>> d : d >>> e);\n")
}

func TestMangleNumberConstructor(t *testing.T) {
Expand Down Expand Up @@ -4141,44 +4145,44 @@ func TestMangleIf(t *testing.T) {
expectPrintedNormalAndMangle(t, "if (!!a ? !!b : !!c) throw 0", "if (!!a ? !!b : !!c) throw 0;\n", "if (a ? b : c) throw 0;\n")

expectPrintedNormalAndMangle(t, "if ((a + b) !== 0) throw 0", "if (a + b !== 0) throw 0;\n", "if (a + b !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a | b) !== 0) throw 0", "if ((a | b) !== 0) throw 0;\n", "if (a | b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a & b) !== 0) throw 0", "if ((a & b) !== 0) throw 0;\n", "if (a & b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a ^ b) !== 0) throw 0", "if ((a ^ b) !== 0) throw 0;\n", "if (a ^ b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a << b) !== 0) throw 0", "if (a << b !== 0) throw 0;\n", "if (a << b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >> b) !== 0) throw 0", "if (a >> b !== 0) throw 0;\n", "if (a >> b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a | b) !== 0) throw 0", "if ((a | b) !== 0) throw 0;\n", "if ((a | b) !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a & b) !== 0) throw 0", "if ((a & b) !== 0) throw 0;\n", "if ((a & b) !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a ^ b) !== 0) throw 0", "if ((a ^ b) !== 0) throw 0;\n", "if ((a ^ b) !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a << b) !== 0) throw 0", "if (a << b !== 0) throw 0;\n", "if (a << b !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >> b) !== 0) throw 0", "if (a >> b !== 0) throw 0;\n", "if (a >> b !== 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >>> b) !== 0) throw 0", "if (a >>> b !== 0) throw 0;\n", "if (a >>> b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (+a !== 0) throw 0", "if (+a !== 0) throw 0;\n", "if (+a != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (~a !== 0) throw 0", "if (~a !== 0) throw 0;\n", "if (~a) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (~a !== 0) throw 0", "if (~a !== 0) throw 0;\n", "if (~a !== 0) throw 0;\n")

expectPrintedNormalAndMangle(t, "if (0 != (a + b)) throw 0", "if (0 != a + b) throw 0;\n", "if (a + b != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a | b)) throw 0", "if (0 != (a | b)) throw 0;\n", "if (a | b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a & b)) throw 0", "if (0 != (a & b)) throw 0;\n", "if (a & b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a ^ b)) throw 0", "if (0 != (a ^ b)) throw 0;\n", "if (a ^ b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a << b)) throw 0", "if (0 != a << b) throw 0;\n", "if (a << b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a >> b)) throw 0", "if (0 != a >> b) throw 0;\n", "if (a >> b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a | b)) throw 0", "if (0 != (a | b)) throw 0;\n", "if ((a | b) != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a & b)) throw 0", "if (0 != (a & b)) throw 0;\n", "if ((a & b) != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a ^ b)) throw 0", "if (0 != (a ^ b)) throw 0;\n", "if ((a ^ b) != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a << b)) throw 0", "if (0 != a << b) throw 0;\n", "if (a << b != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a >> b)) throw 0", "if (0 != a >> b) throw 0;\n", "if (a >> b != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != (a >>> b)) throw 0", "if (0 != a >>> b) throw 0;\n", "if (a >>> b) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != +a) throw 0", "if (0 != +a) throw 0;\n", "if (+a != 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != ~a) throw 0", "if (0 != ~a) throw 0;\n", "if (~a) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 != ~a) throw 0", "if (0 != ~a) throw 0;\n", "if (~a != 0) throw 0;\n")

expectPrintedNormalAndMangle(t, "if ((a + b) === 0) throw 0", "if (a + b === 0) throw 0;\n", "if (a + b === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a | b) === 0) throw 0", "if ((a | b) === 0) throw 0;\n", "if (!(a | b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a & b) === 0) throw 0", "if ((a & b) === 0) throw 0;\n", "if (!(a & b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a ^ b) === 0) throw 0", "if ((a ^ b) === 0) throw 0;\n", "if (!(a ^ b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a << b) === 0) throw 0", "if (a << b === 0) throw 0;\n", "if (!(a << b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >> b) === 0) throw 0", "if (a >> b === 0) throw 0;\n", "if (!(a >> b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a | b) === 0) throw 0", "if ((a | b) === 0) throw 0;\n", "if ((a | b) === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a & b) === 0) throw 0", "if ((a & b) === 0) throw 0;\n", "if ((a & b) === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a ^ b) === 0) throw 0", "if ((a ^ b) === 0) throw 0;\n", "if ((a ^ b) === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a << b) === 0) throw 0", "if (a << b === 0) throw 0;\n", "if (a << b === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >> b) === 0) throw 0", "if (a >> b === 0) throw 0;\n", "if (a >> b === 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if ((a >>> b) === 0) throw 0", "if (a >>> b === 0) throw 0;\n", "if (!(a >>> b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (+a === 0) throw 0", "if (+a === 0) throw 0;\n", "if (+a == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (~a === 0) throw 0", "if (~a === 0) throw 0;\n", "if (!~a) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (~a === 0) throw 0", "if (~a === 0) throw 0;\n", "if (~a === 0) throw 0;\n")

expectPrintedNormalAndMangle(t, "if (0 == (a + b)) throw 0", "if (0 == a + b) throw 0;\n", "if (a + b == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a | b)) throw 0", "if (0 == (a | b)) throw 0;\n", "if (!(a | b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a & b)) throw 0", "if (0 == (a & b)) throw 0;\n", "if (!(a & b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a ^ b)) throw 0", "if (0 == (a ^ b)) throw 0;\n", "if (!(a ^ b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a << b)) throw 0", "if (0 == a << b) throw 0;\n", "if (!(a << b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a >> b)) throw 0", "if (0 == a >> b) throw 0;\n", "if (!(a >> b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a | b)) throw 0", "if (0 == (a | b)) throw 0;\n", "if ((a | b) == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a & b)) throw 0", "if (0 == (a & b)) throw 0;\n", "if ((a & b) == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a ^ b)) throw 0", "if (0 == (a ^ b)) throw 0;\n", "if ((a ^ b) == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a << b)) throw 0", "if (0 == a << b) throw 0;\n", "if (a << b == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a >> b)) throw 0", "if (0 == a >> b) throw 0;\n", "if (a >> b == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == (a >>> b)) throw 0", "if (0 == a >>> b) throw 0;\n", "if (!(a >>> b)) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == +a) throw 0", "if (0 == +a) throw 0;\n", "if (+a == 0) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == ~a) throw 0", "if (0 == ~a) throw 0;\n", "if (!~a) throw 0;\n")
expectPrintedNormalAndMangle(t, "if (0 == ~a) throw 0", "if (0 == ~a) throw 0;\n", "if (~a == 0) throw 0;\n")
}

func TestMangleWrapToAvoidAmbiguousElse(t *testing.T) {
Expand Down
55 changes: 53 additions & 2 deletions scripts/end-to-end-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1151,8 +1151,8 @@ tests.push(
)

// Check big integer lowering
for (const target of [[], ['--target=es6']]) {
for (const minify of [[], '--minify']) {
for (const minify of [[], '--minify']) {
for (const target of [[], ['--target=es6']]) {
tests.push(test(['in.js', '--outfile=node.js', '--bundle', '--log-override:bigint=silent'].concat(target).concat(minify), {
'in.js': `
var BigInt = function() {
Expand All @@ -1177,6 +1177,57 @@ for (const target of [[], ['--target=es6']]) {
`,
}))
}

tests.push(
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!(a | b)) throw 'fail'"
function foo(a, b) { if ((a | b) === 0) throw 'fail' }
foo(0n, 0n)
foo(1n, 1n)
`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!(a & b)) throw 'fail'"
function foo(a, b) { if ((a & b) === 0) throw 'fail' }
foo(0n, 0n)
foo(1n, 1n)
`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!(a ^ b)) throw 'fail'"
function foo(a, b) { if ((a ^ b) === 0) throw 'fail' }
foo(0n, 0n)
foo(0n, 1n)
`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!(a << b)) throw 'fail'"
function foo(a, b) { if ((a << b) === 0) throw 'fail' }
foo(0n, 0n)
foo(1n, 1n)
`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!(a >> b)) throw 'fail'"
function foo(a, b) { if ((a >> b) === 0) throw 'fail' }
foo(1n, 0n)
foo(1n, 1n)
`,
}),
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
// Must not be minified to "if (!~a) throw 'fail'"
function foo(a) { if (~a === 0) throw 'fail' }
foo(-1n)
foo(1n)
`,
}),
)
}

// Check template literal lowering
Expand Down

0 comments on commit da1de1b

Please sign in to comment.