Skip to content

Commit

Permalink
fix #3416, fix #3425: better enum constant folding
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed May 24, 2024
1 parent 8f1faf7 commit 9e2f304
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 19 deletions.
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,35 @@
}
```

* Do additional constant folding after cross-module enum inlining ([#3416](https://github.com/evanw/esbuild/issues/3416), [#3425](https://github.com/evanw/esbuild/issues/3425))

This release adds a few more cases where esbuild does constant folding after cross-module enum inlining.

```ts
// Original code: enum.ts
export enum Platform {
WINDOWS = 'windows',
MACOS = 'macos',
LINUX = 'linux',
}

// Original code: main.ts
import { Platform } from './enum';
declare const PLATFORM: string;
export function logPlatform() {
if (PLATFORM == Platform.WINDOWS) console.log('Windows');
else if (PLATFORM == Platform.MACOS) console.log('macOS');
else if (PLATFORM == Platform.LINUX) console.log('Linux');
else console.log('Other');
}

// Old output (with --bundle '--define:PLATFORM="macos"' --minify --format=esm)
function n(){"windows"=="macos"?console.log("Windows"):"macos"=="macos"?console.log("macOS"):"linux"=="macos"?console.log("Linux"):console.log("Other")}export{n as logPlatform};

// New output (with --bundle '--define:PLATFORM="macos"' --minify --format=esm)
function n(){console.log("macOS")}export{n as logPlatform};
```

* Formatting support for the `@position-try` rule ([#3773](https://github.com/evanw/esbuild/issues/3773))

Chrome shipped this new CSS at-rule in version 125 as part of the [CSS anchor positioning API](https://developer.chrome.com/blog/anchor-positioning-api). With this release, esbuild now knows to expect a declaration list inside of the `@position-try` body block and will format it appropriately.
Expand Down
8 changes: 8 additions & 0 deletions internal/bundler_tests/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3183,6 +3183,8 @@ func TestCrossModuleConstantFoldingNumber(t *testing.T) {
x.a && x.b,
x.a || x.b,
x.a ?? x.b,
x.a ? 'y' : 'n',
!x.b ? 'y' : 'n',
])
`,

Expand Down Expand Up @@ -3226,6 +3228,8 @@ func TestCrossModuleConstantFoldingNumber(t *testing.T) {
a && b,
a || b,
a ?? b,
a ? 'y' : 'n',
!b ? 'y' : 'n',
])
`,

Expand Down Expand Up @@ -3288,6 +3292,8 @@ func TestCrossModuleConstantFoldingString(t *testing.T) {
x.a && x.b,
x.a || x.b,
x.a ?? x.b,
x.a ? 'y' : 'n',
!x.b ? 'y' : 'n',
])
`,

Expand All @@ -3314,6 +3320,8 @@ func TestCrossModuleConstantFoldingString(t *testing.T) {
a && b,
a || b,
a ?? b,
a ? 'y' : 'n',
!b ? 'y' : 'n',
])
`,

Expand Down
16 changes: 12 additions & 4 deletions internal/bundler_tests/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,9 @@ console.log([
], [
6 /* b */,
3 /* a */,
3 /* a */
3 /* a */,
"y",
"n"
]);

---------- /out/const-entry.js ----------
Expand Down Expand Up @@ -331,7 +333,9 @@ console.log([
], [
6,
3,
3
3,
"y",
"n"
]);

---------- /out/nested-entry.js ----------
Expand Down Expand Up @@ -361,7 +365,9 @@ console.log([
], [
"bar" /* b */,
"foo" /* a */,
"foo" /* a */
"foo" /* a */,
"y",
"n"
]);

---------- /out/const-entry.js ----------
Expand All @@ -385,7 +391,9 @@ console.log([
], [
a && b,
a || b,
a ?? b
a ?? b,
a ? "y" : "n",
b ? "n" : "y"
]);

---------- /out/nested-entry.js ----------
Expand Down
47 changes: 32 additions & 15 deletions internal/js_printer/js_printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1749,7 +1749,7 @@ func (p *printer) guardAgainstBehaviorChangeDueToSubstitution(expr js_ast.Expr,
// module numeric constants and bitwise operations. This is not an general-
// purpose/optimal approach and never will be. For example, we can't affect
// tree shaking at this stage because it has already happened.
func (p *printer) lateConstantFoldUnaryOrBinaryExpr(expr js_ast.Expr) js_ast.Expr {
func (p *printer) lateConstantFoldUnaryOrBinaryOrIfExpr(expr js_ast.Expr) js_ast.Expr {
switch e := expr.Data.(type) {
case *js_ast.EImportIdentifier:
ref := ast.FollowSymbols(p.symbols, e.Ref)
Expand Down Expand Up @@ -1779,7 +1779,7 @@ func (p *printer) lateConstantFoldUnaryOrBinaryExpr(expr js_ast.Expr) js_ast.Exp
}

case *js_ast.EUnary:
value := p.lateConstantFoldUnaryOrBinaryExpr(e.Value)
value := p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.Value)

// Only fold again if something chained
if value.Data != e.Value.Data {
Expand All @@ -1802,8 +1802,8 @@ func (p *printer) lateConstantFoldUnaryOrBinaryExpr(expr js_ast.Expr) js_ast.Exp
}

case *js_ast.EBinary:
left := p.lateConstantFoldUnaryOrBinaryExpr(e.Left)
right := p.lateConstantFoldUnaryOrBinaryExpr(e.Right)
left := p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.Left)
right := p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.Right)

// Only fold again if something changed
if left.Data != e.Left.Data || right.Data != e.Right.Data {
Expand All @@ -1819,6 +1819,23 @@ func (p *printer) lateConstantFoldUnaryOrBinaryExpr(expr js_ast.Expr) js_ast.Exp
// Don't mutate the original AST
expr.Data = binary
}

case *js_ast.EIf:
test := p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.Test)

// Only fold again if something changed
if test.Data != e.Test.Data {
if boolean, sideEffects, ok := js_ast.ToBooleanWithSideEffects(test.Data); ok && sideEffects == js_ast.NoSideEffects {
if boolean {
return p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.Yes)
} else {
return p.lateConstantFoldUnaryOrBinaryOrIfExpr(e.No)
}
}

// Don't mutate the original AST
expr.Data = &js_ast.EIf{Test: test, Yes: e.Yes, No: e.No}
}
}

return expr
Expand Down Expand Up @@ -1969,7 +1986,7 @@ const (
isDeleteTarget
isCallTargetOrTemplateTag
isPropertyAccessTarget
parentWasUnaryOrBinary
parentWasUnaryOrBinaryOrIfTest
)

func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFlags) {
Expand All @@ -1983,10 +2000,10 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
// This sets a flag to avoid doing this when the parent is a unary or binary
// operator so that we don't trigger O(n^2) behavior when traversing over a
// large expression tree.
if p.options.MinifySyntax && (flags&parentWasUnaryOrBinary) == 0 {
if p.options.MinifySyntax && (flags&parentWasUnaryOrBinaryOrIfTest) == 0 {
switch expr.Data.(type) {
case *js_ast.EUnary, *js_ast.EBinary:
expr = p.lateConstantFoldUnaryOrBinaryExpr(expr)
case *js_ast.EUnary, *js_ast.EBinary, *js_ast.EIf:
expr = p.lateConstantFoldUnaryOrBinaryOrIfExpr(expr)
}
}

Expand Down Expand Up @@ -2655,7 +2672,7 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
p.print("(")
flags &= ^forbidIn
}
p.printExpr(e.Test, js_ast.LConditional, flags&forbidIn)
p.printExpr(e.Test, js_ast.LConditional, (flags&forbidIn)|parentWasUnaryOrBinaryOrIfTest)
p.printSpace()
p.print("?")
if p.options.LineLimit <= 0 || !p.printNewlinePastLineLimit() {
Expand Down Expand Up @@ -3141,7 +3158,7 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
}

if !e.Op.IsPrefix() {
p.printExpr(e.Value, js_ast.LPostfix-1, parentWasUnaryOrBinary)
p.printExpr(e.Value, js_ast.LPostfix-1, parentWasUnaryOrBinaryOrIfTest)
}

if entry.IsKeyword {
Expand All @@ -3162,7 +3179,7 @@ func (p *printer) printExpr(expr js_ast.Expr, level js_ast.L, flags printExprFla
}

if e.Op.IsPrefix() {
valueFlags := parentWasUnaryOrBinary
valueFlags := parentWasUnaryOrBinaryOrIfTest
if e.Op == js_ast.UnOpDelete {
valueFlags |= isDeleteTarget
}
Expand Down Expand Up @@ -3352,9 +3369,9 @@ func (v *binaryExprVisitor) checkAndPrepare(p *printer) bool {

if e.Op == js_ast.BinOpComma {
// The result of the left operand of the comma operator is unused
v.leftFlags = (v.flags & forbidIn) | exprResultIsUnused | parentWasUnaryOrBinary
v.leftFlags = (v.flags & forbidIn) | exprResultIsUnused | parentWasUnaryOrBinaryOrIfTest
} else {
v.leftFlags = (v.flags & forbidIn) | parentWasUnaryOrBinary
v.leftFlags = (v.flags & forbidIn) | parentWasUnaryOrBinaryOrIfTest
}
return true
}
Expand Down Expand Up @@ -3382,9 +3399,9 @@ func (v *binaryExprVisitor) visitRightAndFinish(p *printer) {

if e.Op == js_ast.BinOpComma {
// The result of the right operand of the comma operator is unused if the caller doesn't use it
p.printExpr(e.Right, v.rightLevel, (v.flags&(forbidIn|exprResultIsUnused))|parentWasUnaryOrBinary)
p.printExpr(e.Right, v.rightLevel, (v.flags&(forbidIn|exprResultIsUnused))|parentWasUnaryOrBinaryOrIfTest)
} else {
p.printExpr(e.Right, v.rightLevel, (v.flags&forbidIn)|parentWasUnaryOrBinary)
p.printExpr(e.Right, v.rightLevel, (v.flags&forbidIn)|parentWasUnaryOrBinaryOrIfTest)
}

if v.wrap {
Expand Down

0 comments on commit 9e2f304

Please sign in to comment.