Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move long path recursion prevention to MemberExpression #4189

Merged
merged 2 commits into from Jul 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 49 additions & 25 deletions src/ast/nodes/MemberExpression.ts
Expand Up @@ -34,6 +34,9 @@ import {
} from './shared/Expression';
import { ExpressionNode, IncludeChildren, NodeBase } from './shared/Node';

// To avoid infinite recursions
const MAX_PATH_DEPTH = 7;

function getResolvablePropertyKey(memberExpression: MemberExpression): string | null {
return memberExpression.computed
? getResolvableComputedPropertyKey(memberExpression.property)
Expand Down Expand Up @@ -124,7 +127,9 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.variable) {
this.variable.deoptimizePath(path);
} else if (!this.replacement) {
this.object.deoptimizePath([this.getPropertyKey(), ...path]);
if (path.length < MAX_PATH_DEPTH) {
this.object.deoptimizePath([this.getPropertyKey(), ...path]);
}
}
}

Expand All @@ -137,12 +142,16 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.variable) {
this.variable.deoptimizeThisOnEventAtPath(event, path, thisParameter, recursionTracker);
} else if (!this.replacement) {
this.object.deoptimizeThisOnEventAtPath(
event,
[this.getPropertyKey(), ...path],
thisParameter,
recursionTracker
);
if (path.length < MAX_PATH_DEPTH) {
this.object.deoptimizeThisOnEventAtPath(
event,
[this.getPropertyKey(), ...path],
thisParameter,
recursionTracker
);
} else {
thisParameter.deoptimizePath(UNKNOWN_PATH);
}
}
}

Expand All @@ -158,11 +167,14 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
return UnknownValue;
}
this.expressionsToBeDeoptimized.push(origin);
return this.object.getLiteralValueAtPath(
[this.getPropertyKey(), ...path],
recursionTracker,
origin
);
if (path.length < MAX_PATH_DEPTH) {
return this.object.getLiteralValueAtPath(
[this.getPropertyKey(), ...path],
recursionTracker,
origin
);
}
return UnknownValue;
}

getReturnExpressionWhenCalledAtPath(
Expand All @@ -183,12 +195,15 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
return UNKNOWN_EXPRESSION;
}
this.expressionsToBeDeoptimized.push(origin);
return this.object.getReturnExpressionWhenCalledAtPath(
[this.getPropertyKey(), ...path],
callOptions,
recursionTracker,
origin
);
if (path.length < MAX_PATH_DEPTH) {
return this.object.getReturnExpressionWhenCalledAtPath(
[this.getPropertyKey(), ...path],
callOptions,
recursionTracker,
origin
);
}
return UNKNOWN_EXPRESSION;
}

hasEffects(context: HasEffectsContext): boolean {
Expand Down Expand Up @@ -217,7 +232,10 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.replacement) {
return true;
}
return this.object.hasEffectsWhenAccessedAtPath([this.getPropertyKey(), ...path], context);
if (path.length < MAX_PATH_DEPTH) {
return this.object.hasEffectsWhenAccessedAtPath([this.getPropertyKey(), ...path], context);
}
return true;
}

hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
Expand All @@ -227,7 +245,10 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.replacement) {
return true;
}
return this.object.hasEffectsWhenAssignedAtPath([this.getPropertyKey(), ...path], context);
if (path.length < MAX_PATH_DEPTH) {
return this.object.hasEffectsWhenAssignedAtPath([this.getPropertyKey(), ...path], context);
}
return true;
}

hasEffectsWhenCalledAtPath(
Expand All @@ -241,11 +262,14 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
if (this.replacement) {
return true;
}
return this.object.hasEffectsWhenCalledAtPath(
[this.getPropertyKey(), ...path],
callOptions,
context
);
if (path.length < MAX_PATH_DEPTH) {
return this.object.hasEffectsWhenCalledAtPath(
[this.getPropertyKey(), ...path],
callOptions,
context
);
}
return true;
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
Expand Down
7 changes: 6 additions & 1 deletion src/ast/nodes/shared/ObjectMember.ts
Expand Up @@ -20,7 +20,12 @@ export class ObjectMember extends ExpressionEntity {
thisParameter: ExpressionEntity,
recursionTracker: PathTracker
): void {
this.object.deoptimizeThisOnEventAtPath(event, path, thisParameter, recursionTracker);
this.object.deoptimizeThisOnEventAtPath(
event,
[this.key, ...path],
thisParameter,
recursionTracker
);
}

getLiteralValueAtPath(
Expand Down
16 changes: 6 additions & 10 deletions src/ast/variables/LocalVariable.ts
Expand Up @@ -17,9 +17,6 @@ import { ExpressionNode, Node } from '../nodes/shared/Node';
import { ObjectPath, PathTracker, UNKNOWN_PATH } from '../utils/PathTracker';
import Variable from './Variable';

// To avoid infinite recursions
const MAX_PATH_DEPTH = 7;

export default class LocalVariable extends Variable {
calledFromTryStatement = false;
declarations: (Identifier | ExportDefaultDeclaration)[];
Expand Down Expand Up @@ -64,7 +61,6 @@ export default class LocalVariable extends Variable {

deoptimizePath(path: ObjectPath): void {
if (
path.length > MAX_PATH_DEPTH ||
this.isReassigned ||
this.deoptimizationTracker.trackEntityAtPathAndGetIfTracked(path, this)
) {
Expand All @@ -91,7 +87,7 @@ export default class LocalVariable extends Variable {
thisParameter: ExpressionEntity,
recursionTracker: PathTracker
): void {
if (this.isReassigned || !this.init || path.length > MAX_PATH_DEPTH) {
if (this.isReassigned || !this.init) {
return thisParameter.deoptimizePath(UNKNOWN_PATH);
}
recursionTracker.withTrackedEntityAtPath(
Expand All @@ -107,7 +103,7 @@ export default class LocalVariable extends Variable {
recursionTracker: PathTracker,
origin: DeoptimizableEntity
): LiteralValueOrUnknown {
if (this.isReassigned || !this.init || path.length > MAX_PATH_DEPTH) {
if (this.isReassigned || !this.init) {
return UnknownValue;
}
return recursionTracker.withTrackedEntityAtPath(
Expand All @@ -127,7 +123,7 @@ export default class LocalVariable extends Variable {
recursionTracker: PathTracker,
origin: DeoptimizableEntity
): ExpressionEntity {
if (this.isReassigned || !this.init || path.length > MAX_PATH_DEPTH) {
if (this.isReassigned || !this.init) {
return UNKNOWN_EXPRESSION;
}
return recursionTracker.withTrackedEntityAtPath(
Expand All @@ -147,14 +143,14 @@ export default class LocalVariable extends Variable {
}

hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
if (this.isReassigned || path.length > MAX_PATH_DEPTH) return true;
if (this.isReassigned) return true;
return (this.init &&
!context.accessed.trackEntityAtPathAndGetIfTracked(path, this) &&
this.init.hasEffectsWhenAccessedAtPath(path, context))!;
}

hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
if (this.included || path.length > MAX_PATH_DEPTH) return true;
if (this.included) return true;
if (path.length === 0) return false;
if (this.isReassigned) return true;
return (this.init &&
Expand All @@ -167,7 +163,7 @@ export default class LocalVariable extends Variable {
callOptions: CallOptions,
context: HasEffectsContext
): boolean {
if (path.length > MAX_PATH_DEPTH || this.isReassigned) return true;
if (this.isReassigned) return true;
return (this.init &&
!(
callOptions.withNew ? context.instantiated : context.called
Expand Down
3 changes: 3 additions & 0 deletions test/form/samples/deep-properties-access/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'handles deeply nested property accesses'
};
2 changes: 2 additions & 0 deletions test/form/samples/deep-properties-access/_expected.js
@@ -0,0 +1,2 @@
var obj = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
console.log(obj.obj.obj.obj.obj.obj.obj.obj.obj.foo);
2 changes: 2 additions & 0 deletions test/form/samples/deep-properties-access/main.js
@@ -0,0 +1,2 @@
var obj = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
console.log(obj.obj.obj.obj.obj.obj.obj.obj.obj.foo);
6 changes: 6 additions & 0 deletions test/form/samples/deep-properties/_config.js
@@ -0,0 +1,6 @@
module.exports = {
description: 'handles deeply nested properties',
options: {
treeshake: { propertyReadSideEffects: false }
}
};
11 changes: 11 additions & 0 deletions test/form/samples/deep-properties/_expected.js
@@ -0,0 +1,11 @@
var obj1 = obj1;
console.log(obj1.foo());

var obj2 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
obj2.obj.obj.obj.obj.obj.obj.obj.foo();

var obj3 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
if (obj3.obj.obj.obj.obj.obj.obj.obj.obj.foo) console.log('nested');

var obj4 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
obj4.obj.obj.obj.obj.obj.obj.obj.foo = 'nested';
11 changes: 11 additions & 0 deletions test/form/samples/deep-properties/main.js
@@ -0,0 +1,11 @@
var obj1 = obj1;
console.log(obj1.foo());

var obj2 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
obj2.obj.obj.obj.obj.obj.obj.obj.foo();

var obj3 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
if (obj3.obj.obj.obj.obj.obj.obj.obj.obj.foo) console.log('nested');

var obj4 = { obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {obj: {}}}}}}}}}}};
obj4.obj.obj.obj.obj.obj.obj.obj.foo = 'nested';
3 changes: 0 additions & 3 deletions test/form/samples/recursive-property-call/_config.js

This file was deleted.

9 changes: 0 additions & 9 deletions test/form/samples/recursive-property-call/_expected.js

This file was deleted.

9 changes: 0 additions & 9 deletions test/form/samples/recursive-property-call/main.js

This file was deleted.

3 changes: 3 additions & 0 deletions test/form/samples/static-method-deoptimization/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'avoids infinite recursions when deoptimizing "this" context'
};
13 changes: 13 additions & 0 deletions test/form/samples/static-method-deoptimization/_expected.js
@@ -0,0 +1,13 @@
class Foo {
static echo(message) {
this.prototype.echo(message);
}
echo(message) {
console.log(message);
}
}

class Bar extends Foo {}

global.baz = 'PASS';
Bar.echo(baz);
13 changes: 13 additions & 0 deletions test/form/samples/static-method-deoptimization/main.js
@@ -0,0 +1,13 @@
class Foo {
static echo(message) {
this.prototype.echo(message);
}
echo(message) {
console.log(message);
}
}

class Bar extends Foo {}

global.baz = 'PASS';
Bar.echo(baz);