diff --git a/src/ast/nodes/AssignmentExpression.ts b/src/ast/nodes/AssignmentExpression.ts index 9daabe0bd36..8dd9796cab8 100644 --- a/src/ast/nodes/AssignmentExpression.ts +++ b/src/ast/nodes/AssignmentExpression.ts @@ -1,11 +1,11 @@ import MagicString from 'magic-string'; import { findFirstOccurrenceOutsideComment, RenderOptions } from '../../utils/renderHelpers'; import { getSystemExportStatement } from '../../utils/systemJsRendering'; -import { HasEffectsContext } from '../ExecutionContext'; +import { HasEffectsContext, InclusionContext } from '../ExecutionContext'; import { EMPTY_PATH, ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker'; import Variable from '../variables/Variable'; import * as NodeType from './NodeType'; -import { ExpressionNode, NodeBase } from './shared/Node'; +import { ExpressionNode, IncludeChildren, NodeBase } from './shared/Node'; import { PatternNode } from './shared/Pattern'; export default class AssignmentExpression extends NodeBase { @@ -26,15 +26,10 @@ export default class AssignmentExpression extends NodeBase { | '**='; right!: ExpressionNode; type!: NodeType.tAssignmentExpression; - - bind() { - super.bind(); - this.left.deoptimizePath(EMPTY_PATH); - // We cannot propagate mutations of the new binding to the old binding with certainty - this.right.deoptimizePath(UNKNOWN_PATH); - } + private deoptimized = false; hasEffects(context: HasEffectsContext): boolean { + if (!this.deoptimized) this.applyDeoptimizations(); return ( this.right.hasEffects(context) || this.left.hasEffects(context) || @@ -46,6 +41,13 @@ export default class AssignmentExpression extends NodeBase { return path.length > 0 && this.right.hasEffectsWhenAccessedAtPath(path, context); } + include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void { + if (!this.deoptimized) this.applyDeoptimizations(); + this.included = true; + this.left.include(context, includeChildrenRecursively); + this.right.include(context, includeChildrenRecursively); + } + render(code: MagicString, options: RenderOptions) { this.left.render(code, options); this.right.render(code, options); @@ -79,4 +81,10 @@ export default class AssignmentExpression extends NodeBase { } } } + + private applyDeoptimizations() { + this.deoptimized = true; + this.left.deoptimizePath(EMPTY_PATH); + this.right.deoptimizePath(UNKNOWN_PATH); + } } diff --git a/src/ast/nodes/IfStatement.ts b/src/ast/nodes/IfStatement.ts index 615fb1c3951..ad629bf667b 100644 --- a/src/ast/nodes/IfStatement.ts +++ b/src/ast/nodes/IfStatement.ts @@ -8,27 +8,26 @@ import { LiteralValueOrUnknown, UnknownValue } from '../values'; import * as NodeType from './NodeType'; import { ExpressionNode, IncludeChildren, StatementBase, StatementNode } from './shared/Node'; +const unset = Symbol('unset'); + export default class IfStatement extends StatementBase implements DeoptimizableEntity { alternate!: StatementNode | null; consequent!: StatementNode; test!: ExpressionNode; type!: NodeType.tIfStatement; - private testValue: LiteralValueOrUnknown; - - bind() { - super.bind(); - // ensure the testValue is set for the tree-shaking passes - this.testValue = this.test.getLiteralValueAtPath(EMPTY_PATH, SHARED_RECURSION_TRACKER, this); - } + private testValue: LiteralValueOrUnknown | typeof unset = unset; deoptimizeCache() { this.testValue = UnknownValue; } hasEffects(context: HasEffectsContext): boolean { - if (this.test.hasEffects(context)) return true; - if (this.testValue === UnknownValue) { + if (this.test.hasEffects(context)) { + return true; + } + const testValue = this.getTestValue(); + if (testValue === UnknownValue) { const { brokenFlow } = context; if (this.consequent.hasEffects(context)) return true; const consequentBrokenFlow = context.brokenFlow; @@ -39,7 +38,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE context.brokenFlow < consequentBrokenFlow ? context.brokenFlow : consequentBrokenFlow; return false; } - return this.testValue + return testValue ? this.consequent.hasEffects(context) : this.alternate !== null && this.alternate.hasEffects(context); } @@ -48,22 +47,24 @@ export default class IfStatement extends StatementBase implements DeoptimizableE this.included = true; if (includeChildrenRecursively) { this.includeRecursively(includeChildrenRecursively, context); - } else if (this.testValue === UnknownValue) { - this.includeUnknownTest(context); } else { - this.includeKnownTest(context); + const testValue = this.getTestValue(); + if (testValue === UnknownValue) { + this.includeUnknownTest(context); + } else { + this.includeKnownTest(context, testValue); + } } } render(code: MagicString, options: RenderOptions) { // Note that unknown test values are always included + const testValue = this.getTestValue(); if ( !this.test.included && - (this.testValue - ? this.alternate === null || !this.alternate.included - : !this.consequent.included) + (testValue ? this.alternate === null || !this.alternate.included : !this.consequent.included) ) { - const singleRetainedBranch = (this.testValue ? this.consequent : this.alternate)!; + const singleRetainedBranch = (testValue ? this.consequent : this.alternate)!; code.remove(this.start, singleRetainedBranch.start); code.remove(singleRetainedBranch.end, this.end); removeAnnotations(this, code); @@ -72,7 +73,7 @@ export default class IfStatement extends StatementBase implements DeoptimizableE if (this.test.included) { this.test.render(code, options); } else { - code.overwrite(this.test.start, this.test.end, this.testValue ? 'true' : 'false'); + code.overwrite(this.test.start, this.test.end, testValue ? 'true' : 'false'); } if (this.consequent.included) { this.consequent.render(code, options); @@ -89,14 +90,25 @@ export default class IfStatement extends StatementBase implements DeoptimizableE } } - private includeKnownTest(context: InclusionContext) { + private getTestValue(): LiteralValueOrUnknown { + if (this.testValue === unset) { + return (this.testValue = this.test.getLiteralValueAtPath( + EMPTY_PATH, + SHARED_RECURSION_TRACKER, + this + )); + } + return this.testValue; + } + + private includeKnownTest(context: InclusionContext, testValue: LiteralValueOrUnknown) { if (this.test.shouldBeIncluded(context)) { this.test.include(context, false); } - if (this.testValue && this.consequent.shouldBeIncluded(context)) { + if (testValue && this.consequent.shouldBeIncluded(context)) { this.consequent.include(context, false); } - if (this.alternate !== null && !this.testValue && this.alternate.shouldBeIncluded(context)) { + if (this.alternate !== null && !testValue && this.alternate.shouldBeIncluded(context)) { this.alternate.include(context, false); } } diff --git a/test/form/samples/lazy-assignment-deoptimization/_config.js b/test/form/samples/lazy-assignment-deoptimization/_config.js new file mode 100644 index 00000000000..3d2fbfd1923 --- /dev/null +++ b/test/form/samples/lazy-assignment-deoptimization/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'only deoptimizes assigned variables when the assignment is included' +}; diff --git a/test/form/samples/lazy-assignment-deoptimization/_expected.js b/test/form/samples/lazy-assignment-deoptimization/_expected.js new file mode 100644 index 00000000000..724e99a15d2 --- /dev/null +++ b/test/form/samples/lazy-assignment-deoptimization/_expected.js @@ -0,0 +1,3 @@ +console.log('retained'); + +console.log('retained'); diff --git a/test/form/samples/lazy-assignment-deoptimization/main.js b/test/form/samples/lazy-assignment-deoptimization/main.js new file mode 100644 index 00000000000..43da5d447fd --- /dev/null +++ b/test/form/samples/lazy-assignment-deoptimization/main.js @@ -0,0 +1,12 @@ +const foo = { toggled: false }; +const bar = { toggled: false }; + +if (foo.toggled) { + foo.toggled = bar; +} + +if (foo.toggled) console.log('this should be removed'); +else console.log('retained'); + +if (bar.toggled) console.log('this should be removed'); +else console.log('retained'); diff --git a/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/_config.js b/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/_config.js new file mode 100644 index 00000000000..25bb08a0f03 --- /dev/null +++ b/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'makes sure the assignee is deoptimized' +}; diff --git a/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/main.js b/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/main.js new file mode 100644 index 00000000000..fc5921b2db5 --- /dev/null +++ b/test/function/samples/assignment-deoptimization/assignee-is-deoptimized/main.js @@ -0,0 +1,5 @@ +const flags = { updated: false }; +let toBeUpdated = {}; +toBeUpdated = flags; +toBeUpdated.updated = true; +if (!flags.updated) throw new Error('Update was not tracked'); diff --git a/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/_config.js b/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/_config.js new file mode 100644 index 00000000000..cd94f45ae26 --- /dev/null +++ b/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'makes sure the assignment target is deoptimized' +}; diff --git a/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/main.js b/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/main.js new file mode 100644 index 00000000000..f7112f60773 --- /dev/null +++ b/test/function/samples/assignment-deoptimization/assignment-target-is-deoptimized/main.js @@ -0,0 +1,3 @@ +let updated = false; +updated = true; +if (!updated) throw new Error('Update was not tracked'); diff --git a/test/function/samples/assignment-deoptimization/nested-assignment/_config.js b/test/function/samples/assignment-deoptimization/nested-assignment/_config.js new file mode 100644 index 00000000000..7c3114ddadc --- /dev/null +++ b/test/function/samples/assignment-deoptimization/nested-assignment/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'tracks assigments nested in expressions that are included for other reasons' +}; diff --git a/test/function/samples/assignment-deoptimization/nested-assignment/main.js b/test/function/samples/assignment-deoptimization/nested-assignment/main.js new file mode 100644 index 00000000000..57e5d4fc85c --- /dev/null +++ b/test/function/samples/assignment-deoptimization/nested-assignment/main.js @@ -0,0 +1,3 @@ +let updated = false; +assert.ok(!updated) || (updated = true); +if (!updated) throw new Error('Update was not tracked'); diff --git a/test/function/samples/assignment-deoptimization/try-catch-inclusion/_config.js b/test/function/samples/assignment-deoptimization/try-catch-inclusion/_config.js new file mode 100644 index 00000000000..4a7b55c699a --- /dev/null +++ b/test/function/samples/assignment-deoptimization/try-catch-inclusion/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'tracks assigments included via try-catch-deoptimization' +}; diff --git a/test/function/samples/assignment-deoptimization/try-catch-inclusion/main.js b/test/function/samples/assignment-deoptimization/try-catch-inclusion/main.js new file mode 100644 index 00000000000..1a34c1d59b0 --- /dev/null +++ b/test/function/samples/assignment-deoptimization/try-catch-inclusion/main.js @@ -0,0 +1,6 @@ +let updated = false; +try { + updated = true; +} catch (err) {} + +if (!updated) throw new Error('Update was not tracked');