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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roll back parameter default tree shaking #4520

Merged
merged 7 commits into from Jun 1, 2022
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
4 changes: 0 additions & 4 deletions src/ast/nodes/ArrayExpression.ts
Expand Up @@ -85,11 +85,7 @@ export default class ArrayExpression extends NodeBase {
if (element) {
if (hasSpread || element instanceof SpreadElement) {
hasSpread = true;
// This also deoptimizes parameter defaults
element.deoptimizePath(UNKNOWN_PATH);
} else {
// We do not track parameter defaults in arrays
element.deoptimizeCallParameters();
}
}
}
Expand Down
14 changes: 12 additions & 2 deletions src/ast/nodes/ArrowFunctionExpression.ts
@@ -1,12 +1,13 @@
import { type CallOptions } from '../CallOptions';
import { type HasEffectsContext } from '../ExecutionContext';
import { type HasEffectsContext, InclusionContext } from '../ExecutionContext';
import ReturnValueScope from '../scopes/ReturnValueScope';
import type Scope from '../scopes/Scope';
import { type ObjectPath } from '../utils/PathTracker';
import BlockStatement from './BlockStatement';
import Identifier from './Identifier';
import * as NodeType from './NodeType';
import FunctionBase from './shared/FunctionBase';
import { type ExpressionNode } from './shared/Node';
import { type ExpressionNode, IncludeChildren } from './shared/Node';
import { ObjectEntity } from './shared/ObjectEntity';
import { OBJECT_PROTOTYPE } from './shared/ObjectPrototype';
import type { PatternNode } from './shared/Pattern';
Expand Down Expand Up @@ -48,6 +49,15 @@ export default class ArrowFunctionExpression extends FunctionBase {
return false;
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
super.include(context, includeChildrenRecursively);
for (const param of this.params) {
if (!(param instanceof Identifier)) {
param.include(context, includeChildrenRecursively);
}
}
}

protected getObjectEntity(): ObjectEntity {
if (this.objectEntity !== null) {
return this.objectEntity;
Expand Down
4 changes: 1 addition & 3 deletions src/ast/nodes/AssignmentPattern.ts
Expand Up @@ -35,10 +35,8 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
return path.length > 0 || this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context);
}

// Note that FunctionBase may directly include .left and .right without
// including the pattern itself. This is how default parameter tree-shaking
// works at the moment.
include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
if (!this.deoptimized) this.applyDeoptimizations();
this.included = true;
this.left.include(context, includeChildrenRecursively);
this.right.include(context, includeChildrenRecursively);
Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/CallExpression.ts
Expand Up @@ -95,7 +95,7 @@ export default class CallExpression extends CallExpressionBase implements Deopti
}
} else {
this.included = true;
this.callee.include(context, false, { includeWithoutParameterDefaults: true });
this.callee.include(context, false);
}
this.callee.includeCallArguments(context, this.arguments);
const returnExpression = this.getReturnExpression();
Expand Down
7 changes: 1 addition & 6 deletions src/ast/nodes/FunctionDeclaration.ts
@@ -1,17 +1,12 @@
import { InclusionContext } from '../ExecutionContext';
import type ChildScope from '../scopes/ChildScope';
import Identifier, { type IdentifierWithVariable } from './Identifier';
import type * as NodeType from './NodeType';
import FunctionNode from './shared/FunctionNode';
import type { GenericEsTreeNode, IncludeChildren } from './shared/Node';
import type { GenericEsTreeNode } from './shared/Node';

export default class FunctionDeclaration extends FunctionNode {
declare type: NodeType.tFunctionDeclaration;

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
super.include(context, includeChildrenRecursively, { includeWithoutParameterDefaults: true });
}

initialise(): void {
super.initialise();
if (this.id !== null) {
Expand Down
4 changes: 0 additions & 4 deletions src/ast/nodes/Identifier.ts
Expand Up @@ -86,10 +86,6 @@ export default class Identifier extends NodeBase implements PatternNode {
return [(this.variable = variable)];
}

deoptimizeCallParameters() {
this.variable!.deoptimizeCallParameters();
}

deoptimizePath(path: ObjectPath): void {
if (path.length === 0 && !this.scope.contains(this.name)) {
this.disallowImportReassignment();
Expand Down
9 changes: 1 addition & 8 deletions src/ast/nodes/ObjectExpression.ts
Expand Up @@ -102,14 +102,7 @@ export default class ObjectExpression extends NodeBase implements DeoptimizableE
}
}

protected applyDeoptimizations() {
this.deoptimized = true;
for (const property of this.properties) {
if (property instanceof Property) {
property.value.deoptimizeCallParameters();
}
}
}
protected applyDeoptimizations() {}

private getObjectEntity(): ObjectEntity {
if (this.objectEntity !== null) {
Expand Down
4 changes: 1 addition & 3 deletions src/ast/nodes/VariableDeclarator.ts
Expand Up @@ -35,9 +35,7 @@ export default class VariableDeclarator extends NodeBase {

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
this.included = true;
this.init?.include(context, includeChildrenRecursively, {
includeWithoutParameterDefaults: true
});
this.init?.include(context, includeChildrenRecursively);
this.id.markDeclarationReached();
if (includeChildrenRecursively || this.id.shouldBeIncluded(context)) {
this.id.include(context, includeChildrenRecursively);
Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/YieldExpression.ts
Expand Up @@ -11,7 +11,7 @@ export default class YieldExpression extends NodeBase {

hasEffects(context: HasEffectsContext): boolean {
if (!this.deoptimized) this.applyDeoptimizations();
return !context.ignore.returnYield || !!this.argument?.hasEffects(context);
return !(context.ignore.returnYield && !this.argument?.hasEffects(context));
}

render(code: MagicString, options: RenderOptions): void {
Expand Down
10 changes: 0 additions & 10 deletions src/ast/nodes/shared/ClassNode.ts
Expand Up @@ -16,7 +16,6 @@ import type ClassBody from '../ClassBody';
import Identifier from '../Identifier';
import type Literal from '../Literal';
import MethodDefinition from '../MethodDefinition';
import PropertyDefinition from '../PropertyDefinition';
import { type ExpressionEntity, type LiteralValueOrUnknown } from './Expression';
import { type ExpressionNode, type IncludeChildren, NodeBase } from './Node';
import { ObjectEntity, type ObjectProperty } from './ObjectEntity';
Expand All @@ -40,12 +39,6 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {

deoptimizePath(path: ObjectPath): void {
this.getObjectEntity().deoptimizePath(path);
if (path.length === 1 && path[0] === UnknownKey) {
// A reassignment of UNKNOWN_PATH is considered equivalent to having lost track
// which means the constructor needs to be reassigned
this.classConstructor?.deoptimizePath(UNKNOWN_PATH);
this.superClass?.deoptimizePath(UNKNOWN_PATH);
}
}

deoptimizeThisOnEventAtPath(
Expand Down Expand Up @@ -150,11 +143,8 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity {
) {
// Calls to methods are not tracked, ensure that the return value is deoptimized
definition.deoptimizePath(UNKNOWN_PATH);
} else if (definition instanceof PropertyDefinition) {
definition.value?.deoptimizeCallParameters();
}
}
this.superClass?.deoptimizeCallParameters();
this.context.requestTreeshakingPass();
}

Expand Down
3 changes: 0 additions & 3 deletions src/ast/nodes/shared/Expression.ts
Expand Up @@ -18,14 +18,11 @@ export interface InclusionOptions {
* Include the id of a declarator even if unused to ensure it is a valid statement.
*/
asSingleStatement?: boolean;
includeWithoutParameterDefaults?: boolean;
}

export class ExpressionEntity implements WritableEntity {
included = false;

deoptimizeCallParameters(): void {}

deoptimizePath(_path: ObjectPath): void {}

deoptimizeThisOnEventAtPath(
Expand Down
109 changes: 8 additions & 101 deletions src/ast/nodes/shared/FunctionBase.ts
@@ -1,5 +1,4 @@
import type { NormalizedTreeshakingOptions } from '../../../rollup/types';
import { BLANK } from '../../../utils/blank';
import { type CallOptions, NO_ARGS } from '../../CallOptions';
import { DeoptimizableEntity } from '../../DeoptimizableEntity';
import {
Expand All @@ -9,27 +8,12 @@ import {
} from '../../ExecutionContext';
import { NodeEvent } from '../../NodeEvents';
import ReturnValueScope from '../../scopes/ReturnValueScope';
import {
EMPTY_PATH,
type ObjectPath,
PathTracker,
SHARED_RECURSION_TRACKER,
UNKNOWN_PATH,
UnknownKey
} from '../../utils/PathTracker';
import LocalVariable from '../../variables/LocalVariable';
import AssignmentPattern from '../AssignmentPattern';
import { type ObjectPath, PathTracker, UNKNOWN_PATH, UnknownKey } from '../../utils/PathTracker';
import BlockStatement from '../BlockStatement';
import * as NodeType from '../NodeType';
import RestElement from '../RestElement';
import type SpreadElement from '../SpreadElement';
import {
type ExpressionEntity,
InclusionOptions,
LiteralValueOrUnknown,
UNKNOWN_EXPRESSION,
UnknownValue
} from './Expression';
import { type ExpressionEntity, LiteralValueOrUnknown, UNKNOWN_EXPRESSION } from './Expression';
import {
type ExpressionNode,
type GenericEsTreeNode,
Expand All @@ -39,31 +23,20 @@ import {
import { ObjectEntity } from './ObjectEntity';
import type { PatternNode } from './Pattern';

export default abstract class FunctionBase extends NodeBase implements DeoptimizableEntity {
export default abstract class FunctionBase extends NodeBase {
declare async: boolean;
declare body: BlockStatement | ExpressionNode;
declare params: readonly PatternNode[];
declare preventChildBlockScope: true;
declare scope: ReturnValueScope;
protected objectEntity: ObjectEntity | null = null;
private deoptimizedReturn = false;
private forceIncludeParameters = false;
private declare parameterVariables: LocalVariable[][];

deoptimizeCache() {
this.forceIncludeParameters = true;
}

deoptimizeCallParameters() {
this.forceIncludeParameters = true;
}

deoptimizePath(path: ObjectPath): void {
this.getObjectEntity().deoptimizePath(path);
if (path.length === 1 && path[0] === UnknownKey) {
// A reassignment of UNKNOWN_PATH is considered equivalent to having lost track
// which means the return expression needs to be reassigned
this.forceIncludeParameters = true;
this.scope.getReturnExpression().deoptimizePath(UNKNOWN_PATH);
}
}
Expand Down Expand Up @@ -150,89 +123,31 @@ export default abstract class FunctionBase extends NodeBase implements Deoptimiz
return true;
}
}
for (let position = 0; position < this.params.length; position++) {
const parameter = this.params[position];
if (parameter instanceof AssignmentPattern) {
if (parameter.left.hasEffects(context)) {
return true;
}
const argumentValue = callOptions.args[position]?.getLiteralValueAtPath(
EMPTY_PATH,
SHARED_RECURSION_TRACKER,
this
);
if (
(argumentValue === undefined || argumentValue === UnknownValue) &&
parameter.right.hasEffects(context)
) {
return true;
}
} else if (parameter.hasEffects(context)) {
return true;
}
for (const param of this.params) {
if (param.hasEffects(context)) return true;
}
return false;
}

include(
context: InclusionContext,
includeChildrenRecursively: IncludeChildren,
{ includeWithoutParameterDefaults }: InclusionOptions = BLANK
): void {
include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
if (!this.deoptimized) this.applyDeoptimizations();
this.included = true;
const { brokenFlow } = context;
context.brokenFlow = BROKEN_FLOW_NONE;
this.body.include(context, includeChildrenRecursively);
context.brokenFlow = brokenFlow;
if (
!includeWithoutParameterDefaults ||
includeChildrenRecursively ||
this.forceIncludeParameters
) {
for (const param of this.params) {
param.include(context, includeChildrenRecursively);
}
}
}

includeCallArguments(
context: InclusionContext,
args: readonly (ExpressionEntity | SpreadElement)[]
): void {
for (let position = 0; position < this.params.length; position++) {
const parameter = this.params[position];
if (parameter instanceof AssignmentPattern) {
if (parameter.left.shouldBeIncluded(context)) {
parameter.left.include(context, false);
}
const argumentValue = args[position]?.getLiteralValueAtPath(
EMPTY_PATH,
SHARED_RECURSION_TRACKER,
this
);
// If argumentValue === UnknownTruthyValue, then we do not need to
// include the default
if (
(argumentValue === undefined || argumentValue === UnknownValue) &&
(this.parameterVariables[position].some(variable => variable.included) ||
parameter.right.shouldBeIncluded(context))
) {
parameter.right.include(context, false);
}
} else if (parameter.shouldBeIncluded(context)) {
parameter.include(context, false);
}
}
this.scope.includeCallArguments(context, args);
}

initialise(): void {
this.parameterVariables = this.params.map(param =>
param.declare('parameter', UNKNOWN_EXPRESSION)
);
this.scope.addParameterVariables(
this.parameterVariables,
this.params.map(param => param.declare('parameter', UNKNOWN_EXPRESSION)),
this.params[this.params.length - 1] instanceof RestElement
);
if (this.body instanceof BlockStatement) {
Expand All @@ -249,15 +164,7 @@ export default abstract class FunctionBase extends NodeBase implements Deoptimiz
super.parseNode(esTreeNode);
}

protected applyDeoptimizations() {
// We currently do not track deoptimizations of default values, deoptimize them
// just as we deoptimize call arguments
for (const param of this.params) {
if (param instanceof AssignmentPattern) {
param.right.deoptimizePath(UNKNOWN_PATH);
}
}
}
protected applyDeoptimizations() {}

protected abstract getObjectEntity(): ObjectEntity;
}
Expand Down
22 changes: 10 additions & 12 deletions src/ast/nodes/shared/FunctionNode.ts
@@ -1,12 +1,11 @@
import { BLANK } from '../../../utils/blank';
import { type CallOptions } from '../../CallOptions';
import { type HasEffectsContext, type InclusionContext } from '../../ExecutionContext';
import { EVENT_CALLED, type NodeEvent } from '../../NodeEvents';
import FunctionScope from '../../scopes/FunctionScope';
import { type ObjectPath, PathTracker } from '../../utils/PathTracker';
import BlockStatement from '../BlockStatement';
import { type IdentifierWithVariable } from '../Identifier';
import { type ExpressionEntity, InclusionOptions, UNKNOWN_EXPRESSION } from './Expression';
import Identifier, { type IdentifierWithVariable } from '../Identifier';
import { type ExpressionEntity, UNKNOWN_EXPRESSION } from './Expression';
import FunctionBase from './FunctionBase';
import { type IncludeChildren } from './Node';
import { ObjectEntity } from './ObjectEntity';
Expand Down Expand Up @@ -74,16 +73,15 @@ export default class FunctionNode extends FunctionBase {
return false;
}

include(
context: InclusionContext,
includeChildrenRecursively: IncludeChildren,
{ includeWithoutParameterDefaults }: InclusionOptions = BLANK
): void {
include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
super.include(context, includeChildrenRecursively);
this.id?.include();
super.include(context, includeChildrenRecursively, {
includeWithoutParameterDefaults:
includeWithoutParameterDefaults && !this.scope.argumentsVariable.included
});
const hasArguments = this.scope.argumentsVariable.included;
for (const param of this.params) {
if (!(param instanceof Identifier) || hasArguments) {
param.include(context, includeChildrenRecursively);
}
}
}

initialise(): void {
Expand Down