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

Ensure arguments are deoptimized when arguments variable is used #4965

Merged
merged 1 commit into from Apr 29, 2023
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
5 changes: 5 additions & 0 deletions src/ast/nodes/shared/FunctionBase.ts
Expand Up @@ -54,8 +54,11 @@ export default abstract class FunctionBase extends NodeBase {
argument.deoptimizePath(UNKNOWN_PATH);
} else if (parameter instanceof Identifier) {
parameters[position][0].addEntityToBeDeoptimized(argument);
this.addArgumentToBeDeoptimized(argument);
} else if (parameter) {
argument.deoptimizePath(UNKNOWN_PATH);
} else {
this.addArgumentToBeDeoptimized(argument);
}
}
} else {
Expand Down Expand Up @@ -181,6 +184,8 @@ export default abstract class FunctionBase extends NodeBase {
super.parseNode(esTreeNode);
}

protected addArgumentToBeDeoptimized(_argument: ExpressionEntity) {}

protected applyDeoptimizations() {}

protected abstract getObjectEntity(): ObjectEntity;
Expand Down
5 changes: 5 additions & 0 deletions src/ast/nodes/shared/FunctionNode.ts
Expand Up @@ -5,6 +5,7 @@ import FunctionScope from '../../scopes/FunctionScope';
import type { ObjectPath, PathTracker } from '../../utils/PathTracker';
import type BlockStatement from '../BlockStatement';
import Identifier, { type IdentifierWithVariable } from '../Identifier';
import type { ExpressionEntity } from './Expression';
import { UNKNOWN_EXPRESSION } from './Expression';
import FunctionBase from './FunctionBase';
import { type IncludeChildren } from './Node';
Expand Down Expand Up @@ -95,6 +96,10 @@ export default class FunctionNode extends FunctionBase {
this.id?.declare('function', this);
}

protected addArgumentToBeDeoptimized(argument: ExpressionEntity) {
this.scope.argumentsVariable.addArgumentToBeDeoptimized(argument);
}

protected getObjectEntity(): ObjectEntity {
if (this.objectEntity !== null) {
return this.objectEntity;
Expand Down
20 changes: 20 additions & 0 deletions src/ast/variables/ArgumentsVariable.ts
@@ -1,16 +1,36 @@
import type { AstContext } from '../../Module';
import type { NodeInteraction } from '../NodeInteractions';
import { INTERACTION_ACCESSED } from '../NodeInteractions';
import type { ExpressionEntity } from '../nodes/shared/Expression';
import { UNKNOWN_EXPRESSION } from '../nodes/shared/Expression';
import type { ObjectPath } from '../utils/PathTracker';
import { UNKNOWN_PATH } from '../utils/PathTracker';
import LocalVariable from './LocalVariable';

export default class ArgumentsVariable extends LocalVariable {
private deoptimizedArguments: ExpressionEntity[] = [];

constructor(context: AstContext) {
super('arguments', null, UNKNOWN_EXPRESSION, context);
}

addArgumentToBeDeoptimized(argument: ExpressionEntity): void {
if (this.included) {
argument.deoptimizePath(UNKNOWN_PATH);
} else {
this.deoptimizedArguments.push(argument);
}
}

hasEffectsOnInteractionAtPath(path: ObjectPath, { type }: NodeInteraction): boolean {
return type !== INTERACTION_ACCESSED || path.length > 1;
}

include() {
super.include();
for (const argument of this.deoptimizedArguments) {
argument.deoptimizePath(UNKNOWN_PATH);
}
this.deoptimizedArguments.length = 0;
}
}
3 changes: 3 additions & 0 deletions test/function/samples/deoptimize-via-arguments/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'deoptimize call parameters when arguments variable is used'
};
17 changes: 17 additions & 0 deletions test/function/samples/deoptimize-via-arguments/main.js
@@ -0,0 +1,17 @@
function mutate(a) {
arguments[0].x = true;
arguments[1].x = true;
}

var obj1 = {
x: false
};

var obj2 = {
x: false
};

mutate(obj1, obj2);

assert.ok(obj1.x ? true : false);
assert.ok(obj2.x ? true : false);