fix #580: improve DCE after unconditional jumps

CHANGELOG.md

@@ -14,6 +14,17 @@
 
     In that specific case, the transformed code could crash when run because the class name is not yet initialized when the static field initializer is run. Only JavaScript code was affected. TypeScript code was not affected. This release fixes this bug.
 
+* Propagate control flow liveness to subsequent statements ([#580](https://github.com/evanw/esbuild/issues/580))
+
+    This change improves dead-code elimination in the case where unused statements follow an unconditional jump, such as a `return`:
+
+    ```js
+    if (true) return
+    if (something) thisIsDeadCode()
+    ```
+
+    These unused statements are removed in more cases than in the previous release. Some statements may still be kept that contain hoisted symbols (`var` and `function` statements) because they could potentially impact the code before the conditional jump.
+
 ## 0.8.19
 
 * Handle non-ambiguous multi-path re-exports ([#568](https://github.com/evanw/esbuild/pull/568))

internal/bundler/bundler_dce_test.go

@@ -1021,3 +1021,140 @@ func TestDisableTreeShaking(t *testing.T) {
 		},
 	})
 }
+
+func TestDeadCodeFollowingJump(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				function testReturn() {
+					if (true) return y + z()
+					if (FAIL) return FAIL
+					if (x) { var y }
+					function z() {}
+					return FAIL
+				}
+
+				function testThrow() {
+					if (true) throw y + z()
+					if (FAIL) return FAIL
+					if (x) { var y }
+					function z() {}
+					return FAIL
+				}
+
+				function testBreak() {
+					while (true) {
+						if (true) {
+							y + z()
+							break
+						}
+						if (FAIL) return FAIL
+						if (x) { var y }
+						function z() {}
+						return FAIL
+					}
+				}
+
+				function testContinue() {
+					while (true) {
+						if (true) {
+							y + z()
+							continue
+						}
+						if (FAIL) return FAIL
+						if (x) { var y }
+						function z() {}
+						return FAIL
+					}
+				}
+
+				function testStmts() {
+					return [a, b, c, d, e, f, g, h, i]
+
+					while (x) { var a }
+					while (FAIL) { let FAIL }
+
+					do { var b } while (x)
+					do { let FAIL } while (FAIL)
+
+					for (var c; ;) ;
+					for (let FAIL; ;) ;
+
+					for (var d in x) ;
+					for (let FAIL in FAIL) ;
+
+					for (var e of x) ;
+					for (let FAIL of FAIL) ;
+
+					if (x) { var f }
+					if (FAIL) { let FAIL }
+
+					if (x) ; else { var g }
+					if (FAIL) ; else { let FAIL }
+
+					{ var h }
+					{ let FAIL }
+
+					x: { var i }
+					x: { let FAIL }
+				}
+
+				testReturn()
+				testThrow()
+				testBreak()
+				testContinue()
+				testStmts()
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+			MangleSyntax:  true,
+		},
+	})
+}
+
+func TestRemoveTrailingReturn(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				function foo() {
+					if (a) b()
+					return
+				}
+				function bar() {
+					if (a) b()
+					return KEEP_ME
+				}
+				export default [
+					foo,
+					bar,
+					function () {
+						if (a) b()
+						return
+					},
+					function () {
+						if (a) b()
+						return KEEP_ME
+					},
+					() => {
+						if (a) b()
+						return
+					},
+					() => {
+						if (a) b()
+						return KEEP_ME
+					},
+				]
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+			MangleSyntax:  true,
+			OutputFormat:  config.FormatESModule,
+		},
+	})
+}

internal/bundler/snapshots/snapshots_dce.txt

@@ -9,6 +9,71 @@ TestDataURLLoaderRemoveUnused
 // entry.js
 console.log("unused import");
 
+================================================================================
+TestDeadCodeFollowingJump
+---------- /out.js ----------
+// entry.js
+function testReturn() {
+  return y + z();
+  if (x)
+    var y;
+  function z() {
+  }
+}
+function testThrow() {
+  throw y + z();
+  if (x)
+    var y;
+  function z() {
+  }
+}
+function testBreak() {
+  for (; ; ) {
+    y + z();
+    break;
+    if (x)
+      var y;
+    function z() {
+    }
+  }
+}
+function testContinue() {
+  for (; ; ) {
+    y + z();
+    continue;
+    if (x)
+      var y;
+    function z() {
+    }
+  }
+}
+function testStmts() {
+  return [a, b, c, d, e, f, g, h, i];
+  for (; x; )
+    var a;
+  do
+    var b;
+  while (x);
+  for (var c; ; )
+    ;
+  for (var d in x)
+    ;
+  for (var e of x)
+    ;
+  if (x)
+    var f;
+  if (!x)
+    var g;
+  var h;
+  x:
+    var i;
+}
+testReturn();
+testThrow();
+testBreak();
+testContinue();
+testStmts();
+
 ================================================================================
 TestDisableTreeShaking
 ---------- /out.js ----------
@@ -295,6 +360,34 @@ console.log("hello");
 // Users/user/project/src/entry.js
 console.log("unused import");
 
+================================================================================
+TestRemoveTrailingReturn
+---------- /out.js ----------
+// entry.js
+function foo() {
+  a && b();
+}
+function bar() {
+  return a && b(), KEEP_ME;
+}
+var entry_default = [
+  foo,
+  bar,
+  function() {
+    a && b();
+  },
+  function() {
+    return a && b(), KEEP_ME;
+  },
+  () => {
+    a && b();
+  },
+  () => (a && b(), KEEP_ME)
+];
+export {
+  entry_default as default
+};
+
 ================================================================================
 TestRemoveUnusedImportMeta
 ---------- /out.js ----------

internal/js_parser/js_parser.go

@@ -6022,6 +6022,27 @@ func shouldKeepStmtInDeadControlFlow(stmt js_ast.Stmt) bool {
 		s.Decls = identifiers
 		return true
 
+	case *js_ast.SIf:
+		return shouldKeepStmtInDeadControlFlow(s.Yes) || (s.No != nil && shouldKeepStmtInDeadControlFlow(*s.No))
+
+	case *js_ast.SWhile:
+		return shouldKeepStmtInDeadControlFlow(s.Body)
+
+	case *js_ast.SDoWhile:
+		return shouldKeepStmtInDeadControlFlow(s.Body)
+
+	case *js_ast.SFor:
+		return (s.Init != nil && shouldKeepStmtInDeadControlFlow(*s.Init)) || shouldKeepStmtInDeadControlFlow(s.Body)
+
+	case *js_ast.SForIn:
+		return shouldKeepStmtInDeadControlFlow(s.Init) || shouldKeepStmtInDeadControlFlow(s.Body)
+
+	case *js_ast.SForOf:
+		return shouldKeepStmtInDeadControlFlow(s.Init) || shouldKeepStmtInDeadControlFlow(s.Body)
+
+	case *js_ast.SLabel:
+		return shouldKeepStmtInDeadControlFlow(s.Stmt)
+
 	default:
 		// Everything else must be kept
 		return true
@@ -6084,21 +6105,46 @@ func (p *parser) visitStmtsAndPrependTempRefs(stmts []js_ast.Stmt, opts prependT
 }
 
 func (p *parser) visitStmts(stmts []js_ast.Stmt) []js_ast.Stmt {
+	// Save the current control-flow liveness. This represents if we are
+	// currently inside an "if (false) { ... }" block.
+	oldIsControlFlowDead := p.isControlFlowDead
+
 	// Visit all statements first
 	visited := make([]js_ast.Stmt, 0, len(stmts))
 	var after []js_ast.Stmt
 	for _, stmt := range stmts {
+		var target *[]js_ast.Stmt
 		if _, ok := stmt.Data.(*js_ast.SExportEquals); ok {
 			// TypeScript "export = value;" becomes "module.exports = value;". This
 			// must happen at the end after everything is parsed because TypeScript
 			// moves this statement to the end when it generates code.
-			after = p.visitAndAppendStmt(after, stmt)
+			target = &after
 		} else {
-			visited = p.visitAndAppendStmt(visited, stmt)
+			target = &visited
+		}
+
+		// This call should only append new statements but never modify old ones
+		nextStmtIndex := len(*target)
+		*target = p.visitAndAppendStmt(*target, stmt)
+
+		// If there is a jump-like statement in the middle of this statement
+		// sequence, control flow is dead for all of the following statements.
+		// This improves dead-code elimination.
+		if p.options.mangleSyntax {
+			for _, stmt := range (*target)[nextStmtIndex:] {
+				if isJumpStatement(stmt.Data) {
+					p.isControlFlowDead = true
+					break
+				}
+			}
 		}
 	}
 	visited = append(visited, after...)
 
+	// Restore the current control-flow liveness if it was changed inside the
+	// loop above. This is important because the caller will not restore it.
+	p.isControlFlowDead = oldIsControlFlowDead
+
 	// Stop now if we're not mangling
 	if !p.options.mangleSyntax {
 		return visited
@@ -6213,6 +6259,9 @@ func (p *parser) visitStmts(stmts []js_ast.Stmt) []js_ast.Stmt {
 
 			isControlFlowDead = true
 
+		case *js_ast.SBreak, *js_ast.SContinue:
+			isControlFlowDead = true
+
 		case *js_ast.SFor:
 			if len(result) > 0 {
 				prevStmt := result[len(result)-1]

internal/js_parser/js_parser_lower.go

@@ -154,6 +154,16 @@ func (p *parser) lowerFunction(
 	hasRestArg *bool,
 	isArrow bool,
 ) {
+	// Size optimization: trim an implicit return from the end of the function body
+	if p.options.mangleSyntax {
+		if n := len(*bodyStmts); n > 0 {
+			last := (*bodyStmts)[n-1]
+			if s, ok := last.Data.(*js_ast.SReturn); ok && s.Value == nil {
+				*bodyStmts = (*bodyStmts)[:n-1]
+			}
+		}
+	}
+
 	// Lower object rest binding patterns in function arguments
 	if p.options.unsupportedJSFeatures.Has(compat.ObjectRestSpread) {
 		var prefixStmts []js_ast.Stmt