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

Refactor side effect handling for property interactions #4522

Merged
merged 13 commits into from Jun 7, 2022
Merged
10 changes: 0 additions & 10 deletions src/ast/CallOptions.ts

This file was deleted.

7 changes: 6 additions & 1 deletion src/ast/Entity.ts
@@ -1,4 +1,5 @@
import type { HasEffectsContext } from './ExecutionContext';
import { NodeInteractionAssigned } from './NodeInteractions';
import type { ObjectPath } from './utils/PathTracker';

// eslint-disable-next-line @typescript-eslint/no-empty-interface
Expand All @@ -13,5 +14,9 @@ export interface WritableEntity extends Entity {
*/
deoptimizePath(path: ObjectPath): void;

hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
hasEffectsOnInteractionAtPath(
path: ObjectPath,
interaction: NodeInteractionAssigned,
context: HasEffectsContext
): boolean;
}
5 changes: 0 additions & 5 deletions src/ast/NodeEvents.ts

This file was deleted.

56 changes: 56 additions & 0 deletions src/ast/NodeInteractions.ts
@@ -0,0 +1,56 @@
import SpreadElement from './nodes/SpreadElement';
import { ExpressionEntity, UNKNOWN_EXPRESSION } from './nodes/shared/Expression';

export const INTERACTION_ACCESSED = 0;
export const INTERACTION_ASSIGNED = 1;
export const INTERACTION_CALLED = 2;

export interface NodeInteractionAccessed {
thisArg: ExpressionEntity | null;
type: typeof INTERACTION_ACCESSED;
}

export const NODE_INTERACTION_UNKNOWN_ACCESS: NodeInteractionAccessed = {
thisArg: null,
type: INTERACTION_ACCESSED
};

export interface NodeInteractionAssigned {
args: readonly [ExpressionEntity];
thisArg: ExpressionEntity | null;
type: typeof INTERACTION_ASSIGNED;
}

export const UNKNOWN_ARG = [UNKNOWN_EXPRESSION] as const;

export const NODE_INTERACTION_UNKNOWN_ASSIGNMENT: NodeInteractionAssigned = {
args: UNKNOWN_ARG,
thisArg: null,
type: INTERACTION_ASSIGNED
};

export interface NodeInteractionCalled {
args: readonly (ExpressionEntity | SpreadElement)[];
thisArg: ExpressionEntity | null;
type: typeof INTERACTION_CALLED;
withNew: boolean;
}

export const NO_ARGS = [];

// While this is technically a call without arguments, we can compare against
// this reference in places where precise values or thisArg would make a
// difference
export const NODE_INTERACTION_UNKNOWN_CALL: NodeInteractionCalled = {
args: NO_ARGS,
thisArg: null,
type: INTERACTION_CALLED,
withNew: false
};

export type NodeInteraction =
| NodeInteractionAccessed
| NodeInteractionAssigned
| NodeInteractionCalled;

export type NodeInteractionWithThisArg = NodeInteraction & { thisArg: ExpressionEntity };
34 changes: 10 additions & 24 deletions src/ast/nodes/ArrayExpression.ts
@@ -1,7 +1,7 @@
import type { CallOptions } from '../CallOptions';
import type { DeoptimizableEntity } from '../DeoptimizableEntity';
import type { HasEffectsContext } from '../ExecutionContext';
import type { NodeEvent } from '../NodeEvents';
import type { NodeInteractionCalled, NodeInteractionWithThisArg } from '../NodeInteractions';
import { NodeInteraction } from '../NodeInteractions';
import {
type ObjectPath,
type PathTracker,
Expand All @@ -25,18 +25,12 @@ export default class ArrayExpression extends NodeBase {
this.getObjectEntity().deoptimizePath(path);
}

deoptimizeThisOnEventAtPath(
event: NodeEvent,
deoptimizeThisOnInteractionAtPath(
interaction: NodeInteractionWithThisArg,
path: ObjectPath,
thisParameter: ExpressionEntity,
recursionTracker: PathTracker
): void {
this.getObjectEntity().deoptimizeThisOnEventAtPath(
event,
path,
thisParameter,
recursionTracker
);
this.getObjectEntity().deoptimizeThisOnInteractionAtPath(interaction, path, recursionTracker);
}

getLiteralValueAtPath(
Expand All @@ -49,32 +43,24 @@ export default class ArrayExpression extends NodeBase {

getReturnExpressionWhenCalledAtPath(
path: ObjectPath,
callOptions: CallOptions,
interaction: NodeInteractionCalled,
recursionTracker: PathTracker,
origin: DeoptimizableEntity
): ExpressionEntity {
return this.getObjectEntity().getReturnExpressionWhenCalledAtPath(
path,
callOptions,
interaction,
recursionTracker,
origin
);
}

hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
return this.getObjectEntity().hasEffectsWhenAccessedAtPath(path, context);
}

hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
}

hasEffectsWhenCalledAtPath(
hasEffectsOnInteractionAtPath(
path: ObjectPath,
callOptions: CallOptions,
interaction: NodeInteraction,
context: HasEffectsContext
): boolean {
return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
return this.getObjectEntity().hasEffectsOnInteractionAtPath(path, interaction, context);
}

protected applyDeoptimizations(): void {
Expand Down
9 changes: 7 additions & 2 deletions src/ast/nodes/ArrayPattern.ts
@@ -1,4 +1,5 @@
import type { HasEffectsContext } from '../ExecutionContext';
import { NodeInteractionAssigned } from '../NodeInteractions';
import { EMPTY_PATH, type ObjectPath } from '../utils/PathTracker';
import type LocalVariable from '../variables/LocalVariable';
import type Variable from '../variables/Variable';
Expand Down Expand Up @@ -38,9 +39,13 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
}

// Patterns are only checked at the emtpy path at the moment
hasEffectsWhenAssignedAtPath(_path: ObjectPath, context: HasEffectsContext): boolean {
hasEffectsOnInteractionAtPath(
_path: ObjectPath,
interaction: NodeInteractionAssigned,
context: HasEffectsContext
): boolean {
for (const element of this.elements) {
if (element?.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)) return true;
if (element?.hasEffectsOnInteractionAtPath(EMPTY_PATH, interaction, context)) return true;
}
return false;
}
Expand Down
30 changes: 16 additions & 14 deletions src/ast/nodes/ArrowFunctionExpression.ts
@@ -1,5 +1,5 @@
import { type CallOptions } from '../CallOptions';
import { type HasEffectsContext, InclusionContext } from '../ExecutionContext';
import { INTERACTION_CALLED, NodeInteraction } from '../NodeInteractions';
import ReturnValueScope from '../scopes/ReturnValueScope';
import type Scope from '../scopes/Scope';
import { type ObjectPath } from '../utils/PathTracker';
Expand Down Expand Up @@ -30,22 +30,24 @@ export default class ArrowFunctionExpression extends FunctionBase {
return false;
}

hasEffectsWhenCalledAtPath(
hasEffectsOnInteractionAtPath(
path: ObjectPath,
callOptions: CallOptions,
interaction: NodeInteraction,
context: HasEffectsContext
): boolean {
if (super.hasEffectsWhenCalledAtPath(path, callOptions, context)) return true;
const { ignore, brokenFlow } = context;
context.ignore = {
breaks: false,
continues: false,
labels: new Set(),
returnYield: true
};
if (this.body.hasEffects(context)) return true;
context.ignore = ignore;
context.brokenFlow = brokenFlow;
if (super.hasEffectsOnInteractionAtPath(path, interaction, context)) return true;
if (interaction.type === INTERACTION_CALLED) {
const { ignore, brokenFlow } = context;
context.ignore = {
breaks: false,
continues: false,
labels: new Set(),
returnYield: true
};
if (this.body.hasEffects(context)) return true;
context.ignore = ignore;
context.brokenFlow = brokenFlow;
}
return false;
}

Expand Down
83 changes: 46 additions & 37 deletions src/ast/nodes/AssignmentExpression.ts
Expand Up @@ -17,6 +17,7 @@ import {
type HasEffectsContext,
type InclusionContext
} from '../ExecutionContext';
import { NodeInteraction } from '../NodeInteractions';
import { EMPTY_PATH, type ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
import type Variable from '../variables/Variable';
import Identifier from './Identifier';
Expand Down Expand Up @@ -45,70 +46,78 @@ export default class AssignmentExpression extends NodeBase {
declare type: NodeType.tAssignmentExpression;

hasEffects(context: HasEffectsContext): boolean {
if (!this.deoptimized) this.applyDeoptimizations();
const { deoptimized, left, right } = this;
if (!deoptimized) this.applyDeoptimizations();
// MemberExpressions do not access the property before assignments if the
// operator is '='.
return (
this.right.hasEffects(context) ||
this.left.hasEffects(context) ||
this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)
right.hasEffects(context) || left.hasEffectsAsAssignmentTarget(context, this.operator !== '=')
);
}

hasEffectsWhenAccessedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context);
hasEffectsOnInteractionAtPath(
path: ObjectPath,
interaction: NodeInteraction,
context: HasEffectsContext
): boolean {
return this.right.hasEffectsOnInteractionAtPath(path, interaction, context);
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
if (!this.deoptimized) this.applyDeoptimizations();
const { deoptimized, left, right, operator } = this;
if (!deoptimized) this.applyDeoptimizations();
this.included = true;
let hasEffectsContext;
if (
includeChildrenRecursively ||
this.operator !== '=' ||
this.left.included ||
((hasEffectsContext = createHasEffectsContext()),
this.left.hasEffects(hasEffectsContext) ||
this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, hasEffectsContext))
operator !== '=' ||
left.included ||
left.hasEffectsAsAssignmentTarget(createHasEffectsContext(), false)
) {
this.left.include(context, includeChildrenRecursively);
left.includeAsAssignmentTarget(context, includeChildrenRecursively, operator !== '=');
}
this.right.include(context, includeChildrenRecursively);
right.include(context, includeChildrenRecursively);
}

initialise(): void {
this.left.setAssignedValue(this.right);
}

render(
code: MagicString,
options: RenderOptions,
{ preventASI, renderedParentType, renderedSurroundingElement }: NodeRenderOptions = BLANK
): void {
if (this.left.included) {
this.left.render(code, options);
this.right.render(code, options);
const { left, right, start, end, parent } = this;
if (left.included) {
left.render(code, options);
right.render(code, options);
} else {
const inclusionStart = findNonWhiteSpace(
code.original,
findFirstOccurrenceOutsideComment(code.original, '=', this.left.end) + 1
findFirstOccurrenceOutsideComment(code.original, '=', left.end) + 1
);
code.remove(this.start, inclusionStart);
code.remove(start, inclusionStart);
if (preventASI) {
removeLineBreaks(code, inclusionStart, this.right.start);
removeLineBreaks(code, inclusionStart, right.start);
}
this.right.render(code, options, {
renderedParentType: renderedParentType || this.parent.type,
renderedSurroundingElement: renderedSurroundingElement || this.parent.type
right.render(code, options, {
renderedParentType: renderedParentType || parent.type,
renderedSurroundingElement: renderedSurroundingElement || parent.type
});
}
if (options.format === 'system') {
if (this.left instanceof Identifier) {
const variable = this.left.variable!;
if (left instanceof Identifier) {
const variable = left.variable!;
const exportNames = options.exportNamesByVariable.get(variable);
if (exportNames) {
if (exportNames.length === 1) {
renderSystemExportExpression(variable, this.start, this.end, code, options);
renderSystemExportExpression(variable, start, end, code, options);
} else {
renderSystemExportSequenceAfterExpression(
variable,
this.start,
this.end,
this.parent.type !== NodeType.ExpressionStatement,
start,
end,
parent.type !== NodeType.ExpressionStatement,
code,
options
);
Expand All @@ -117,12 +126,12 @@ export default class AssignmentExpression extends NodeBase {
}
} else {
const systemPatternExports: Variable[] = [];
this.left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
left.addExportedVariables(systemPatternExports, options.exportNamesByVariable);
if (systemPatternExports.length > 0) {
renderSystemExportFunction(
systemPatternExports,
this.start,
this.end,
start,
end,
renderedSurroundingElement === NodeType.ExpressionStatement,
code,
options
Expand All @@ -132,13 +141,13 @@ export default class AssignmentExpression extends NodeBase {
}
}
if (
this.left.included &&
this.left instanceof ObjectPattern &&
left.included &&
left instanceof ObjectPattern &&
(renderedSurroundingElement === NodeType.ExpressionStatement ||
renderedSurroundingElement === NodeType.ArrowFunctionExpression)
) {
code.appendRight(this.start, '(');
code.prependLeft(this.end, ')');
code.appendRight(start, '(');
code.prependLeft(end, ')');
}
}

Expand Down