[WIP] handle assignments in dead code (#3212)

* wip handle assignments in dead code

* Test skipped code does not cause deoptimizations

* Add tests and slightly adapt logic

* Some mini-optimizations

Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>

src/ast/nodes/AssignmentExpression.ts

@@ -1,11 +1,11 @@
 import MagicString from 'magic-string';
 import { findFirstOccurrenceOutsideComment, RenderOptions } from '../../utils/renderHelpers';
 import { getSystemExportStatement } from '../../utils/systemJsRendering';
-import { HasEffectsContext } from '../ExecutionContext';
+import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import { EMPTY_PATH, ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
 import Variable from '../variables/Variable';
 import * as NodeType from './NodeType';
-import { ExpressionNode, NodeBase } from './shared/Node';
+import { ExpressionNode, IncludeChildren, NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
 
 export default class AssignmentExpression extends NodeBase {
@@ -26,15 +26,10 @@ export default class AssignmentExpression extends NodeBase {
 		| '**=';
 	right!: ExpressionNode;
 	type!: NodeType.tAssignmentExpression;
-
-	bind() {
-		super.bind();
-		this.left.deoptimizePath(EMPTY_PATH);
-		// We cannot propagate mutations of the new binding to the old binding with certainty
-		this.right.deoptimizePath(UNKNOWN_PATH);
-	}
+	private deoptimized = false;
 
 	hasEffects(context: HasEffectsContext): boolean {
+		if (!this.deoptimized) this.applyDeoptimizations();
 		return (
 			this.right.hasEffects(context) ||
 			this.left.hasEffects(context) ||
@@ -46,6 +41,13 @@ export default class AssignmentExpression extends NodeBase {
 		return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context);
 	}
 
+	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
+		if (!this.deoptimized) this.applyDeoptimizations();
+		this.included = true;
+		this.left.include(context, includeChildrenRecursively);
+		this.right.include(context, includeChildrenRecursively);
+	}
+
 	render(code: MagicString, options: RenderOptions) {
 		this.left.render(code, options);
 		this.right.render(code, options);
@@ -79,4 +81,10 @@ export default class AssignmentExpression extends NodeBase {
 			}
 		}
 	}
+
+	private applyDeoptimizations() {
+		this.deoptimized = true;
+		this.left.deoptimizePath(EMPTY_PATH);
+		this.right.deoptimizePath(UNKNOWN_PATH);
+	}
 }

src/ast/nodes/IfStatement.ts

@@ -8,27 +8,26 @@ import { LiteralValueOrUnknown, UnknownValue } from '../values';
 import * as NodeType from './NodeType';
 import { ExpressionNode, IncludeChildren, StatementBase, StatementNode } from './shared/Node';
 
+const unset = Symbol('unset');
+
 export default class IfStatement extends StatementBase implements DeoptimizableEntity {
 	alternate!: StatementNode | null;
 	consequent!: StatementNode;
 	test!: ExpressionNode;
 	type!: NodeType.tIfStatement;
 
-	private testValue: LiteralValueOrUnknown;
-
-	bind() {
-		super.bind();
-		// ensure the testValue is set for the tree-shaking passes
-		this.testValue = this.test.getLiteralValueAtPath(EMPTY_PATH, SHARED_RECURSION_TRACKER, this);
-	}
+	private testValue: LiteralValueOrUnknown | typeof unset = unset;
 
 	deoptimizeCache() {
 		this.testValue = UnknownValue;
 	}
 
 	hasEffects(context: HasEffectsContext): boolean {
-		if (this.test.hasEffects(context)) return true;
-		if (this.testValue === UnknownValue) {
+		if (this.test.hasEffects(context)) {
+			return true;
+		}
+		const testValue = this.getTestValue();
+		if (testValue === UnknownValue) {
 			const { brokenFlow } = context;
 			if (this.consequent.hasEffects(context)) return true;
 			const consequentBrokenFlow = context.brokenFlow;
@@ -39,7 +38,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 				context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow;
 			return false;
 		}
-		return this.testValue
+		return testValue
 			? this.consequent.hasEffects(context)
 			: this.alternate !== null && this.alternate.hasEffects(context);
 	}
@@ -48,22 +47,24 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		this.included = true;
 		if (includeChildrenRecursively) {
 			this.includeRecursively(includeChildrenRecursively, context);
-		} else if (this.testValue === UnknownValue) {
-			this.includeUnknownTest(context);
 		} else {
-			this.includeKnownTest(context);
+			const testValue = this.getTestValue();
+			if (testValue === UnknownValue) {
+				this.includeUnknownTest(context);
+			} else {
+				this.includeKnownTest(context, testValue);
+			}
 		}
 	}
 
 	render(code: MagicString, options: RenderOptions) {
 		// Note that unknown test values are always included
+		const testValue = this.getTestValue();
 		if (
 			!this.test.included &&
-			(this.testValue
-				? this.alternate === null || !this.alternate.included
-				: !this.consequent.included)
+			(testValue ? this.alternate === null || !this.alternate.included : !this.consequent.included)
 		) {
-			const singleRetainedBranch = (this.testValue ? this.consequent : this.alternate)!;
+			const singleRetainedBranch = (testValue ? this.consequent : this.alternate)!;
 			code.remove(this.start, singleRetainedBranch.start);
 			code.remove(singleRetainedBranch.end, this.end);
 			removeAnnotations(this, code);
@@ -72,7 +73,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 			if (this.test.included) {
 				this.test.render(code, options);
 			} else {
-				code.overwrite(this.test.start, this.test.end, this.testValue ? 'true' : 'false');
+				code.overwrite(this.test.start, this.test.end, testValue ? 'true' : 'false');
 			}
 			if (this.consequent.included) {
 				this.consequent.render(code, options);
@@ -89,14 +90,25 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		}
 	}
 
-	private includeKnownTest(context: InclusionContext) {
+	private getTestValue(): LiteralValueOrUnknown {
+		if (this.testValue === unset) {
+			return (this.testValue = this.test.getLiteralValueAtPath(
+				EMPTY_PATH,
+				SHARED_RECURSION_TRACKER,
+				this
+			));
+		}
+		return this.testValue;
+	}
+
+	private includeKnownTest(context: InclusionContext, testValue: LiteralValueOrUnknown) {
 		if (this.test.shouldBeIncluded(context)) {
 			this.test.include(context, false);
 		}
-		if (this.testValue && this.consequent.shouldBeIncluded(context)) {
+		if (testValue && this.consequent.shouldBeIncluded(context)) {
 			this.consequent.include(context, false);
 		}
-		if (this.alternate !== null && !this.testValue && this.alternate.shouldBeIncluded(context)) {
+		if (this.alternate !== null && !testValue && this.alternate.shouldBeIncluded(context)) {
 			this.alternate.include(context, false);
 		}
 	}

test/form/samples/lazy-assignment-deoptimization/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'only deoptimizes assigned variables when the assignment is included'
+};

test/form/samples/lazy-assignment-deoptimization/_expected.js

@@ -0,0 +1,3 @@
+console.log('retained');
+
+console.log('retained');

test/form/samples/lazy-assignment-deoptimization/main.js

@@ -0,0 +1,12 @@
+const foo = { toggled: false };
+const bar = { toggled: false };
+
+if (foo.toggled) {
+	foo.toggled = bar;
+}
+
+if (foo.toggled) console.log('this should be removed');
+else console.log('retained');
+
+if (bar.toggled) console.log('this should be removed');
+else console.log('retained');

test/function/samples/assignment-deoptimization/assignee-is-deoptimized/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'makes sure the assignee is deoptimized'
+};

test/function/samples/assignment-deoptimization/assignee-is-deoptimized/main.js

@@ -0,0 +1,5 @@
+const flags = { updated: false };
+let toBeUpdated = {};
+toBeUpdated = flags;
+toBeUpdated.updated = true;
+if (!flags.updated) throw new Error('Update was not tracked');

test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'makes sure the assignment target is deoptimized'
+};

test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/main.js

@@ -0,0 +1,3 @@
+let updated = false;
+updated = true;
+if (!updated) throw new Error('Update was not tracked');

test/function/samples/assignment-deoptimization/nested-assignment/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'tracks assigments nested in expressions that are included for other reasons'
+};

test/function/samples/assignment-deoptimization/nested-assignment/main.js

@@ -0,0 +1,3 @@
+let updated = false;
+assert.ok(!updated) || (updated = true);
+if (!updated) throw new Error('Update was not tracked');

test/function/samples/assignment-deoptimization/try-catch-inclusion/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'tracks assigments included via try-catch-deoptimization'
+};

test/function/samples/assignment-deoptimization/try-catch-inclusion/main.js

@@ -0,0 +1,6 @@
+let updated = false;
+try {
+	updated = true;
+} catch (err) {}
+
+if (!updated) throw new Error('Update was not tracked');