diff --git a/src/ast/Entity.ts b/src/ast/Entity.ts index 4447be9f109..32bd4f51ea8 100644 --- a/src/ast/Entity.ts +++ b/src/ast/Entity.ts @@ -12,5 +12,10 @@ export interface WritableEntity extends Entity { * expression of this node is reassigned as well. */ deoptimizePath(path: ObjectPath): void; - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean; + + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors?: boolean + ): boolean; } diff --git a/src/ast/nodes/ArrayExpression.ts b/src/ast/nodes/ArrayExpression.ts index 406687d99fb..f79a82df6ad 100644 --- a/src/ast/nodes/ArrayExpression.ts +++ b/src/ast/nodes/ArrayExpression.ts @@ -60,8 +60,12 @@ export default class ArrayExpression extends NodeBase { return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context); } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { - return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context); + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { + return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors); } hasEffectsWhenCalledAtPath( diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts index 89965f79287..b786581b493 100644 --- a/src/ast/nodes/Identifier.ts +++ b/src/ast/nodes/Identifier.ts @@ -144,13 +144,17 @@ export default class Identifier extends NodeBase implements PatternNode { ); } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { return ( !this.variable || (path.length > 0 ? this.getVariableRespectingTDZ() : this.variable - ).hasEffectsWhenAssignedAtPath(path, context) + ).hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors) ); } diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts index 61ff336a6c1..9b3399adbd3 100644 --- a/src/ast/nodes/ObjectExpression.ts +++ b/src/ast/nodes/ObjectExpression.ts @@ -79,8 +79,12 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context); } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { - return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context); + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { + return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors); } hasEffectsWhenCalledAtPath( diff --git a/src/ast/nodes/shared/ClassNode.ts b/src/ast/nodes/shared/ClassNode.ts index f8823593b89..3bf932041b3 100644 --- a/src/ast/nodes/shared/ClassNode.ts +++ b/src/ast/nodes/shared/ClassNode.ts @@ -86,8 +86,12 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity { return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context); } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { - return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context); + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { + return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors); } hasEffectsWhenCalledAtPath( diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts index 60764af727c..01a46c7745f 100644 --- a/src/ast/nodes/shared/Expression.ts +++ b/src/ast/nodes/shared/Expression.ts @@ -52,7 +52,11 @@ export class ExpressionEntity implements WritableEntity { return true; } - hasEffectsWhenAssignedAtPath(_path: ObjectPath, _context: HasEffectsContext): boolean { + hasEffectsWhenAssignedAtPath( + _path: ObjectPath, + _context: HasEffectsContext, + _ignoreAccessors?: boolean + ): boolean { return true; } diff --git a/src/ast/nodes/shared/ObjectEntity.ts b/src/ast/nodes/shared/ObjectEntity.ts index ad84a57eb36..bb382bc38a4 100644 --- a/src/ast/nodes/shared/ObjectEntity.ts +++ b/src/ast/nodes/shared/ObjectEntity.ts @@ -302,7 +302,11 @@ export class ObjectEntity extends ExpressionEntity { return false; } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { const [key, ...subPath] = path; if (path.length > 1) { if (typeof key !== 'string') { @@ -310,14 +314,19 @@ export class ObjectEntity extends ExpressionEntity { } const expressionAtPath = this.getMemberExpression(key); if (expressionAtPath) { - return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context); + return expressionAtPath.hasEffectsWhenAssignedAtPath(subPath, context, ignoreAccessors); } if (this.prototypeExpression) { - return this.prototypeExpression.hasEffectsWhenAssignedAtPath(path, context); + return this.prototypeExpression.hasEffectsWhenAssignedAtPath( + path, + context, + ignoreAccessors + ); } return true; } + if (ignoreAccessors) return false; if (this.hasUnknownDeoptimizedProperty) return true; // We do not need to test for unknown properties as in that case, hasUnknownDeoptimizedProperty is true if (typeof key === 'string') { diff --git a/src/ast/nodes/shared/knownGlobals.ts b/src/ast/nodes/shared/knownGlobals.ts index b628fd3924f..3b8c05979f0 100644 --- a/src/ast/nodes/shared/knownGlobals.ts +++ b/src/ast/nodes/shared/knownGlobals.ts @@ -5,7 +5,11 @@ import type { ObjectPath } from '../../utils/PathTracker'; const ValueProperties = Symbol('Value Properties'); interface ValueDescription { - pure: boolean; + // Denotes a proper function except for the side effects listed explicitly + function: boolean; + // This assumes that mutation itself cannot have side effects and that this + // does not trigger setters or getters + mutatesArg1: boolean; } interface GlobalDescription { @@ -14,8 +18,8 @@ interface GlobalDescription { __proto__: null; } -const PURE: ValueDescription = { pure: true }; -const IMPURE: ValueDescription = { pure: false }; +const PURE: ValueDescription = { function: true, mutatesArg1: false }; +const IMPURE: ValueDescription = { function: false, mutatesArg1: false }; // We use shortened variables to reduce file size here /* OBJECT */ @@ -30,6 +34,12 @@ const PF: GlobalDescription = { [ValueProperties]: PURE }; +/* FUNCTION THAT MUTATES FIRST ARG WITHOUT TRIGGERING ACCESSORS */ +const MUTATES_ARG_WITHOUT_ACCESSOR: GlobalDescription = { + __proto__: null, + [ValueProperties]: { function: true, mutatesArg1: true } +}; + /* CONSTRUCTOR */ const C: GlobalDescription = { __proto__: null, @@ -173,6 +183,11 @@ const knownGlobals: GlobalDescription = { __proto__: null, [ValueProperties]: PURE, create: PF, + // Technically those can throw in certain situations, but we ignore this as + // code that relies on this will hopefully wrap this in a try-catch, which + // deoptimizes everything anyway + defineProperty: MUTATES_ARG_WITHOUT_ACCESSOR, + defineProperties: MUTATES_ARG_WITHOUT_ACCESSOR, getOwnPropertyDescriptor: PF, getOwnPropertyNames: PF, getOwnPropertySymbols: PF, @@ -847,7 +862,7 @@ for (const global of ['window', 'global', 'self', 'globalThis']) { knownGlobals[global] = knownGlobals; } -function getGlobalAtPath(path: ObjectPath): ValueDescription | null { +export function getGlobalAtPath(path: ObjectPath): ValueDescription | null { let currentGlobal: GlobalDescription | null = knownGlobals; for (const pathSegment of path) { if (typeof pathSegment !== 'string') { @@ -860,15 +875,3 @@ function getGlobalAtPath(path: ObjectPath): ValueDescription | null { } return currentGlobal[ValueProperties]; } - -export function isPureGlobal(path: ObjectPath): boolean { - const globalAtPath = getGlobalAtPath(path); - return globalAtPath !== null && globalAtPath.pure; -} - -export function isGlobalMember(path: ObjectPath): boolean { - if (path.length === 1) { - return path[0] === 'undefined' || getGlobalAtPath(path) !== null; - } - return getGlobalAtPath(path.slice(0, -1)) !== null; -} diff --git a/src/ast/variables/GlobalVariable.ts b/src/ast/variables/GlobalVariable.ts index 7ac5c466962..160d0825fd9 100644 --- a/src/ast/variables/GlobalVariable.ts +++ b/src/ast/variables/GlobalVariable.ts @@ -1,4 +1,7 @@ -import { isGlobalMember, isPureGlobal } from '../nodes/shared/knownGlobals'; +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 Variable from './Variable'; @@ -6,10 +9,24 @@ export default class GlobalVariable extends Variable { isReassigned = true; hasEffectsWhenAccessedAtPath(path: ObjectPath): boolean { - return !isGlobalMember([this.name, ...path]); + if (path.length === 0) { + return this.name !== 'undefined' && getGlobalAtPath([this.name]) === null; + } + return getGlobalAtPath([this.name, ...path].slice(0, -1)) === null; } - hasEffectsWhenCalledAtPath(path: ObjectPath): boolean { - return !isPureGlobal([this.name, ...path]); + hasEffectsWhenCalledAtPath( + path: ObjectPath, + callOptions: CallOptions, + context: HasEffectsContext + ): boolean { + const globalAtPath = getGlobalAtPath([this.name, ...path]); + return ( + globalAtPath === null || + !globalAtPath.function || + (globalAtPath.mutatesArg1 && + (!callOptions.args.length || + callOptions.args[0].hasEffectsWhenAssignedAtPath(UNKNOWN_PATH, context, true))) + ); } } diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts index a90613a2825..2ebbd5dd91b 100644 --- a/src/ast/variables/LocalVariable.ts +++ b/src/ast/variables/LocalVariable.ts @@ -150,13 +150,17 @@ export default class LocalVariable extends Variable { this.init.hasEffectsWhenAccessedAtPath(path, context))!; } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): 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))!; + this.init.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors))!; } hasEffectsWhenCalledAtPath( diff --git a/src/ast/variables/ThisVariable.ts b/src/ast/variables/ThisVariable.ts index 7c4cba21045..c783be89803 100644 --- a/src/ast/variables/ThisVariable.ts +++ b/src/ast/variables/ThisVariable.ts @@ -73,10 +73,14 @@ export default class ThisVariable extends LocalVariable { ); } - hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean { + hasEffectsWhenAssignedAtPath( + path: ObjectPath, + context: HasEffectsContext, + ignoreAccessors: boolean + ): boolean { return ( this.getInit(context).hasEffectsWhenAssignedAtPath(path, context) || - super.hasEffectsWhenAssignedAtPath(path, context) + super.hasEffectsWhenAssignedAtPath(path, context, ignoreAccessors) ); } diff --git a/test/form/samples/object-define-property/_config.js b/test/form/samples/object-define-property/_config.js new file mode 100644 index 00000000000..406267436f3 --- /dev/null +++ b/test/form/samples/object-define-property/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'allows globals to have parameter mutation side effects' +}; diff --git a/test/form/samples/object-define-property/_expected.js b/test/form/samples/object-define-property/_expected.js new file mode 100644 index 00000000000..58141e185ae --- /dev/null +++ b/test/form/samples/object-define-property/_expected.js @@ -0,0 +1,7 @@ +const retained1 = {}; +Object.defineProperty(retained1, 'foo', { value: true }); +console.log(retained1); + +const retained2 = {}; +Object.defineProperties(retained2, { bar: { value: true } }); +console.log(retained2); diff --git a/test/form/samples/object-define-property/main.js b/test/form/samples/object-define-property/main.js new file mode 100644 index 00000000000..0b23bfb3f22 --- /dev/null +++ b/test/form/samples/object-define-property/main.js @@ -0,0 +1,20 @@ +const removed = {}; +Object.defineProperty(removed, 'foo', { value: true }); +Object.defineProperties(removed, { bar: { value: true } }); + +const retained1 = {}; +Object.defineProperty(retained1, 'foo', { value: true }); +console.log(retained1); + +const retained2 = {}; +Object.defineProperties(retained2, { bar: { value: true } }); +console.log(retained2); + +const removed2 = []; +Object.defineProperty(removed2, 'foo', { value: true }); + +class removed3 {} +Object.defineProperty(removed3, 'foo', { value: true }); + +function removed4() {} +Object.defineProperty(removed4, 'foo', { value: true });