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

Deoptimize awaited expressions #4162

Merged
merged 1 commit into from Jul 1, 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
4 changes: 2 additions & 2 deletions src/ast/ExecutionContext.ts
Expand Up @@ -7,7 +7,7 @@ interface ExecutionContextIgnore {
breaks: boolean;
continues: boolean;
labels: Set<string>;
returnAwaitYield: boolean;
returnYield: boolean;
}

export const BROKEN_FLOW_NONE = 0;
Expand Down Expand Up @@ -51,7 +51,7 @@ export function createHasEffectsContext(): HasEffectsContext {
breaks: false,
continues: false,
labels: new Set(),
returnAwaitYield: false
returnYield: false
},
includedLabels: new Set(),
instantiated: new DiscriminatedPathTracker(),
Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/ArrowFunctionExpression.ts
Expand Up @@ -83,7 +83,7 @@ export default class ArrowFunctionExpression extends NodeBase {
breaks: false,
continues: false,
labels: new Set(),
returnAwaitYield: true
returnYield: true
};
if (this.body.hasEffects(context)) return true;
context.ignore = ignore;
Expand Down
30 changes: 12 additions & 18 deletions src/ast/nodes/AwaitExpression.ts
@@ -1,6 +1,5 @@
import { NormalizedTreeshakingOptions } from '../../rollup/types';
import { NO_ARGS } from '../CallOptions';
import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
import { InclusionContext } from '../ExecutionContext';
import { UNKNOWN_PATH } from '../utils/PathTracker';
import ArrowFunctionExpression from './ArrowFunctionExpression';
import * as NodeType from './NodeType';
import FunctionNode from './shared/FunctionNode';
Expand All @@ -9,25 +8,15 @@ import { ExpressionNode, IncludeChildren, Node, NodeBase } from './shared/Node';
export default class AwaitExpression extends NodeBase {
argument!: ExpressionNode;
type!: NodeType.tAwaitExpression;
protected deoptimized = false;

hasEffects(context: HasEffectsContext): boolean {
const { propertyReadSideEffects } = this.context.options
.treeshake as NormalizedTreeshakingOptions;
return (
!context.ignore.returnAwaitYield ||
this.argument.hasEffects(context) ||
this.argument.hasEffectsWhenCalledAtPath(
['then'],
{ args: NO_ARGS, thisParam: null, withNew: false },
context
) ||
(propertyReadSideEffects &&
(propertyReadSideEffects === 'always' ||
this.argument.hasEffectsWhenAccessedAtPath(['then'], context)))
);
hasEffects(): boolean {
if (!this.deoptimized) this.applyDeoptimizations();
return true;
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
if (!this.deoptimized) this.applyDeoptimizations();
if (!this.included) {
this.included = true;
checkTopLevelAwait: if (!this.context.usesTopLevelAwait) {
Expand All @@ -41,4 +30,9 @@ export default class AwaitExpression extends NodeBase {
}
this.argument.include(context, includeChildrenRecursively);
}

protected applyDeoptimizations(): void {
this.deoptimized = true;
this.argument.deoptimizePath(UNKNOWN_PATH);
}
}
2 changes: 1 addition & 1 deletion src/ast/nodes/ReturnStatement.ts
Expand Up @@ -15,7 +15,7 @@ export default class ReturnStatement extends StatementBase {

hasEffects(context: HasEffectsContext): boolean {
if (
!context.ignore.returnAwaitYield ||
!context.ignore.returnYield ||
(this.argument !== null && this.argument.hasEffects(context))
)
return true;
Expand Down
3 changes: 1 addition & 2 deletions src/ast/nodes/YieldExpression.ts
Expand Up @@ -14,8 +14,7 @@ export default class YieldExpression extends NodeBase {
hasEffects(context: HasEffectsContext): boolean {
if (!this.deoptimized) this.applyDeoptimizations();
return (
!context.ignore.returnAwaitYield ||
(this.argument !== null && this.argument.hasEffects(context))
!context.ignore.returnYield || (this.argument !== null && this.argument.hasEffects(context))
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/ast/nodes/shared/FunctionNode.ts
Expand Up @@ -114,7 +114,7 @@ export default class FunctionNode extends NodeBase {
breaks: false,
continues: false,
labels: new Set(),
returnAwaitYield: true
returnYield: true
};
if (this.body.hasEffects(context)) return true;
context.brokenFlow = brokenFlow;
Expand Down
18 changes: 0 additions & 18 deletions test/form/samples/async-function-effects/main.js
Expand Up @@ -77,14 +77,6 @@
return { then() {} };
})();

// removed
(async function () {
await {
then: function () {}
};
return { then() {} };
})();

(async function () {
await {
get then() {
Expand All @@ -104,16 +96,6 @@
return { then() {} };
})();

// removed
(async function () {
await {
get then() {
return () => {};
}
};
return { then() {} };
})();

(async function () {
await await {
then(resolve) {
Expand Down
10 changes: 0 additions & 10 deletions test/form/samples/ignore-property-access-side-effects/main.js
Expand Up @@ -38,13 +38,3 @@ const foo7 = (async () => ({
return () => {};
}
}))();

const foo8 = (async function () {
await {
get then() {
console.log('effect');
return () => {};
}
};
return { then() {} };
})();
10 changes: 10 additions & 0 deletions test/function/samples/track-awaited-object/_config.js
@@ -0,0 +1,10 @@
const assert = require('assert');

module.exports = {
description: 'tracks object mutations through await',
async exports(exports) {
assert.strictEqual(exports.toggled, false);
await exports.test();
assert.strictEqual(exports.toggled, true);
}
};
19 changes: 19 additions & 0 deletions test/function/samples/track-awaited-object/main.js
@@ -0,0 +1,19 @@
function fn() {
const obj = {
test() {
if (typeof obj.testfn === 'function') {
obj.testfn();
}
}
};

return obj;
}

export let toggled = false;

export const test = async function () {
const obj = await fn();
obj.testfn = () => (toggled = true);
obj.test();
};