Handle optional control flow in labeled statements, remove unused lab…

…els (#3170)

* Fix conditional break in labeled statement handling

* Track and remove unused labels

* Rename breakFlow -> brokenFlow

* Improve coverage

src/ast/ExecutionContext.ts

@@ -9,18 +9,19 @@ interface ExecutionContextIgnore {
 	returnAwaitYield: boolean;
 }
 
-export const BREAKFLOW_NONE: false = false;
-export const BREAKFLOW_ERROR_RETURN: true = true;
-
-export type BreakFlow = typeof BREAKFLOW_NONE | typeof BREAKFLOW_ERROR_RETURN | Set<string | null>;
+export const BROKEN_FLOW_NONE = 0;
+export const BROKEN_FLOW_BREAK_CONTINUE = 1;
+export const BROKEN_FLOW_ERROR_RETURN_LABEL = 2;
 
 export interface InclusionContext {
-	breakFlow: BreakFlow;
+	brokenFlow: number;
+	includedLabels: Set<string>;
 }
 
 export interface HasEffectsContext extends InclusionContext {
 	accessed: PathTracker;
 	assigned: PathTracker;
+	brokenFlow: number;
 	called: PathTracker;
 	ignore: ExecutionContextIgnore;
 	instantiated: PathTracker;
@@ -29,22 +30,24 @@ export interface HasEffectsContext extends InclusionContext {
 
 export function createInclusionContext(): InclusionContext {
 	return {
-		breakFlow: BREAKFLOW_NONE
+		brokenFlow: BROKEN_FLOW_NONE,
+		includedLabels: new Set()
 	};
 }
 
 export function createHasEffectsContext(): HasEffectsContext {
 	return {
 		accessed: new PathTracker(),
 		assigned: new PathTracker(),
-		breakFlow: BREAKFLOW_NONE,
+		brokenFlow: BROKEN_FLOW_NONE,
 		called: new PathTracker(),
 		ignore: {
 			breaks: false,
 			continues: false,
 			labels: new Set(),
 			returnAwaitYield: false
 		},
+		includedLabels: new Set(),
 		instantiated: new PathTracker(),
 		replacedVariableInits: new Map()
 	};

src/ast/nodes/ArrowFunctionExpression.ts

@@ -1,5 +1,5 @@
 import { CallOptions } from '../CallOptions';
-import { BREAKFLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import ReturnValueScope from '../scopes/ReturnValueScope';
 import Scope from '../scopes/Scope';
 import { ObjectPath, UNKNOWN_PATH, UnknownKey } from '../utils/PathTracker';
@@ -56,7 +56,7 @@ export default class ArrowFunctionExpression extends NodeBase {
 		for (const param of this.params) {
 			if (param.hasEffects(context)) return true;
 		}
-		const { ignore, breakFlow } = context;
+		const { ignore, brokenFlow } = context;
 		context.ignore = {
 			breaks: false,
 			continues: false,
@@ -65,7 +65,7 @@ export default class ArrowFunctionExpression extends NodeBase {
 		};
 		if (this.body.hasEffects(context)) return true;
 		context.ignore = ignore;
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 		return false;
 	}
 
@@ -76,10 +76,10 @@ export default class ArrowFunctionExpression extends NodeBase {
 				param.include(context, includeChildrenRecursively);
 			}
 		}
-		const { breakFlow } = context;
-		context.breakFlow = BREAKFLOW_NONE;
+		const { brokenFlow } = context;
+		context.brokenFlow = BROKEN_FLOW_NONE;
 		this.body.include(context, includeChildrenRecursively);
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 	}
 
 	includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void {

src/ast/nodes/BlockStatement.ts

@@ -28,7 +28,7 @@ export default class BlockStatement extends StatementBase {
 	hasEffects(context: HasEffectsContext) {
 		for (const node of this.body) {
 			if (node.hasEffects(context)) return true;
-			if (context.breakFlow) break;
+			if (context.brokenFlow) break;
 		}
 		return false;
 	}

src/ast/nodes/BreakStatement.ts

@@ -1,4 +1,9 @@
-import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import {
+	BROKEN_FLOW_BREAK_CONTINUE,
+	BROKEN_FLOW_ERROR_RETURN_LABEL,
+	HasEffectsContext,
+	InclusionContext
+} from '../ExecutionContext';
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import { StatementBase } from './shared/Node';
@@ -8,15 +13,23 @@ export default class BreakStatement extends StatementBase {
 	type!: NodeType.tBreakStatement;
 
 	hasEffects(context: HasEffectsContext) {
-		if (!(this.label ? context.ignore.labels.has(this.label.name) : context.ignore.breaks))
-			return true;
-		context.breakFlow = new Set([this.label && this.label.name]);
+		if (this.label) {
+			if (!context.ignore.labels.has(this.label.name)) return true;
+			context.includedLabels.add(this.label.name);
+			context.brokenFlow = BROKEN_FLOW_ERROR_RETURN_LABEL;
+		} else {
+			if (!context.ignore.breaks) return true;
+			context.brokenFlow = BROKEN_FLOW_BREAK_CONTINUE;
+		}
 		return false;
 	}
 
 	include(context: InclusionContext) {
 		this.included = true;
-		if (this.label) this.label.include(context);
-		context.breakFlow = new Set([this.label && this.label.name]);
+		if (this.label) {
+			this.label.include(context);
+			context.includedLabels.add(this.label.name);
+		}
+		context.brokenFlow = this.label ? BROKEN_FLOW_ERROR_RETURN_LABEL : BROKEN_FLOW_BREAK_CONTINUE;
 	}
 }

src/ast/nodes/ContinueStatement.ts

@@ -1,4 +1,9 @@
-import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import {
+	BROKEN_FLOW_BREAK_CONTINUE,
+	BROKEN_FLOW_ERROR_RETURN_LABEL,
+	HasEffectsContext,
+	InclusionContext
+} from '../ExecutionContext';
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import { StatementBase } from './shared/Node';
@@ -8,15 +13,23 @@ export default class ContinueStatement extends StatementBase {
 	type!: NodeType.tContinueStatement;
 
 	hasEffects(context: HasEffectsContext) {
-		if (!(this.label ? context.ignore.labels.has(this.label.name) : context.ignore.continues))
-			return true;
-		context.breakFlow = new Set([this.label && this.label.name]);
+		if (this.label) {
+			if (!context.ignore.labels.has(this.label.name)) return true;
+			context.includedLabels.add(this.label.name);
+			context.brokenFlow = BROKEN_FLOW_ERROR_RETURN_LABEL;
+		} else {
+			if (!context.ignore.continues) return true;
+			context.brokenFlow = BROKEN_FLOW_BREAK_CONTINUE;
+		}
 		return false;
 	}
 
 	include(context: InclusionContext) {
 		this.included = true;
-		if (this.label) this.label.include(context);
-		context.breakFlow = new Set([this.label && this.label.name]);
+		if (this.label) {
+			this.label.include(context);
+			context.includedLabels.add(this.label.name);
+		}
+		context.brokenFlow = this.label ? BROKEN_FLOW_ERROR_RETURN_LABEL : BROKEN_FLOW_BREAK_CONTINUE;
 	}
 }

src/ast/nodes/DoWhileStatement.ts

@@ -1,4 +1,8 @@
-import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import {
+	BROKEN_FLOW_BREAK_CONTINUE,
+	HasEffectsContext,
+	InclusionContext
+} from '../ExecutionContext';
 import * as NodeType from './NodeType';
 import { ExpressionNode, IncludeChildren, StatementBase, StatementNode } from './shared/Node';
 
@@ -10,27 +14,27 @@ export default class DoWhileStatement extends StatementBase {
 	hasEffects(context: HasEffectsContext): boolean {
 		if (this.test.hasEffects(context)) return true;
 		const {
-			breakFlow,
+			brokenFlow,
 			ignore: { breaks, continues }
 		} = context;
 		context.ignore.breaks = true;
 		context.ignore.continues = true;
 		if (this.body.hasEffects(context)) return true;
 		context.ignore.breaks = breaks;
 		context.ignore.continues = continues;
-		if (context.breakFlow instanceof Set && context.breakFlow.has(null)) {
-			context.breakFlow = breakFlow;
+		if (context.brokenFlow === BROKEN_FLOW_BREAK_CONTINUE) {
+			context.brokenFlow = brokenFlow;
 		}
 		return false;
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
 		this.included = true;
 		this.test.include(context, includeChildrenRecursively);
-		const { breakFlow } = context;
+		const { brokenFlow } = context;
 		this.body.include(context, includeChildrenRecursively);
-		if (context.breakFlow instanceof Set && context.breakFlow.has(null)) {
-			context.breakFlow = breakFlow;
+		if (context.brokenFlow === BROKEN_FLOW_BREAK_CONTINUE) {
+			context.brokenFlow = brokenFlow;
 		}
 	}
 }

src/ast/nodes/ForInStatement.ts

@@ -35,15 +35,15 @@ export default class ForInStatement extends StatementBase {
 		)
 			return true;
 		const {
-			breakFlow,
+			brokenFlow,
 			ignore: { breaks, continues }
 		} = context;
 		context.ignore.breaks = true;
 		context.ignore.continues = true;
 		if (this.body.hasEffects(context)) return true;
 		context.ignore.breaks = breaks;
 		context.ignore.continues = continues;
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 		return false;
 	}
 
@@ -52,9 +52,9 @@ export default class ForInStatement extends StatementBase {
 		this.left.includeWithAllDeclaredVariables(includeChildrenRecursively, context);
 		this.left.deoptimizePath(EMPTY_PATH);
 		this.right.include(context, includeChildrenRecursively);
-		const { breakFlow } = context;
+		const { brokenFlow } = context;
 		this.body.include(context, includeChildrenRecursively);
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 	}
 
 	render(code: MagicString, options: RenderOptions) {

src/ast/nodes/ForOfStatement.ts

@@ -37,9 +37,9 @@ export default class ForOfStatement extends StatementBase {
 		this.left.includeWithAllDeclaredVariables(includeChildrenRecursively, context);
 		this.left.deoptimizePath(EMPTY_PATH);
 		this.right.include(context, includeChildrenRecursively);
-		const { breakFlow } = context;
+		const { brokenFlow } = context;
 		this.body.include(context, includeChildrenRecursively);
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 	}
 
 	render(code: MagicString, options: RenderOptions) {

src/ast/nodes/ForStatement.ts

@@ -26,26 +26,26 @@ export default class ForStatement extends StatementBase {
 		)
 			return true;
 		const {
-			breakFlow,
+			brokenFlow,
 			ignore: { breaks, continues }
 		} = context;
 		context.ignore.breaks = true;
 		context.ignore.continues = true;
 		if (this.body.hasEffects(context)) return true;
 		context.ignore.breaks = breaks;
 		context.ignore.continues = continues;
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 		return false;
 	}
 
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
 		this.included = true;
 		if (this.init) this.init.include(context, includeChildrenRecursively);
 		if (this.test) this.test.include(context, includeChildrenRecursively);
-		const { breakFlow } = context;
+		const { brokenFlow } = context;
 		if (this.update) this.update.include(context, includeChildrenRecursively);
 		this.body.include(context, includeChildrenRecursively);
-		context.breakFlow = breakFlow;
+		context.brokenFlow = brokenFlow;
 	}
 
 	render(code: MagicString, options: RenderOptions) {

src/ast/nodes/IfStatement.ts

@@ -2,12 +2,7 @@ import MagicString from 'magic-string';
 import { RenderOptions } from '../../utils/renderHelpers';
 import { removeAnnotations } from '../../utils/treeshakeNode';
 import { DeoptimizableEntity } from '../DeoptimizableEntity';
-import {
-	BreakFlow,
-	BREAKFLOW_NONE,
-	HasEffectsContext,
-	InclusionContext
-} from '../ExecutionContext';
+import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import { EMPTY_IMMUTABLE_TRACKER, EMPTY_PATH } from '../utils/PathTracker';
 import { LiteralValueOrUnknown, UnknownValue } from '../values';
 import * as NodeType from './NodeType';
@@ -33,13 +28,14 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 	hasEffects(context: HasEffectsContext): boolean {
 		if (this.test.hasEffects(context)) return true;
 		if (this.testValue === UnknownValue) {
-			const { breakFlow } = context;
+			const { brokenFlow } = context;
 			if (this.consequent.hasEffects(context)) return true;
-			const consequentBreakFlow = context.breakFlow;
-			context.breakFlow = breakFlow;
+			const consequentBrokenFlow = context.brokenFlow;
+			context.brokenFlow = brokenFlow;
 			if (this.alternate === null) return false;
 			if (this.alternate.hasEffects(context)) return true;
-			this.updateBreakFlowUnknownCondition(consequentBreakFlow, context);
+			context.brokenFlow =
+				context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow;
 			return false;
 		}
 		return this.testValue
@@ -119,33 +115,17 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 
 	private includeUnknownTest(context: InclusionContext) {
 		this.test.include(context, false);
-		const { breakFlow } = context;
-		let consequentBreakFlow: BreakFlow | false = false;
+		const { brokenFlow } = context;
+		let consequentBrokenFlow = BROKEN_FLOW_NONE;
 		if (this.consequent.shouldBeIncluded(context)) {
 			this.consequent.include(context, false);
-			consequentBreakFlow = context.breakFlow;
-			context.breakFlow = breakFlow;
+			consequentBrokenFlow = context.brokenFlow;
+			context.brokenFlow = brokenFlow;
 		}
 		if (this.alternate !== null && this.alternate.shouldBeIncluded(context)) {
 			this.alternate.include(context, false);
-			this.updateBreakFlowUnknownCondition(consequentBreakFlow, context);
-		}
-	}
-
-	private updateBreakFlowUnknownCondition(
-		consequentBreakFlow: BreakFlow | false,
-		context: InclusionContext
-	) {
-		if (!(consequentBreakFlow && context.breakFlow)) {
-			context.breakFlow = BREAKFLOW_NONE;
-		} else if (context.breakFlow instanceof Set) {
-			if (consequentBreakFlow instanceof Set) {
-				for (const label of consequentBreakFlow) {
-					context.breakFlow.add(label);
-				}
-			}
-		} else {
-			context.breakFlow = consequentBreakFlow;
+			context.brokenFlow =
+				context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow;
 		}
 	}
 }