From b0eb2715574071c6a21d44498c51e479c89e1758 Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Wed, 24 Mar 2021 17:23:25 +0100 Subject: [PATCH 1/5] Disable pessimistic object deoptimization for calls when the called function doesn't ref this Issue #3989 --- src/ast/nodes/CallExpression.ts | 3 ++- src/ast/nodes/Identifier.ts | 6 ++++++ src/ast/nodes/MemberExpression.ts | 9 +++++++++ src/ast/nodes/ObjectExpression.ts | 10 ++++++++++ src/ast/nodes/ThisExpression.ts | 7 +++++++ src/ast/nodes/shared/Expression.ts | 3 +++ src/ast/nodes/shared/FunctionNode.ts | 10 ++++++++++ src/ast/nodes/shared/MultiExpression.ts | 4 ++++ src/ast/nodes/shared/Node.ts | 6 ++++++ src/ast/values.ts | 13 +++++++++++++ src/ast/variables/LocalVariable.ts | 9 +++++++++ src/ast/variables/Variable.ts | 6 ++++++ test/form/samples/getter-return-values/_expected.js | 12 ------------ .../method-side-effects/_expected.js | 9 --------- .../early-access-getter-return/_expected.js | 11 +---------- test/form/samples/recursive-calls/_expected.js | 7 ------- 16 files changed, 86 insertions(+), 39 deletions(-) diff --git a/src/ast/nodes/CallExpression.ts b/src/ast/nodes/CallExpression.ts index aed124fa615..9df64414e34 100644 --- a/src/ast/nodes/CallExpression.ts +++ b/src/ast/nodes/CallExpression.ts @@ -65,7 +65,8 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt // ensure the returnExpression is set for the tree-shaking passes this.getReturnExpression(SHARED_RECURSION_TRACKER); // This deoptimizes "this" for non-namespace calls until we have a better solution - if (this.callee instanceof MemberExpression && !this.callee.variable) { + if (this.callee instanceof MemberExpression && !this.callee.variable && + this.callee.mayModifyThisWhenCalledAtPath([])) { this.callee.object.deoptimizePath(UNKNOWN_PATH); } for (const argument of this.arguments) { diff --git a/src/ast/nodes/Identifier.ts b/src/ast/nodes/Identifier.ts index 59ef41f2e97..e68090cced3 100644 --- a/src/ast/nodes/Identifier.ts +++ b/src/ast/nodes/Identifier.ts @@ -141,6 +141,12 @@ export default class Identifier extends NodeBase implements PatternNode { this.variable!.includeCallArguments(context, args); } + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ) { + return this.variable ? this.variable.mayModifyThisWhenCalledAtPath(path) : true + } + render( code: MagicString, _options: RenderOptions, diff --git a/src/ast/nodes/MemberExpression.ts b/src/ast/nodes/MemberExpression.ts index dbadffebb9e..0b0b13f886e 100644 --- a/src/ast/nodes/MemberExpression.ts +++ b/src/ast/nodes/MemberExpression.ts @@ -233,6 +233,15 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE this.propertyKey = getResolvablePropertyKey(this); } + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ) { + if (this.variable) { + return this.variable.mayModifyThisWhenCalledAtPath(path); + } + return this.object.mayModifyThisWhenCalledAtPath([this.propertyKey as ObjectPathKey].concat(path)); + } + render( code: MagicString, options: RenderOptions, diff --git a/src/ast/nodes/ObjectExpression.ts b/src/ast/nodes/ObjectExpression.ts index 1925c70bc93..3b32620de48 100644 --- a/src/ast/nodes/ObjectExpression.ts +++ b/src/ast/nodes/ObjectExpression.ts @@ -256,6 +256,16 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE return false; } + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ) { + if (!path.length || typeof path[0] !== "string") { + return true; + } + const property = this.getPropertyMap()[path[0]]?.exactMatchRead; + return property ? property.value.mayModifyThisWhenCalledAtPath(path.slice(1)) : true; + } + render( code: MagicString, options: RenderOptions, diff --git a/src/ast/nodes/ThisExpression.ts b/src/ast/nodes/ThisExpression.ts index 512ac7b933b..f68631adae6 100644 --- a/src/ast/nodes/ThisExpression.ts +++ b/src/ast/nodes/ThisExpression.ts @@ -4,6 +4,7 @@ import ModuleScope from '../scopes/ModuleScope'; import { ObjectPath } from '../utils/PathTracker'; import ThisVariable from '../variables/ThisVariable'; import * as NodeType from './NodeType'; +import FunctionNode from './shared/FunctionNode'; import { NodeBase } from './shared/Node'; export default class ThisExpression extends NodeBase { @@ -38,6 +39,12 @@ export default class ThisExpression extends NodeBase { this.start ); } + for (let parent = this.parent; parent instanceof NodeBase; parent = parent.parent) { + if (parent instanceof FunctionNode) { + parent.referencesThis = true; + break; + } + } } render(code: MagicString) { diff --git a/src/ast/nodes/shared/Expression.ts b/src/ast/nodes/shared/Expression.ts index 08b03ae8e1a..013d859e853 100644 --- a/src/ast/nodes/shared/Expression.ts +++ b/src/ast/nodes/shared/Expression.ts @@ -33,4 +33,7 @@ export interface ExpressionEntity extends WritableEntity { ): boolean; include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void; includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void; + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ): boolean; } diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts index 629f02118c4..7965a9bbf67 100644 --- a/src/ast/nodes/shared/FunctionNode.ts +++ b/src/ast/nodes/shared/FunctionNode.ts @@ -16,12 +16,16 @@ export default class FunctionNode extends NodeBase { id!: IdentifierWithVariable | null; params!: PatternNode[]; preventChildBlockScope!: true; + referencesThis!: boolean; scope!: FunctionScope; private isPrototypeDeoptimized = false; createScope(parentScope: FunctionScope) { this.scope = new FunctionScope(parentScope, this.context); + // Initialized here because child nodes will update it before + // `this.initialize` even runs. + this.referencesThis = false; } deoptimizePath(path: ObjectPath) { @@ -120,6 +124,12 @@ export default class FunctionNode extends NodeBase { this.body.addImplicitReturnExpressionToScope(); } + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ) { + return path.length ? true : this.referencesThis + } + parseNode(esTreeNode: GenericEsTreeNode) { this.body = new this.context.nodeConstructors.BlockStatement( esTreeNode.body, diff --git a/src/ast/nodes/shared/MultiExpression.ts b/src/ast/nodes/shared/MultiExpression.ts index 31c18750c49..6f3c7c40aa4 100644 --- a/src/ast/nodes/shared/MultiExpression.ts +++ b/src/ast/nodes/shared/MultiExpression.ts @@ -73,4 +73,8 @@ export class MultiExpression implements ExpressionEntity { } includeCallArguments(): void {} + + mayModifyThisWhenCalledAtPath(path: ObjectPath) { + return this.expressions.some(e => e.mayModifyThisWhenCalledAtPath(path)); + } } diff --git a/src/ast/nodes/shared/Node.ts b/src/ast/nodes/shared/Node.ts index 524ea0c2628..6797be5b135 100644 --- a/src/ast/nodes/shared/Node.ts +++ b/src/ast/nodes/shared/Node.ts @@ -229,6 +229,12 @@ export class NodeBase implements ExpressionNode { } } + mayModifyThisWhenCalledAtPath( + _path: ObjectPath + ) { + return true + } + parseNode(esTreeNode: GenericEsTreeNode) { for (const key of Object.keys(esTreeNode)) { // That way, we can override this function to add custom initialisation and then call super.parseNode diff --git a/src/ast/values.ts b/src/ast/values.ts index f2c179ac2c8..dc0e4bd124c 100644 --- a/src/ast/values.ts +++ b/src/ast/values.ts @@ -45,6 +45,7 @@ export const UNKNOWN_EXPRESSION: ExpressionEntity = { } }, included: true, + mayModifyThisWhenCalledAtPath: () => true, toString: () => '[[UNKNOWN]]' }; @@ -58,6 +59,7 @@ export const UNDEFINED_EXPRESSION: ExpressionEntity = { include: () => {}, includeCallArguments(): void {}, included: true, + mayModifyThisWhenCalledAtPath: () => true, toString: () => 'undefined' }; @@ -121,6 +123,10 @@ export class UnknownArrayExpression implements ExpressionEntity { } } + mayModifyThisWhenCalledAtPath(_path: ObjectPath) { + return true; + } + toString() { return '[[UNKNOWN ARRAY]]'; } @@ -184,6 +190,7 @@ const UNKNOWN_LITERAL_BOOLEAN: ExpressionEntity = { } }, included: true, + mayModifyThisWhenCalledAtPath: () => true, toString: () => '[[UNKNOWN BOOLEAN]]' }; @@ -229,6 +236,7 @@ const UNKNOWN_LITERAL_NUMBER: ExpressionEntity = { } }, included: true, + mayModifyThisWhenCalledAtPath: () => true, toString: () => '[[UNKNOWN NUMBER]]' }; @@ -281,6 +289,7 @@ const UNKNOWN_LITERAL_STRING: ExpressionEntity = { } }, included: true, + mayModifyThisWhenCalledAtPath: () => true, toString: () => '[[UNKNOWN STRING]]' }; @@ -338,6 +347,10 @@ export class UnknownObjectExpression implements ExpressionEntity { } } + mayModifyThisWhenCalledAtPath(_path: ObjectPath) { + return true; + } + toString() { return '[[UNKNOWN OBJECT]]'; } diff --git a/src/ast/variables/LocalVariable.ts b/src/ast/variables/LocalVariable.ts index 3dd5502afa8..0a28aaa0ef4 100644 --- a/src/ast/variables/LocalVariable.ts +++ b/src/ast/variables/LocalVariable.ts @@ -188,4 +188,13 @@ export default class LocalVariable extends Variable { markCalledFromTryStatement() { this.calledFromTryStatement = true; } + + mayModifyThisWhenCalledAtPath( + path: ObjectPath + ) { + if (this.isReassigned || !this.init || path.length > MAX_PATH_DEPTH) { + return true; + } + return this.init.mayModifyThisWhenCalledAtPath(path); + } } diff --git a/src/ast/variables/Variable.ts b/src/ast/variables/Variable.ts index dd0c8ee4fb7..1765e579c0e 100644 --- a/src/ast/variables/Variable.ts +++ b/src/ast/variables/Variable.ts @@ -96,6 +96,12 @@ export default class Variable implements ExpressionEntity { markCalledFromTryStatement() {} + mayModifyThisWhenCalledAtPath( + _path: ObjectPath + ) { + return true + } + setRenderNames(baseName: string | null, name: string | null) { this.renderBaseName = baseName; this.renderName = name; diff --git a/test/form/samples/getter-return-values/_expected.js b/test/form/samples/getter-return-values/_expected.js index 7682aece034..61ea5587251 100644 --- a/test/form/samples/getter-return-values/_expected.js +++ b/test/form/samples/getter-return-values/_expected.js @@ -9,12 +9,6 @@ return {}; } }).foo.bar.baz; - -({ - get foo () { - return () => {}; - } -}).foo(); ({ get foo () { console.log( 'effect' ); @@ -26,12 +20,6 @@ return () => console.log( 'effect' ); } }).foo(); - -({ - get foo () { - return () => () => {}; - } -}).foo()(); ({ get foo () { console.log( 'effect' ); diff --git a/test/form/samples/object-expression/method-side-effects/_expected.js b/test/form/samples/object-expression/method-side-effects/_expected.js index d40ea5a9315..c56d49d6d6a 100644 --- a/test/form/samples/object-expression/method-side-effects/_expected.js +++ b/test/form/samples/object-expression/method-side-effects/_expected.js @@ -1,12 +1,3 @@ -const x = { - [globalThis.unknown]() { - console.log('effect'); - }, - a() {} -}; - -x.a(); - const y = { a() {}, [globalThis.unknown]() { diff --git a/test/form/samples/property-setters-and-getters/early-access-getter-return/_expected.js b/test/form/samples/property-setters-and-getters/early-access-getter-return/_expected.js index 745a1206e6e..b57b090e68c 100644 --- a/test/form/samples/property-setters-and-getters/early-access-getter-return/_expected.js +++ b/test/form/samples/property-setters-and-getters/early-access-getter-return/_expected.js @@ -1,16 +1,7 @@ function getReturnExpressionBeforeInit() { - const bar = { - [foo.value()]: true - }; - if (bar.baz) { + { console.log('retained'); } } -const foo = { - get value() { - return () => 'baz'; - } -}; - getReturnExpressionBeforeInit(); diff --git a/test/form/samples/recursive-calls/_expected.js b/test/form/samples/recursive-calls/_expected.js index ac8c402c4d1..00f0e5d1be2 100644 --- a/test/form/samples/recursive-calls/_expected.js +++ b/test/form/samples/recursive-calls/_expected.js @@ -1,13 +1,6 @@ const removed4 = () => globalThis.unknown ? removed4() : { x: () => {} }; removed4().x(); -const removed6 = { - get x () { - return globalThis.unknown ? removed6.x : () => {}; - } -}; -removed6.x(); - const removed8 = { get x () { return globalThis.unknown ? removed8.x : { y: () => {} }; From 41b345308db15cda9936400c4b6a03853a60d89b Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Thu, 25 Mar 2021 11:48:51 +0100 Subject: [PATCH 2/5] Use a base class for special-purpose value objects To avoid duplicating all ExpressionEntity methods for every object. --- src/ast/values.ts | 202 ++++++++++++++++++++++------------------------ 1 file changed, 96 insertions(+), 106 deletions(-) diff --git a/src/ast/values.ts b/src/ast/values.ts index dc0e4bd124c..71234fbe223 100644 --- a/src/ast/values.ts +++ b/src/ast/values.ts @@ -31,36 +31,68 @@ function assembleMemberDescriptions( export const UnknownValue = Symbol('Unknown Value'); export type LiteralValueOrUnknown = LiteralValue | typeof UnknownValue; -export const UNKNOWN_EXPRESSION: ExpressionEntity = { - deoptimizePath: () => {}, - getLiteralValueAtPath: () => UnknownValue, - getReturnExpressionWhenCalledAtPath: () => UNKNOWN_EXPRESSION, - hasEffectsWhenAccessedAtPath: path => path.length > 0, - hasEffectsWhenAssignedAtPath: path => path.length > 0, - hasEffectsWhenCalledAtPath: () => true, - include: () => {}, +abstract class ValueBase implements ExpressionEntity { + included = true; + + deoptimizePath() {} + + getLiteralValueAtPath(): LiteralValueOrUnknown { + return UnknownValue; + } + + getReturnExpressionWhenCalledAtPath(_path: ObjectPath) { + return UNKNOWN_EXPRESSION; + } + + hasEffectsWhenAccessedAtPath( + path: ObjectPath, + _context: HasEffectsContext + ) { + return path.length > 0 + } + + hasEffectsWhenAssignedAtPath(path: ObjectPath) { + return path.length > 0 + } + + hasEffectsWhenCalledAtPath( + _path: ObjectPath, + _callOptions: CallOptions, + _context: HasEffectsContext + ) { + return true; + } + + include() {} + + includeCallArguments(_context: InclusionContext, _args: (ExpressionNode | SpreadElement)[]) {} + + mayModifyThisWhenCalledAtPath() { return true; } + + abstract toString(): string +} + +function includeAll(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]) { + for (const arg of args) { + arg.include(context, false); + } +} + +export const UNKNOWN_EXPRESSION: ExpressionEntity = new class extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - }, - included: true, - mayModifyThisWhenCalledAtPath: () => true, - toString: () => '[[UNKNOWN]]' + includeAll(context, args); + } + toString() { return '[[UNKNOWN]]' } }; -export const UNDEFINED_EXPRESSION: ExpressionEntity = { - deoptimizePath: () => {}, - getLiteralValueAtPath: () => undefined, - getReturnExpressionWhenCalledAtPath: () => UNKNOWN_EXPRESSION, - hasEffectsWhenAccessedAtPath: path => path.length > 0, - hasEffectsWhenAssignedAtPath: path => path.length > 0, - hasEffectsWhenCalledAtPath: () => true, - include: () => {}, - includeCallArguments(): void {}, - included: true, - mayModifyThisWhenCalledAtPath: () => true, - toString: () => 'undefined' +export const UNDEFINED_EXPRESSION: ExpressionEntity = new class extends ValueBase { + getLiteralValueAtPath() { + return undefined; + } + + toString() { + return 'undefined' + } }; const returnsUnknown: RawMemberDescription = { @@ -78,15 +110,9 @@ const callsArgReturnsUnknown: RawMemberDescription = { value: { returns: null, returnsPrimitive: UNKNOWN_EXPRESSION, callsArgs: [0], mutatesSelf: false } }; -export class UnknownArrayExpression implements ExpressionEntity { +export class UnknownArrayExpression extends ValueBase { included = false; - deoptimizePath() {} - - getLiteralValueAtPath(): LiteralValueOrUnknown { - return UnknownValue; - } - getReturnExpressionWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { return getMemberReturnExpressionWhenCalled(arrayMembers, path[0]); @@ -118,13 +144,7 @@ export class UnknownArrayExpression implements ExpressionEntity { } includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - } - - mayModifyThisWhenCalledAtPath(_path: ObjectPath) { - return true; + includeAll(context, args); } toString() { @@ -165,33 +185,27 @@ const callsArgMutatesSelfReturnsArray: RawMemberDescription = { } }; -const UNKNOWN_LITERAL_BOOLEAN: ExpressionEntity = { - deoptimizePath: () => {}, - getLiteralValueAtPath: () => UnknownValue, - getReturnExpressionWhenCalledAtPath: path => { +const UNKNOWN_LITERAL_BOOLEAN: ExpressionEntity = new class extends ValueBase { + getReturnExpressionWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { return getMemberReturnExpressionWhenCalled(literalBooleanMembers, path[0]); } return UNKNOWN_EXPRESSION; - }, - hasEffectsWhenAccessedAtPath: path => path.length > 1, - hasEffectsWhenAssignedAtPath: path => path.length > 0, - hasEffectsWhenCalledAtPath: path => { + } + hasEffectsWhenAccessedAtPath(path: ObjectPath) { + return path.length > 1 + } + hasEffectsWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { const subPath = path[0]; return typeof subPath !== 'string' || !literalBooleanMembers[subPath]; } return true; - }, - include: () => {}, + } includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - }, - included: true, - mayModifyThisWhenCalledAtPath: () => true, - toString: () => '[[UNKNOWN BOOLEAN]]' + includeAll(context, args); + } + toString() { return '[[UNKNOWN BOOLEAN]]' } }; const returnsBoolean: RawMemberDescription = { @@ -211,33 +225,25 @@ const callsArgReturnsBoolean: RawMemberDescription = { } }; -const UNKNOWN_LITERAL_NUMBER: ExpressionEntity = { - deoptimizePath: () => {}, - getLiteralValueAtPath: () => UnknownValue, - getReturnExpressionWhenCalledAtPath: path => { +const UNKNOWN_LITERAL_NUMBER: ExpressionEntity = new class extends ValueBase { + getReturnExpressionWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { return getMemberReturnExpressionWhenCalled(literalNumberMembers, path[0]); } return UNKNOWN_EXPRESSION; - }, - hasEffectsWhenAccessedAtPath: path => path.length > 1, - hasEffectsWhenAssignedAtPath: path => path.length > 0, - hasEffectsWhenCalledAtPath: path => { + } + hasEffectsWhenAccessedAtPath(path: ObjectPath) { return path.length > 1; } + hasEffectsWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { const subPath = path[0]; return typeof subPath !== 'string' || !literalNumberMembers[subPath]; } return true; - }, - include: () => {}, + } includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - }, - included: true, - mayModifyThisWhenCalledAtPath: () => true, - toString: () => '[[UNKNOWN NUMBER]]' + includeAll(context, args); + } + toString() { return '[[UNKNOWN NUMBER]]'; } }; const returnsNumber: RawMemberDescription = { @@ -265,32 +271,28 @@ const callsArgReturnsNumber: RawMemberDescription = { } }; -const UNKNOWN_LITERAL_STRING: ExpressionEntity = { - deoptimizePath: () => {}, - getLiteralValueAtPath: () => UnknownValue, - getReturnExpressionWhenCalledAtPath: path => { +const UNKNOWN_LITERAL_STRING: ExpressionEntity = new class extends ValueBase { + getReturnExpressionWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { return getMemberReturnExpressionWhenCalled(literalStringMembers, path[0]); } return UNKNOWN_EXPRESSION; - }, - hasEffectsWhenAccessedAtPath: path => path.length > 1, - hasEffectsWhenAssignedAtPath: path => path.length > 0, - hasEffectsWhenCalledAtPath: (path, callOptions, context) => { + } + hasEffectsWhenAccessedAtPath(path: ObjectPath) { return path.length > 1 } + hasEffectsWhenCalledAtPath( + path: ObjectPath, + callOptions: CallOptions, + context: HasEffectsContext + ) { if (path.length === 1) { return hasMemberEffectWhenCalled(literalStringMembers, path[0], true, callOptions, context); } return true; - }, - include: () => {}, + } includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - }, - included: true, - mayModifyThisWhenCalledAtPath: () => true, - toString: () => '[[UNKNOWN STRING]]' + includeAll(context, args); + } + toString() { return '[[UNKNOWN STRING]]' } }; const returnsString: RawMemberDescription = { @@ -302,15 +304,9 @@ const returnsString: RawMemberDescription = { } }; -export class UnknownObjectExpression implements ExpressionEntity { +export class UnknownObjectExpression extends ValueBase { included = false; - deoptimizePath() {} - - getLiteralValueAtPath(): LiteralValueOrUnknown { - return UnknownValue; - } - getReturnExpressionWhenCalledAtPath(path: ObjectPath) { if (path.length === 1) { return getMemberReturnExpressionWhenCalled(objectMembers, path[0]); @@ -342,13 +338,7 @@ export class UnknownObjectExpression implements ExpressionEntity { } includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { - for (const arg of args) { - arg.include(context, false); - } - } - - mayModifyThisWhenCalledAtPath(_path: ObjectPath) { - return true; + includeAll(context, args); } toString() { From e9cb664f9e27f8e48211f552758e10b3c253f70d Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 26 Mar 2021 07:01:25 +0100 Subject: [PATCH 3/5] Remove toString for values --- src/ast/values.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/ast/values.ts b/src/ast/values.ts index 71234fbe223..0d7ea2ca50f 100644 --- a/src/ast/values.ts +++ b/src/ast/values.ts @@ -68,8 +68,6 @@ abstract class ValueBase implements ExpressionEntity { includeCallArguments(_context: InclusionContext, _args: (ExpressionNode | SpreadElement)[]) {} mayModifyThisWhenCalledAtPath() { return true; } - - abstract toString(): string } function includeAll(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]) { @@ -82,17 +80,12 @@ export const UNKNOWN_EXPRESSION: ExpressionEntity = new class extends ValueBase includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - toString() { return '[[UNKNOWN]]' } }; export const UNDEFINED_EXPRESSION: ExpressionEntity = new class extends ValueBase { getLiteralValueAtPath() { return undefined; } - - toString() { - return 'undefined' - } }; const returnsUnknown: RawMemberDescription = { @@ -146,10 +139,6 @@ export class UnknownArrayExpression extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - - toString() { - return '[[UNKNOWN ARRAY]]'; - } } const returnsArray: RawMemberDescription = { @@ -205,7 +194,6 @@ const UNKNOWN_LITERAL_BOOLEAN: ExpressionEntity = new class extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - toString() { return '[[UNKNOWN BOOLEAN]]' } }; const returnsBoolean: RawMemberDescription = { @@ -243,7 +231,6 @@ const UNKNOWN_LITERAL_NUMBER: ExpressionEntity = new class extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - toString() { return '[[UNKNOWN NUMBER]]'; } }; const returnsNumber: RawMemberDescription = { @@ -292,7 +279,6 @@ const UNKNOWN_LITERAL_STRING: ExpressionEntity = new class extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - toString() { return '[[UNKNOWN STRING]]' } }; const returnsString: RawMemberDescription = { @@ -340,10 +326,6 @@ export class UnknownObjectExpression extends ValueBase { includeCallArguments(context: InclusionContext, args: (ExpressionNode | SpreadElement)[]): void { includeAll(context, args); } - - toString() { - return '[[UNKNOWN OBJECT]]'; - } } export const objectMembers: MemberDescriptions = assembleMemberDescriptions({ From 12792eb0f3f7a9c9822d74d5530c0aef4aed17cf Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 26 Mar 2021 07:01:55 +0100 Subject: [PATCH 4/5] Move function Node initialization to parseNode --- src/ast/nodes/shared/FunctionNode.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ast/nodes/shared/FunctionNode.ts b/src/ast/nodes/shared/FunctionNode.ts index 7965a9bbf67..65e81bed072 100644 --- a/src/ast/nodes/shared/FunctionNode.ts +++ b/src/ast/nodes/shared/FunctionNode.ts @@ -23,9 +23,6 @@ export default class FunctionNode extends NodeBase { createScope(parentScope: FunctionScope) { this.scope = new FunctionScope(parentScope, this.context); - // Initialized here because child nodes will update it before - // `this.initialize` even runs. - this.referencesThis = false; } deoptimizePath(path: ObjectPath) { @@ -131,6 +128,7 @@ export default class FunctionNode extends NodeBase { } parseNode(esTreeNode: GenericEsTreeNode) { + this.referencesThis = false; this.body = new this.context.nodeConstructors.BlockStatement( esTreeNode.body, this, From 4214087fcbcff67ba8d6770fbdf32226cf54c442 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 26 Mar 2021 07:13:55 +0100 Subject: [PATCH 5/5] Also handle arrow functions --- src/ast/nodes/ArrowFunctionExpression.ts | 4 ++++ .../method-side-effects/_expected.js | 4 +++- .../method-side-effects/main.js | 8 ++++++-- .../return-expressions/_expected.js | 7 ------- .../_expected.js | 20 ------------------- .../_expected.js | 6 ------ 6 files changed, 13 insertions(+), 36 deletions(-) diff --git a/src/ast/nodes/ArrowFunctionExpression.ts b/src/ast/nodes/ArrowFunctionExpression.ts index 8d26d04a7df..52c08376857 100644 --- a/src/ast/nodes/ArrowFunctionExpression.ts +++ b/src/ast/nodes/ArrowFunctionExpression.ts @@ -98,6 +98,10 @@ export default class ArrowFunctionExpression extends NodeBase { } } + mayModifyThisWhenCalledAtPath() { + return false; + } + parseNode(esTreeNode: GenericEsTreeNode) { if (esTreeNode.body.type === NodeType.BlockStatement) { this.body = new this.context.nodeConstructors.BlockStatement( diff --git a/test/form/samples/object-expression/method-side-effects/_expected.js b/test/form/samples/object-expression/method-side-effects/_expected.js index c56d49d6d6a..4a4393f1dc8 100644 --- a/test/form/samples/object-expression/method-side-effects/_expected.js +++ b/test/form/samples/object-expression/method-side-effects/_expected.js @@ -1,15 +1,17 @@ const y = { a() {}, + b: () => {}, [globalThis.unknown]() { console.log('effect'); } }; y.a(); +y.b(); const z = { [globalThis.unknown]() {} }; z.a(); -z.hasOwnProperty('a'); // removed +z.hasOwnProperty('a'); diff --git a/test/form/samples/object-expression/method-side-effects/main.js b/test/form/samples/object-expression/method-side-effects/main.js index d40ea5a9315..5c272ae9920 100644 --- a/test/form/samples/object-expression/method-side-effects/main.js +++ b/test/form/samples/object-expression/method-side-effects/main.js @@ -2,23 +2,27 @@ const x = { [globalThis.unknown]() { console.log('effect'); }, - a() {} + a() {}, + b: () => {} }; x.a(); +x.b(); const y = { a() {}, + b: () => {}, [globalThis.unknown]() { console.log('effect'); } }; y.a(); +y.b(); const z = { [globalThis.unknown]() {} }; z.a(); -z.hasOwnProperty('a'); // removed +z.hasOwnProperty('a'); diff --git a/test/form/samples/object-expression/return-expressions/_expected.js b/test/form/samples/object-expression/return-expressions/_expected.js index 9565727abf5..0405706b107 100644 --- a/test/form/samples/object-expression/return-expressions/_expected.js +++ b/test/form/samples/object-expression/return-expressions/_expected.js @@ -1,10 +1,3 @@ -const x = { - [globalThis.unknown]: () => () => console.log('effect'), - a: () => () => {} -}; - -x.a()(); - const y = { a: () => () => {}, [globalThis.unknown]: () => () => console.log('effect') diff --git a/test/form/samples/object-literal-property-overwrites/_expected.js b/test/form/samples/object-literal-property-overwrites/_expected.js index 1f2665f813e..ecd783e1d52 100644 --- a/test/form/samples/object-literal-property-overwrites/_expected.js +++ b/test/form/samples/object-literal-property-overwrites/_expected.js @@ -1,23 +1,3 @@ -const removed1 = { - foo: () => {}, - foo: () => {}, - ['f' + 'oo']: () => {} -}; -removed1.foo(); - -const removed2 = { - foo: () => console.log( 'effect' ), - foo: () => {} -}; -removed2.foo(); - -const removed3 = { - ['fo' + 'o']: function () {this.x = 1;}, - ['f' + 'oo']: () => console.log( 'effect' ), - foo: () => {} -}; -removed3.foo(); - const retained1 = { foo: () => {}, foo: function () {this.x = 1;} diff --git a/test/form/samples/side-effects-object-literal-calls/_expected.js b/test/form/samples/side-effects-object-literal-calls/_expected.js index 5f7c1d32a3d..fc8dcbfe778 100644 --- a/test/form/samples/side-effects-object-literal-calls/_expected.js +++ b/test/form/samples/side-effects-object-literal-calls/_expected.js @@ -1,9 +1,3 @@ -const removed1 = { x: () => {} }; -removed1.x(); - -const removed2 = { x: { y: () => {} } }; -removed2.x.y(); - const retained1 = { x: () => {} }; retained1.y();