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

Class method effects #4018

Merged
merged 56 commits into from May 23, 2021
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
7b8304e
Move logic from ClassBody into ClassNode
marijnh Mar 25, 2021
d524abd
Track static class fields and improve handling of class getters/setters
marijnh Mar 25, 2021
185097d
Remove ClassNode.deoptimizeCache
marijnh Mar 27, 2021
0960a33
Keep a table for class property effects
marijnh Mar 27, 2021
cea520d
Add comment explaining property map
lukastaegert Apr 2, 2021
bcd7f5a
Merge branch 'master' into class-method-effects
lukastaegert Apr 14, 2021
0e0fdaa
Merge branch 'master' into class-method-effects
lukastaegert Apr 14, 2021
1251f5a
Fix types
lukastaegert Apr 15, 2021
d8ec2ea
Merge branch 'master' into class-method-effects
lukastaegert Apr 15, 2021
056d9f0
Make getReturnExpression and getLiteralValue more similar for objects
lukastaegert Apr 17, 2021
9507110
Use common logic for return expression and literal value
lukastaegert Apr 17, 2021
7f51eef
Use common logic for return access and call effects
lukastaegert Apr 17, 2021
10c4e40
Extract shared logic from ObjectExpression
lukastaegert Apr 19, 2021
99a794d
Use an object for better performance
lukastaegert Apr 19, 2021
72dc62b
Simplify handling for setters and other properties
lukastaegert Apr 20, 2021
ba6e043
Small simplification
lukastaegert Apr 20, 2021
f19fc61
Work towards better class handling
lukastaegert Apr 21, 2021
132fb94
merge ObjectPathHandler into ObjectEntity
lukastaegert Apr 21, 2021
f09cd5b
Slightly refactor default values
lukastaegert Apr 21, 2021
10ee697
Separate unknown nodes from other Nodes to avoid future circular depe…
lukastaegert Apr 21, 2021
0350110
Introduce new prototype tracking
lukastaegert Apr 22, 2021
d723df6
Improve coverage
lukastaegert Apr 23, 2021
a2f02c1
Fix class deoptimization in arrow functions via this/super
lukastaegert Apr 23, 2021
f89085c
Simplify and merge property and method definition
lukastaegert Apr 23, 2021
6d70426
Improve coverage
lukastaegert Apr 23, 2021
4eff4ed
Replace deoptimizeProperties by deoptimizing a double unknown path
lukastaegert Apr 24, 2021
dd5f73b
Assume functions can add getters to parameters
lukastaegert Apr 24, 2021
333116a
Improve class.prototype handling
lukastaegert Apr 25, 2021
8f498f2
Assume created instance getters have side-effects
lukastaegert Apr 25, 2021
ef0929f
Base all expressions on a base class
lukastaegert Apr 25, 2021
0fcd42d
Merge branch 'master' into class-method-effects
lukastaegert May 6, 2021
145d01f
Only deoptimize necessary paths when deoptimizing "this"
lukastaegert Apr 30, 2021
71a27ba
Handle deoptimizing "this" in getters
lukastaegert May 6, 2021
616a6d3
Handle deoptimizing "this" in setters
lukastaegert May 8, 2021
182a0ba
Merge branch 'master' into class-method-effects
lukastaegert May 8, 2021
e33e06e
Unify this deoptimization
lukastaegert May 8, 2021
afad828
Simplify recursion tracking
lukastaegert May 9, 2021
d290c6c
Get rid of deoptimizations during bind phase
lukastaegert May 10, 2021
0609907
Get rid of unneeded double-binding checks
lukastaegert May 10, 2021
a70e660
Inline deoptimizations into NodeBase for simple cases
lukastaegert May 10, 2021
ea5fe1d
Add more efficient way to create object entities
lukastaegert May 10, 2021
02c1127
Add thisParameter to CallOptions
lukastaegert May 12, 2021
ab15b60
Move NodeEvents to separate file
lukastaegert May 12, 2021
ece8f31
Track array elements
lukastaegert May 14, 2021
275946e
Merge branch 'master' into class-method-effects
lukastaegert May 15, 2021
f4766a6
Simplify namespace handling
lukastaegert May 15, 2021
92b1e1d
Use Object.values and Object.entries instead of Object.keys where useful
lukastaegert May 15, 2021
224a4e1
Improve code and simplify literal handling
lukastaegert May 16, 2021
87e8029
Improve coverage
lukastaegert May 16, 2021
d2b61c9
Improve coverage
lukastaegert May 16, 2021
66a5cf9
Improve coverage in conditional and logical expressions
lukastaegert May 16, 2021
76b5cbe
Improve coverage
lukastaegert May 17, 2021
b6bd8d4
2.49.0-0
lukastaegert May 18, 2021
99964aa
Fix test to support pre-release versions
lukastaegert May 18, 2021
84b0d94
Fix failed deoptimization of array props
lukastaegert May 20, 2021
7490a87
2.49.0-1
lukastaegert May 20, 2021
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: 1 addition & 3 deletions cli/run/build.ts
Expand Up @@ -24,9 +24,7 @@ export default async function build(
} else if (inputOptions.input instanceof Array) {
inputFiles = inputOptions.input.join(', ');
} else if (typeof inputOptions.input === 'object' && inputOptions.input !== null) {
inputFiles = Object.keys(inputOptions.input)
.map(name => (inputOptions.input as Record<string, string>)[name])
.join(', ');
inputFiles = Object.values(inputOptions.input).join(', ');
}
stderr(cyan(`\n${bold(inputFiles!)} → ${bold(files.join(', '))}...`));
}
Expand Down
4 changes: 1 addition & 3 deletions cli/run/watch-cli.ts
Expand Up @@ -106,9 +106,7 @@ export async function watch(command: any) {
if (typeof input !== 'string') {
input = Array.isArray(input)
? input.join(', ')
: Object.keys(input as Record<string, string>)
.map(key => (input as Record<string, string>)[key])
.join(', ');
: Object.values(input as Record<string, string>).join(', ');
}
stderr(
cyan(`bundles ${bold(input)} → ${bold(event.output.map(relativeId).join(', '))}...`)
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
@@ -1,6 +1,6 @@
{
"name": "rollup",
"version": "2.48.0",
"version": "2.49.0-0",
"description": "Next-generation ES module bundler",
"main": "dist/rollup.js",
"module": "dist/es/rollup.js",
Expand Down
22 changes: 9 additions & 13 deletions src/Bundle.ts
Expand Up @@ -6,6 +6,7 @@ import {
GetManualChunk,
NormalizedInputOptions,
NormalizedOutputOptions,
OutputAsset,
OutputBundle,
OutputBundleWithPlaceholders,
OutputChunk,
Expand Down Expand Up @@ -41,11 +42,7 @@ export default class Bundle {
async generate(isWrite: boolean): Promise<OutputBundle> {
timeStart('GENERATE', 1);
const outputBundle: OutputBundleWithPlaceholders = Object.create(null);
this.pluginDriver.setOutputBundle(
outputBundle,
this.outputOptions,
this.facadeChunkByModule
);
this.pluginDriver.setOutputBundle(outputBundle, this.outputOptions, this.facadeChunkByModule);
try {
await this.pluginDriver.hookParallel('renderStart', [this.outputOptions, this.inputOptions]);

Expand Down Expand Up @@ -104,9 +101,9 @@ export default class Bundle {
): Promise<Map<Module, string>> {
const manualChunkAliasByEntry = new Map<Module, string>();
const chunkEntries = await Promise.all(
Object.keys(manualChunks).map(async alias => ({
Object.entries(manualChunks).map(async ([alias, files]) => ({
alias,
entries: await this.graph.moduleLoader.addAdditionalModules(manualChunks[alias])
entries: await this.graph.moduleLoader.addAdditionalModules(files)
}))
);
for (const { alias, entries } of chunkEntries) {
Expand Down Expand Up @@ -169,24 +166,23 @@ export default class Bundle {
}

private finaliseAssets(outputBundle: OutputBundleWithPlaceholders): void {
for (const key of Object.keys(outputBundle)) {
const file = outputBundle[key] as any;
for (const file of Object.values(outputBundle)) {
if (!file.type) {
warnDeprecation(
'A plugin is directly adding properties to the bundle object in the "generateBundle" hook. This is deprecated and will be removed in a future Rollup version, please use "this.emitFile" instead.',
true,
this.inputOptions
);
file.type = 'asset';
(file as OutputAsset).type = 'asset';
}
if (this.outputOptions.validate && typeof file.code == 'string') {
if (this.outputOptions.validate && typeof (file as OutputChunk).code == 'string') {
try {
this.graph.contextParse(file.code, {
this.graph.contextParse((file as OutputChunk).code, {
allowHashBang: true,
ecmaVersion: 'latest'
});
} catch (exception) {
this.inputOptions.onwarn(errChunkInvalid(file, exception));
this.inputOptions.onwarn(errChunkInvalid(file as OutputChunk, exception));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/Chunk.ts
Expand Up @@ -1326,8 +1326,8 @@ export default class Chunk {
if (!this.outputOptions.preserveModules) {
if (this.includedNamespaces.has(module)) {
const memberVariables = module.namespace.getMemberVariables();
for (const name of Object.keys(memberVariables)) {
moduleImports.add(memberVariables[name]);
for (const variable of Object.values(memberVariables)) {
moduleImports.add(variable);
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions src/Graph.ts
Expand Up @@ -34,9 +34,9 @@ function normalizeEntryModules(
name: null
}));
}
return Object.keys(entryModules).map(name => ({
return Object.entries(entryModules).map(([name, id]) => ({
fileName: null,
id: entryModules[name],
id,
implicitlyLoadedAfter: [],
importer: undefined,
name
Expand Down Expand Up @@ -74,13 +74,14 @@ export default class Graph {
// increment access counter
for (const name in this.pluginCache) {
const cache = this.pluginCache[name];
for (const key of Object.keys(cache)) cache[key][0]++;
for (const value of Object.values(cache)) value[0]++;
}
}

if (watcher) {
this.watchMode = true;
const handleChange: WatchChangeHook = (...args) => this.pluginDriver.hookSeqSync('watchChange', args);
const handleChange: WatchChangeHook = (...args) =>
this.pluginDriver.hookSeqSync('watchChange', args);
const handleClose = () => this.pluginDriver.hookSeqSync('closeWatcher', []);
watcher.on('change', handleChange);
watcher.on('close', handleClose);
Expand Down Expand Up @@ -118,9 +119,9 @@ export default class Graph {

if (onCommentOrig && typeof onCommentOrig == 'function') {
options.onComment = (block, text, start, end, ...args) => {
comments.push({type: block ? "Block" : "Line", value: text, start, end});
comments.push({ type: block ? 'Block' : 'Line', value: text, start, end });
return onCommentOrig.call(options, block, text, start, end, ...args);
}
};
} else {
options.onComment = comments;
}
Expand All @@ -146,8 +147,8 @@ export default class Graph {
for (const name in this.pluginCache) {
const cache = this.pluginCache[name];
let allDeleted = true;
for (const key of Object.keys(cache)) {
if (cache[key][0] >= this.options.experimentalCacheExpiry) delete cache[key];
for (const [key, value] of Object.entries(cache)) {
if (value[0] >= this.options.experimentalCacheExpiry) delete cache[key];
else allDeleted = false;
}
if (allDeleted) delete this.pluginCache[name];
Expand Down Expand Up @@ -238,8 +239,7 @@ export default class Graph {

private warnForMissingExports() {
for (const module of this.modules) {
for (const importName of Object.keys(module.importDescriptions)) {
const importDescription = module.importDescriptions[importName];
for (const importDescription of Object.values(module.importDescriptions)) {
if (
importDescription.name !== '*' &&
!(importDescription.module as Module).getVariableForExportName(importDescription.name)
Expand Down
3 changes: 1 addition & 2 deletions src/Module.ts
Expand Up @@ -1002,8 +1002,7 @@ export default class Module {
private addModulesToImportDescriptions(importDescription: {
[name: string]: ImportDescription | ReexportDescription;
}) {
for (const name of Object.keys(importDescription)) {
const specifier = importDescription[name];
for (const specifier of Object.values(importDescription)) {
const id = this.resolvedIds[specifier.source].id;
specifier.module = this.graph.modulesById.get(id)!;
}
Expand Down
1 change: 1 addition & 0 deletions src/ast/CallOptions.ts
Expand Up @@ -5,5 +5,6 @@ export const NO_ARGS = [];

export interface CallOptions {
args: (ExpressionEntity | SpreadElement)[];
thisParam: ExpressionEntity | null;
withNew: boolean;
}
6 changes: 2 additions & 4 deletions src/ast/Entity.ts
@@ -1,9 +1,7 @@
import { HasEffectsContext } from './ExecutionContext';
import { ObjectPath } from './utils/PathTracker';

export interface Entity {
toString: () => string;
}
export interface Entity {}

export interface WritableEntity extends Entity {
/**
Expand All @@ -13,5 +11,5 @@ export interface WritableEntity extends Entity {
* expression of this node is reassigned as well.
*/
deoptimizePath(path: ObjectPath): void;
hasEffectsWhenAssignedAtPath(path: ObjectPath, execution: HasEffectsContext): boolean;
hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean;
}
4 changes: 4 additions & 0 deletions src/ast/NodeEvents.ts
@@ -0,0 +1,4 @@
export const EVENT_ACCESSED = 0;
export const EVENT_ASSIGNED = 1;
export const EVENT_CALLED = 2;
export type NodeEvent = typeof EVENT_ACCESSED | typeof EVENT_ASSIGNED | typeof EVENT_CALLED;
2 changes: 1 addition & 1 deletion src/ast/keys.ts
Expand Up @@ -9,7 +9,7 @@ export const keys: {

export function getAndCreateKeys(esTreeNode: GenericEsTreeNode) {
keys[esTreeNode.type] = Object.keys(esTreeNode).filter(
key => key !== '_rollupAnnotations' && typeof esTreeNode[key] === 'object'
key => typeof esTreeNode[key] === 'object' && key !== '_rollupAnnotations'
);
return keys[esTreeNode.type];
}
90 changes: 70 additions & 20 deletions src/ast/nodes/ArrayExpression.ts
@@ -1,44 +1,94 @@
import { CallOptions } from '../CallOptions';
import { DeoptimizableEntity } from '../DeoptimizableEntity';
import { HasEffectsContext } from '../ExecutionContext';
import { ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
import {
arrayMembers,
getMemberReturnExpressionWhenCalled,
hasMemberEffectWhenCalled,
UNKNOWN_EXPRESSION
} from '../values';
import { NodeEvent } from '../NodeEvents';
import { ObjectPath, PathTracker, UnknownInteger } from '../utils/PathTracker';
import { UNDEFINED_EXPRESSION, UNKNOWN_LITERAL_NUMBER } from '../values';
import * as NodeType from './NodeType';
import { ARRAY_PROTOTYPE } from './shared/ArrayPrototype';
import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression';
import { ExpressionNode, NodeBase } from './shared/Node';
import { ObjectEntity, ObjectProperty } from './shared/ObjectEntity';
import SpreadElement from './SpreadElement';

export default class ArrayExpression extends NodeBase {
elements!: (ExpressionNode | SpreadElement | null)[];
type!: NodeType.tArrayExpression;
private objectEntity: ObjectEntity | null = null;

bind() {
super.bind();
for (const element of this.elements) {
if (element !== null) element.deoptimizePath(UNKNOWN_PATH);
}
deoptimizePath(path: ObjectPath) {
this.getObjectEntity().deoptimizePath(path);
}

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

getLiteralValueAtPath(
path: ObjectPath,
recursionTracker: PathTracker,
origin: DeoptimizableEntity
): LiteralValueOrUnknown {
return this.getObjectEntity().getLiteralValueAtPath(path, recursionTracker, origin);
}

getReturnExpressionWhenCalledAtPath(path: ObjectPath) {
if (path.length !== 1) return UNKNOWN_EXPRESSION;
return getMemberReturnExpressionWhenCalled(arrayMembers, path[0]);
getReturnExpressionWhenCalledAtPath(
path: ObjectPath,
callOptions: CallOptions,
recursionTracker: PathTracker,
origin: DeoptimizableEntity
): ExpressionEntity {
return this.getObjectEntity().getReturnExpressionWhenCalledAtPath(
path,
callOptions,
recursionTracker,
origin
);
}

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

hasEffectsWhenAccessedAtPath(path: ObjectPath) {
return path.length > 1;
hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext) {
return this.getObjectEntity().hasEffectsWhenAssignedAtPath(path, context);
}

hasEffectsWhenCalledAtPath(
path: ObjectPath,
callOptions: CallOptions,
context: HasEffectsContext
): boolean {
if (path.length === 1) {
return hasMemberEffectWhenCalled(arrayMembers, path[0], this.included, callOptions, context);
return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
}

private getObjectEntity(): ObjectEntity {
if (this.objectEntity !== null) {
return this.objectEntity;
}
const properties: ObjectProperty[] = [
{ kind: 'init', key: 'length', property: UNKNOWN_LITERAL_NUMBER }
];
for (let index = 0; index < this.elements.length; index++) {
const element = this.elements[index];
if (element instanceof SpreadElement) {
properties.unshift({ kind: 'init', key: UnknownInteger, property: element });
} else if (!element) {
properties.push({ kind: 'init', key: String(index), property: UNDEFINED_EXPRESSION });
} else {
properties.push({ kind: 'init', key: String(index), property: element });
}
}
return true;
return (this.objectEntity = new ObjectEntity(properties, ARRAY_PROTOTYPE));
}
}
2 changes: 1 addition & 1 deletion src/ast/nodes/ArrayPattern.ts
@@ -1,8 +1,8 @@
import { HasEffectsContext } from '../ExecutionContext';
import { EMPTY_PATH, ObjectPath } from '../utils/PathTracker';
import { UNKNOWN_EXPRESSION } from '../values';
import Variable from '../variables/Variable';
import * as NodeType from './NodeType';
import { UNKNOWN_EXPRESSION } from './shared/Expression';
import { NodeBase } from './shared/Node';
import { PatternNode } from './shared/Pattern';

Expand Down
9 changes: 4 additions & 5 deletions src/ast/nodes/ArrowFunctionExpression.ts
Expand Up @@ -3,11 +3,11 @@ import { BROKEN_FLOW_NONE, HasEffectsContext, InclusionContext } from '../Execut
import ReturnValueScope from '../scopes/ReturnValueScope';
import Scope from '../scopes/Scope';
import { ObjectPath, UnknownKey, UNKNOWN_PATH } from '../utils/PathTracker';
import { UNKNOWN_EXPRESSION } from '../values';
import BlockStatement from './BlockStatement';
import Identifier from './Identifier';
import * as NodeType from './NodeType';
import RestElement from './RestElement';
import { UNKNOWN_EXPRESSION } from './shared/Expression';
import { ExpressionNode, GenericEsTreeNode, IncludeChildren, NodeBase } from './shared/Node';
import { PatternNode } from './shared/Pattern';
import SpreadElement from './SpreadElement';
Expand All @@ -31,6 +31,9 @@ export default class ArrowFunctionExpression extends NodeBase {
}
}

// Arrow functions do not mutate their context
deoptimizeThisOnEventAtPath() {}

getReturnExpressionWhenCalledAtPath(path: ObjectPath) {
return path.length === 0 ? this.scope.getReturnExpression() : UNKNOWN_EXPRESSION;
}
Expand Down Expand Up @@ -98,10 +101,6 @@ 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(
Expand Down
4 changes: 2 additions & 2 deletions src/ast/nodes/AssignmentExpression.ts
Expand Up @@ -33,7 +33,7 @@ export default class AssignmentExpression extends NodeBase {
| '**=';
right!: ExpressionNode;
type!: NodeType.tAssignmentExpression;
private deoptimized = false;
protected deoptimized = false;

hasEffects(context: HasEffectsContext): boolean {
if (!this.deoptimized) this.applyDeoptimizations();
Expand Down Expand Up @@ -122,7 +122,7 @@ export default class AssignmentExpression extends NodeBase {
}
}

private applyDeoptimizations() {
protected applyDeoptimizations() {
this.deoptimized = true;
this.left.deoptimizePath(EMPTY_PATH);
this.right.deoptimizePath(UNKNOWN_PATH);
Expand Down