Use new path key for defineProperty side effects

rollup · May 10, 2022 · 47a4de2 · 47a4de2
1 parent 1a9cb92
commit 47a4de2
Show file tree

Hide file tree

Showing 12 changed files with 28 additions and 67 deletions.
diff --git a/src/ast/Entity.ts b/src/ast/Entity.ts
@@ -13,10 +13,5 @@ export interface WritableEntity extends Entity {
 	 */
 	deoptimizePath(path: ObjectPath): void;
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		// TODO Lukas use the new key value instead?
-		ignoreAccessors?: boolean
-	): boolean;
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
 }
diff --git a/src/ast/nodes/ArrayExpression.ts b/src/ast/nodes/ArrayExpression.ts
@@ -60,12 +60,8 @@ export default class ArrayExpression extends NodeBase {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(

diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts
@@ -144,17 +144,13 @@ export default class Identifier extends NodeBase implements PatternNode {
 		);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
 		return (
 			!this.variable ||
 			(path.length > 0
 				? this.getVariableRespectingTDZ()
 				: this.variable
-			).hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors)
+			).hasEffectsWhenAssignedAtPath(path, context)
 		);
 	}
 

diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts
@@ -79,12 +79,8 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(

diff --git a/src/ast/nodes/shared/ClassNode.ts b/src/ast/nodes/shared/ClassNode.ts
@@ -86,12 +86,8 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(

diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts
@@ -52,11 +52,7 @@ export class ExpressionEntity implements WritableEntity {
 		return true;
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		_path: ObjectPath,
-		_context: HasEffectsContext,
-		_ignoreAccessors?: boolean
-	): boolean {
+	hasEffectsWhenAssignedAtPath(_path: ObjectPath, _context: HasEffectsContext): boolean {
 		return true;
 	}
 

diff --git a/src/ast/nodes/shared/FunctionBase.ts b/src/ast/nodes/shared/FunctionBase.ts
@@ -95,12 +95,8 @@ export default class FunctionBase extends NodeBase {
 		return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
-		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors);
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
+		return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
 	}
 
 	hasEffectsWhenCalledAtPath(

diff --git a/src/ast/nodes/shared/ObjectEntity.ts b/src/ast/nodes/shared/ObjectEntity.ts
@@ -317,31 +317,23 @@ export class ObjectEntity extends ExpressionEntity {
 		return false;
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
 		const [key, ...subPath] = path;
 		if (path.length > 1) {
 			if (typeof key !== 'string') {
 				return true;
 			}
 			const expressionAtPath = this.getMemberExpression(key);
 			if (expressionAtPath) {
-				return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context, ignoreAccessors);
+				return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context);
 			}
 			if (this.prototypeExpression) {
-				return this.prototypeExpression.hasEffectsWhenAssignedAtPath(
-					path,
-					context,
-					ignoreAccessors
-				);
+				return this.prototypeExpression.hasEffectsWhenAssignedAtPath(path, context);
 			}
 			return true;
 		}
 
-		if (ignoreAccessors) return false;
+		if (key === UnknownNonAccessorKey) return false;
 		if (this.hasLostTrack) return true;
 		if (typeof key === 'string') {
 			if (this.propertiesAndSettersByKey[key]) {

diff --git a/src/ast/utils/PathTracker.ts b/src/ast/utils/PathTracker.ts
@@ -13,6 +13,12 @@ export type ObjectPathKey =
 export type ObjectPath = ObjectPathKey[];
 export const EMPTY_PATH: ObjectPath = [];
 export const UNKNOWN_PATH: ObjectPath = [UnknownKey];
+// For deoptimizations, this means we are modifying an unknown property but did
+// not lose track of the object or are creating a setter/getter;
+// For assignment effects it means we do not check for setter/getter effects
+// but only if something is mutated that is included, which is relevant for
+// Object.defineProperty
+export const UNKNOWN_NON_ACCESSOR_PATH: ObjectPath = [UnknownNonAccessorKey];
 export const UNKNOWN_INTEGER_PATH: ObjectPath = [UnknownInteger];
 
 const EntitiesKey = Symbol('Entities');

diff --git a/src/ast/variables/GlobalVariable.ts b/src/ast/variables/GlobalVariable.ts
@@ -1,8 +1,8 @@
 import { CallOptions } from '../CallOptions';
 import { HasEffectsContext } from '../ExecutionContext';
 import { getGlobalAtPath } from '../nodes/shared/knownGlobals';
-import { UNKNOWN_PATH } from '../utils/PathTracker';
 import type { ObjectPath } from '../utils/PathTracker';
+import { UNKNOWN_NON_ACCESSOR_PATH } from '../utils/PathTracker';
 import Variable from './Variable';
 
 export default class GlobalVariable extends Variable {
@@ -26,7 +26,7 @@ export default class GlobalVariable extends Variable {
 			!globalAtPath.function ||
 			(globalAtPath.mutatesArg1 &&
 				(!callOptions.args.length ||
-					callOptions.args[0].hasEffectsWhenAssignedAtPath(UNKNOWN_PATH, context, true)))
+					callOptions.args[0].hasEffectsWhenAssignedAtPath(UNKNOWN_NON_ACCESSOR_PATH, context)))
 		);
 	}
 }
diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts
@@ -150,17 +150,13 @@ export default class LocalVariable extends Variable {
 			this.init.hasEffectsWhenAccessedAtPath(path, context))!;
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
 		if (this.included) return true;
 		if (path.length === 0) return false;
 		if (this.isReassigned) return true;
 		return (this.init &&
 			!context.assigned.trackEntityAtPathAndGetIfTracked(path, this) &&
-			this.init.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors))!;
+			this.init.hasEffectsWhenAssignedAtPath(path, context))!;
 	}
 
 	hasEffectsWhenCalledAtPath(

diff --git a/src/ast/variables/ThisVariable.ts b/src/ast/variables/ThisVariable.ts
@@ -73,14 +73,10 @@ export default class ThisVariable extends LocalVariable {
 		);
 	}
 
-	hasEffectsWhenAssignedAtPath(
-		path: ObjectPath,
-		context: HasEffectsContext,
-		ignoreAccessors: boolean
-	): boolean {
+	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
 		return (
 			this.getInit(context).hasEffectsWhenAssignedAtPath(path, context) ||
-			super.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors)
+			super.hasEffectsWhenAssignedAtPath(path, context)
 		);
 	}