Deoptimize parameter defaults when referenced from object/array/class literals (#4518)

lukastaegert · web-flow · commit d25b6e270bc0 · 2022-05-31T13:18:08.000+02:00
diff --git a/src/ast/nodes/ArrayExpression.ts b/src/ast/nodes/ArrayExpression.ts
@@ -19,7 +19,6 @@ import { ObjectEntity, type ObjectProperty } from './shared/ObjectEntity';
 export default class ArrayExpression extends NodeBase {
 	declare elements: readonly (ExpressionNode | SpreadElement | null)[];
 	declare type: NodeType.tArrayExpression;
-	protected deoptimized = false;
 	private objectEntity: ObjectEntity | null = null;
 
 	deoptimizePath(path: ObjectPath): void {
@@ -83,10 +82,14 @@ export default class ArrayExpression extends NodeBase {
 		let hasSpread = false;
 		for (let index = 0; index < this.elements.length; index++) {
 			const element = this.elements[index];
-			if (hasSpread || element instanceof SpreadElement) {
-				if (element) {
+			if (element) {
+				if (hasSpread || element instanceof SpreadElement) {
 					hasSpread = true;
+					// This also deoptimizes parameter defaults
 					element.deoptimizePath(UNKNOWN_PATH);
+				} else {
+					// We do not track parameter defaults in arrays
+					element.deoptimizeCallParameters();
 				}
 			}
 		}
diff --git a/src/ast/nodes/AssignmentExpression.ts b/src/ast/nodes/AssignmentExpression.ts
@@ -43,7 +43,6 @@ export default class AssignmentExpression extends NodeBase {
 		| '**=';
 	declare right: ExpressionNode;
 	declare type: NodeType.tAssignmentExpression;
-	protected deoptimized = false;
 
 	hasEffects(context: HasEffectsContext): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
diff --git a/src/ast/nodes/AssignmentPattern.ts b/src/ast/nodes/AssignmentPattern.ts
@@ -15,7 +15,6 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 	declare left: PatternNode;
 	declare right: ExpressionNode;
 	declare type: NodeType.tAssignmentPattern;
-	protected deoptimized = false;
 
 	addExportedVariables(
 		variables: readonly Variable[],
diff --git a/src/ast/nodes/AwaitExpression.ts b/src/ast/nodes/AwaitExpression.ts
@@ -7,7 +7,6 @@ import { type ExpressionNode, type IncludeChildren, type Node, NodeBase } from '
 export default class AwaitExpression extends NodeBase {
 	declare argument: ExpressionNode;
 	declare type: NodeType.tAwaitExpression;
-	protected deoptimized = false;
 
 	hasEffects(): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
diff --git a/src/ast/nodes/ClassBody.ts b/src/ast/nodes/ClassBody.ts
@@ -37,4 +37,6 @@ export default class ClassBody extends NodeBase {
 		}
 		super.parseNode(esTreeNode);
 	}
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/ExportAllDeclaration.ts b/src/ast/nodes/ExportAllDeclaration.ts
@@ -22,6 +22,8 @@ export default class ExportAllDeclaration extends NodeBase {
 	render(code: MagicString, _options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void {
 		code.remove(nodeRenderOptions!.start!, nodeRenderOptions!.end!);
 	}
+
+	protected applyDeoptimizations() {}
 }
 
 ExportAllDeclaration.prototype.needsBoundaries = true;
diff --git a/src/ast/nodes/ExportDefaultDeclaration.ts b/src/ast/nodes/ExportDefaultDeclaration.ts
@@ -109,6 +109,8 @@ export default class ExportDefaultDeclaration extends NodeBase {
 		this.declaration.render(code, options);
 	}
 
+	protected applyDeoptimizations() {}
+
 	private renderNamedDeclaration(
 		code: MagicString,
 		declarationStart: number,
diff --git a/src/ast/nodes/ExportNamedDeclaration.ts b/src/ast/nodes/ExportNamedDeclaration.ts
@@ -38,6 +38,8 @@ export default class ExportNamedDeclaration extends NodeBase {
 			(this.declaration as Node).render(code, options, { end, start });
 		}
 	}
+
+	protected applyDeoptimizations() {}
 }
 
 ExportNamedDeclaration.prototype.needsBoundaries = true;
diff --git a/src/ast/nodes/ExportSpecifier.ts b/src/ast/nodes/ExportSpecifier.ts
@@ -6,4 +6,6 @@ export default class ExportSpecifier extends NodeBase {
 	declare exported: Identifier;
 	declare local: Identifier;
 	declare type: NodeType.tExportSpecifier;
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/ExpressionStatement.ts b/src/ast/nodes/ExpressionStatement.ts
@@ -36,4 +36,6 @@ export default class ExpressionStatement extends StatementBase {
 
 		return super.shouldBeIncluded(context);
 	}
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/ForInStatement.ts b/src/ast/nodes/ForInStatement.ts
@@ -19,7 +19,6 @@ export default class ForInStatement extends StatementBase {
 	declare left: VariableDeclaration | PatternNode;
 	declare right: ExpressionNode;
 	declare type: NodeType.tForInStatement;
-	protected deoptimized = false;
 
 	createScope(parentScope: Scope): void {
 		this.scope = new BlockScope(parentScope);
diff --git a/src/ast/nodes/ForOfStatement.ts b/src/ast/nodes/ForOfStatement.ts
@@ -20,7 +20,6 @@ export default class ForOfStatement extends StatementBase {
 	declare left: VariableDeclaration | PatternNode;
 	declare right: ExpressionNode;
 	declare type: NodeType.tForOfStatement;
-	protected deoptimized = false;
 
 	createScope(parentScope: Scope): void {
 		this.scope = new BlockScope(parentScope);
diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -36,7 +36,6 @@ export default class Identifier extends NodeBase implements PatternNode {
 	declare name: string;
 	declare type: NodeType.tIdentifier;
 	variable: Variable | null = null;
-	protected deoptimized = false;
 	private isTDZAccess: boolean | null = null;
 
 	addExportedVariables(
@@ -87,11 +86,17 @@ export default class Identifier extends NodeBase implements PatternNode {
 		return [(this.variable = variable)];
 	}
 
+	deoptimizeCallParameters() {
+		this.variable!.deoptimizeCallParameters();
+	}
+
 	deoptimizePath(path: ObjectPath): void {
 		if (path.length === 0 && !this.scope.contains(this.name)) {
 			this.disallowImportReassignment();
 		}
-		this.variable!.deoptimizePath(path);
+		// We keep conditional chaining because an unknown Node could have an
+		// Identifier as property that might be deoptimized by default
+		this.variable?.deoptimizePath(path);
 	}
 
 	deoptimizeThisOnEventAtPath(
diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts
@@ -129,6 +129,8 @@ export default class IfStatement extends StatementBase implements DeoptimizableE
 		this.renderHoistedDeclarations(hoistedDeclarations, code, getPropertyAccess);
 	}
 
+	protected applyDeoptimizations() {}
+
 	private getTestValue(): LiteralValueOrUnknown {
 		if (this.testValue === unset) {
 			return (this.testValue = this.test.getLiteralValueAtPath(
diff --git a/src/ast/nodes/ImportDeclaration.ts b/src/ast/nodes/ImportDeclaration.ts
@@ -27,6 +27,8 @@ export default class ImportDeclaration extends NodeBase {
 	render(code: MagicString, _options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void {
 		code.remove(nodeRenderOptions!.start!, nodeRenderOptions!.end!);
 	}
+
+	protected applyDeoptimizations() {}
 }
 
 ImportDeclaration.prototype.needsBoundaries = true;
diff --git a/src/ast/nodes/ImportDefaultSpecifier.ts b/src/ast/nodes/ImportDefaultSpecifier.ts
@@ -5,4 +5,6 @@ import { NodeBase } from './shared/Node';
 export default class ImportDefaultSpecifier extends NodeBase {
 	declare local: Identifier;
 	declare type: NodeType.tImportDefaultSpecifier;
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/ImportExpression.ts b/src/ast/nodes/ImportExpression.ts
@@ -124,6 +124,8 @@ export default class ImportExpression extends NodeBase {
 		this.inlineNamespace = inlineNamespace;
 	}
 
+	protected applyDeoptimizations() {}
+
 	private getDynamicImportMechanismAndHelper(
 		resolution: Module | ExternalModule | string | null,
 		exportMode: 'none' | 'named' | 'default' | 'external',
diff --git a/src/ast/nodes/ImportNamespaceSpecifier.ts b/src/ast/nodes/ImportNamespaceSpecifier.ts
@@ -5,4 +5,6 @@ import { NodeBase } from './shared/Node';
 export default class ImportNamespaceSpecifier extends NodeBase {
 	declare local: Identifier;
 	declare type: NodeType.tImportNamespaceSpecifier;
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/ImportSpecifier.ts b/src/ast/nodes/ImportSpecifier.ts
@@ -6,4 +6,6 @@ export default class ImportSpecifier extends NodeBase {
 	declare imported: Identifier;
 	declare local: Identifier;
 	declare type: NodeType.tImportSpecifier;
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts
@@ -89,7 +89,6 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 	declare propertyKey: ObjectPathKey | null;
 	declare type: NodeType.tMemberExpression;
 	variable: Variable | null = null;
-	protected deoptimized = false;
 	private bound = false;
 	private expressionsToBeDeoptimized: DeoptimizableEntity[] = [];
 	private replacement: string | null = null;
diff --git a/src/ast/nodes/MethodDefinition.ts b/src/ast/nodes/MethodDefinition.ts
@@ -10,4 +10,6 @@ export default class MethodDefinition extends MethodBase {
 	declare static: boolean;
 	declare type: NodeType.tMethodDefinition;
 	declare value: FunctionExpression;
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/NewExpression.ts b/src/ast/nodes/NewExpression.ts
@@ -10,7 +10,6 @@ export default class NewExpression extends NodeBase {
 	declare arguments: ExpressionNode[];
 	declare callee: ExpressionNode;
 	declare type: NodeType.tNewExpression;
-	protected deoptimized = false;
 	private declare callOptions: CallOptions;
 
 	hasEffects(context: HasEffectsContext): boolean {
diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts
@@ -15,7 +15,7 @@ import {
 import Identifier from './Identifier';
 import Literal from './Literal';
 import * as NodeType from './NodeType';
-import type Property from './Property';
+import Property from './Property';
 import SpreadElement from './SpreadElement';
 import { type ExpressionEntity, type LiteralValueOrUnknown } from './shared/Expression';
 import { NodeBase } from './shared/Node';
@@ -102,6 +102,15 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE
 		}
 	}
 
+	protected applyDeoptimizations() {
+		this.deoptimized = true;
+		for (const property of this.properties) {
+			if (property instanceof Property) {
+				property.value.deoptimizeCallParameters();
+			}
+		}
+	}
+
 	private getObjectEntity(): ObjectEntity {
 		if (this.objectEntity !== null) {
 			return this.objectEntity;
diff --git a/src/ast/nodes/Program.ts b/src/ast/nodes/Program.ts
@@ -38,4 +38,6 @@ export default class Program extends NodeBase {
 			super.render(code, options);
 		}
 	}
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/Property.ts b/src/ast/nodes/Property.ts
@@ -16,7 +16,6 @@ export default class Property extends MethodBase implements PatternNode {
 	declare method: boolean;
 	declare shorthand: boolean;
 	declare type: NodeType.tProperty;
-	protected deoptimized = false;
 	private declarationInit: ExpressionEntity | null = null;
 
 	declare(kind: string, init: ExpressionEntity): LocalVariable[] {
diff --git a/src/ast/nodes/PropertyDefinition.ts b/src/ast/nodes/PropertyDefinition.ts
@@ -73,4 +73,6 @@ export default class PropertyDefinition extends NodeBase {
 	): boolean {
 		return !this.value || this.value.hasEffectsWhenCalledAtPath(path, callOptions, context);
 	}
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/RestElement.ts b/src/ast/nodes/RestElement.ts
@@ -10,7 +10,6 @@ import type { PatternNode } from './shared/Pattern';
 export default class RestElement extends NodeBase implements PatternNode {
 	declare argument: PatternNode;
 	declare type: NodeType.tRestElement;
-	protected deoptimized = false;
 	private declarationInit: ExpressionEntity | null = null;
 
 	addExportedVariables(
diff --git a/src/ast/nodes/SpreadElement.ts b/src/ast/nodes/SpreadElement.ts
@@ -9,7 +9,6 @@ import { type ExpressionNode, NodeBase } from './shared/Node';
 export default class SpreadElement extends NodeBase {
 	declare argument: ExpressionNode;
 	declare type: NodeType.tSpreadElement;
-	protected deoptimized = false;
 
 	deoptimizeThisOnEventAtPath(
 		event: NodeEvent,
diff --git a/src/ast/nodes/UnaryExpression.ts b/src/ast/nodes/UnaryExpression.ts
@@ -24,7 +24,6 @@ export default class UnaryExpression extends NodeBase {
 	declare operator: '!' | '+' | '-' | 'delete' | 'typeof' | 'void' | '~';
 	declare prefix: boolean;
 	declare type: NodeType.tUnaryExpression;
-	protected deoptimized = false;
 
 	getLiteralValueAtPath(
 		path: ObjectPath,
diff --git a/src/ast/nodes/UpdateExpression.ts b/src/ast/nodes/UpdateExpression.ts
@@ -16,7 +16,6 @@ export default class UpdateExpression extends NodeBase {
 	declare operator: '++' | '--';
 	declare prefix: boolean;
 	declare type: NodeType.tUpdateExpression;
-	protected deoptimized = false;
 
 	hasEffects(context: HasEffectsContext): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
diff --git a/src/ast/nodes/VariableDeclaration.ts b/src/ast/nodes/VariableDeclaration.ts
@@ -96,6 +96,8 @@ export default class VariableDeclaration extends NodeBase {
 		}
 	}
 
+	protected applyDeoptimizations() {}
+
 	private renderDeclarationEnd(
 		code: MagicString,
 		separatorString: string,
diff --git a/src/ast/nodes/VariableDeclarator.ts b/src/ast/nodes/VariableDeclarator.ts
@@ -69,4 +69,6 @@ export default class VariableDeclarator extends NodeBase {
 			code.appendLeft(this.end, `${_}=${_}void 0`);
 		}
 	}
+
+	protected applyDeoptimizations() {}
 }
diff --git a/src/ast/nodes/YieldExpression.ts b/src/ast/nodes/YieldExpression.ts
@@ -8,7 +8,6 @@ export default class YieldExpression extends NodeBase {
 	declare argument: ExpressionNode | null;
 	declare delegate: boolean;
 	declare type: NodeType.tYieldExpression;
-	protected deoptimized = false;
 
 	hasEffects(context: HasEffectsContext): boolean {
 		if (!this.deoptimized) this.applyDeoptimizations();
diff --git a/src/ast/nodes/shared/CallExpressionBase.ts b/src/ast/nodes/shared/CallExpressionBase.ts
@@ -13,7 +13,6 @@ import { NodeBase } from './Node';
 
 export default abstract class CallExpressionBase extends NodeBase implements DeoptimizableEntity {
 	protected declare callOptions: CallOptions;
-	protected deoptimized = false;
 	protected returnExpression: ExpressionEntity | null = null;
 	private readonly deoptimizableDependentExpressions: DeoptimizableEntity[] = [];
 	private readonly expressionsToBeDeoptimized = new Set<ExpressionEntity>();
diff --git a/src/ast/nodes/shared/ClassNode.ts b/src/ast/nodes/shared/ClassNode.ts
@@ -16,6 +16,7 @@ import type ClassBody from '../ClassBody';
 import Identifier from '../Identifier';
 import type Literal from '../Literal';
 import MethodDefinition from '../MethodDefinition';
+import PropertyDefinition from '../PropertyDefinition';
 import { type ExpressionEntity, type LiteralValueOrUnknown } from './Expression';
 import { type ExpressionNode, type IncludeChildren, NodeBase } from './Node';
 import { ObjectEntity, type ObjectProperty } from './ObjectEntity';
@@ -26,7 +27,6 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 	declare body: ClassBody;
 	declare id: Identifier | null;
 	declare superClass: ExpressionNode | null;
-	protected deoptimized = false;
 	private declare classConstructor: MethodDefinition | null;
 	private objectEntity: ObjectEntity | null = null;
 
@@ -150,8 +150,11 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 			) {
 				// Calls to methods are not tracked, ensure that the return value is deoptimized
 				definition.deoptimizePath(UNKNOWN_PATH);
+			} else if (definition instanceof PropertyDefinition) {
+				definition.value?.deoptimizeCallParameters();
 			}
 		}
+		this.superClass?.deoptimizeCallParameters();
 		this.context.requestTreeshakingPass();
 	}
 
diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts
@@ -24,6 +24,8 @@ export interface InclusionOptions {
 export class ExpressionEntity implements WritableEntity {
 	included = false;
 
+	deoptimizeCallParameters(): void {}
+
 	deoptimizePath(_path: ObjectPath): void {}
 
 	deoptimizeThisOnEventAtPath(
diff --git a/src/ast/nodes/shared/FunctionBase.ts b/src/ast/nodes/shared/FunctionBase.ts
@@ -54,6 +54,10 @@ export default abstract class FunctionBase extends NodeBase implements Deoptimiz
 		this.forceIncludeParameters = true;
 	}
 
+	deoptimizeCallParameters() {
+		this.forceIncludeParameters = true;
+	}
+
 	deoptimizePath(path: ObjectPath): void {
 		this.getObjectEntity().deoptimizePath(path);
 		if (path.length === 1 && path[0] === UnknownKey) {
diff --git a/src/ast/nodes/shared/MethodBase.ts b/src/ast/nodes/shared/MethodBase.ts
@@ -116,6 +116,8 @@ export default class MethodBase extends NodeBase implements DeoptimizableEntity
 		return this.getAccessedValue().hasEffectsWhenCalledAtPath(path, callOptions, context);
 	}
 
+	protected applyDeoptimizations() {}
+
 	protected getAccessedValue(): ExpressionEntity {
 		if (this.accessedValue === null) {
 			if (this.kind === 'get') {
diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts
diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts
diff --git a/test/function/samples/parameter-defaults/arrays/_config.js b/test/function/samples/parameter-defaults/arrays/_config.js
diff --git a/test/function/samples/parameter-defaults/arrays/main.js b/test/function/samples/parameter-defaults/arrays/main.js
diff --git a/test/function/samples/parameter-defaults/class-fields/_config.js b/test/function/samples/parameter-defaults/class-fields/_config.js
diff --git a/test/function/samples/parameter-defaults/class-fields/main.js b/test/function/samples/parameter-defaults/class-fields/main.js
diff --git a/test/function/samples/parameter-defaults/objects/_config.js b/test/function/samples/parameter-defaults/objects/_config.js
diff --git a/test/function/samples/parameter-defaults/objects/main.js b/test/function/samples/parameter-defaults/objects/main.js
diff --git a/test/function/samples/parameter-defaults/super-classes/main.js b/test/function/samples/parameter-defaults/super-classes/main.js

Original file line number	Diff line number	Diff line change
`@@ -37,4 +37,6 @@ export default class ClassBody extends NodeBase {`
`37`	`37`	`}`
`38`	`38`	`super.parseNode(esTreeNode);`
`39`	`39`	`}`
	`40`	`+`
	`41`	`+ protected applyDeoptimizations() {}`
`40`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,8 @@ export default class ExportAllDeclaration extends NodeBase {`
`22`	`22`	`render(code: MagicString, _options: RenderOptions, nodeRenderOptions?: NodeRenderOptions): void {`
`23`	`23`	`code.remove(nodeRenderOptions!.start!, nodeRenderOptions!.end!);`
`24`	`24`	`}`
	`25`	`+`
	`26`	`+ protected applyDeoptimizations() {}`
`25`	`27`	`}`
`26`	`28`
`27`	`29`	`ExportAllDeclaration.prototype.needsBoundaries = true;`
Original file line number	Diff line number	Diff line change
`@@ -109,6 +109,8 @@ export default class ExportDefaultDeclaration extends NodeBase {`
`109`	`109`	`this.declaration.render(code, options);`
`110`	`110`	`}`
`111`	`111`
	`112`	`+ protected applyDeoptimizations() {}`
	`113`	`+`
`112`	`114`	`private renderNamedDeclaration(`
`113`	`115`	`code: MagicString,`
`114`	`116`	`declarationStart: number,`
Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,8 @@ export default class ExportNamedDeclaration extends NodeBase {`
`38`	`38`	`(this.declaration as Node).render(code, options, { end, start });`
`39`	`39`	`}`
`40`	`40`	`}`
	`41`	`+`
	`42`	`+ protected applyDeoptimizations() {}`
`41`	`43`	`}`
`42`	`44`
`43`	`45`	`ExportNamedDeclaration.prototype.needsBoundaries = true;`
Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,6 @@ export default class ExportSpecifier extends NodeBase {`
`6`	`6`	`declare exported: Identifier;`
`7`	`7`	`declare local: Identifier;`
`8`	`8`	`declare type: NodeType.tExportSpecifier;`
	`9`	`+`
	`10`	`+ protected applyDeoptimizations() {}`
`9`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,4 +36,6 @@ export default class ExpressionStatement extends StatementBase {`
`36`	`36`
`37`	`37`	`return super.shouldBeIncluded(context);`
`38`	`38`	`}`
	`39`	`+`
	`40`	`+ protected applyDeoptimizations() {}`
`39`	`41`	`}`