From 001639d2a66a927b2bd73b5631cc2cd4cc5d3e97 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Fri, 10 Jun 2022 10:45:53 +0200 Subject: [PATCH] Remove feature and add regression test --- src/ast/nodes/BinaryExpression.ts | 56 ++----------------- src/ast/nodes/shared/ClassNode.ts | 3 - src/ast/nodes/shared/FunctionBase.ts | 11 +--- src/ast/nodes/shared/ObjectEntity.ts | 2 - src/ast/utils/PathTracker.ts | 13 +---- test/form/samples/instanceof/_config.js | 3 - test/form/samples/instanceof/_expected.js | 15 ----- test/form/samples/instanceof/main.js | 21 ------- .../instanceof/used-parameter/_config.js | 3 + .../samples/instanceof/used-parameter/main.js | 12 ++++ 10 files changed, 21 insertions(+), 118 deletions(-) delete mode 100644 test/form/samples/instanceof/_config.js delete mode 100644 test/form/samples/instanceof/_expected.js delete mode 100644 test/form/samples/instanceof/main.js create mode 100644 test/function/samples/instanceof/used-parameter/_config.js create mode 100644 test/function/samples/instanceof/used-parameter/main.js diff --git a/src/ast/nodes/BinaryExpression.ts b/src/ast/nodes/BinaryExpression.ts index 4f8c9db133c..5e73aa8d715 100644 --- a/src/ast/nodes/BinaryExpression.ts +++ b/src/ast/nodes/BinaryExpression.ts @@ -2,25 +2,19 @@ import type MagicString from 'magic-string'; import { BLANK } from '../../utils/blank'; import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers'; import type { DeoptimizableEntity } from '../DeoptimizableEntity'; -import type { HasEffectsContext, InclusionContext } from '../ExecutionContext'; -import { createHasEffectsContext, createInclusionContext } from '../ExecutionContext'; -import { - INTERACTION_ACCESSED, - NODE_INTERACTION_UNKNOWN_ASSIGNMENT, - NodeInteraction -} from '../NodeInteractions'; +import type { HasEffectsContext } from '../ExecutionContext'; +import { INTERACTION_ACCESSED, NodeInteraction } from '../NodeInteractions'; import { EMPTY_PATH, type ObjectPath, type PathTracker, - SHARED_RECURSION_TRACKER, - TEST_INSTANCEOF_PATH + SHARED_RECURSION_TRACKER } from '../utils/PathTracker'; import ExpressionStatement from './ExpressionStatement'; import type { LiteralValue } from './Literal'; import type * as NodeType from './NodeType'; import { type LiteralValueOrUnknown, UnknownValue } from './shared/Expression'; -import { type ExpressionNode, IncludeChildren, NodeBase } from './shared/Node'; +import { type ExpressionNode, NodeBase } from './shared/Node'; type Operator = | '!=' @@ -80,7 +74,6 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE declare operator: keyof typeof binaryOperators; declare right: ExpressionNode; declare type: NodeType.tBinaryExpression; - private expressionsDependingOnNegativeInstanceof = new Set(); deoptimizeCache(): void {} @@ -90,22 +83,6 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE origin: DeoptimizableEntity ): LiteralValueOrUnknown { if (path.length > 0) return UnknownValue; - if (this.operator === 'instanceof') { - // We need to include out-of-order to handle situations where left is the - // only usage of right - this.left.include(createInclusionContext(), false); - if ( - this.right.hasEffectsOnInteractionAtPath( - TEST_INSTANCEOF_PATH, - NODE_INTERACTION_UNKNOWN_ASSIGNMENT, - createHasEffectsContext() - ) - ) { - return UnknownValue; - } - this.expressionsDependingOnNegativeInstanceof.add(origin); - return false; - } const leftValue = this.left.getLiteralValueAtPath(EMPTY_PATH, recursionTracker, origin); if (typeof leftValue === 'symbol') return UnknownValue; @@ -127,7 +104,6 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE ) { return true; } - this.deoptimizeInstanceof(context); return super.hasEffects(context); } @@ -135,11 +111,6 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE return type !== INTERACTION_ACCESSED || path.length > 1; } - include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) { - this.deoptimizeInstanceof(); - super.include(context, includeChildrenRecursively); - } - render( code: MagicString, options: RenderOptions, @@ -148,23 +119,4 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE this.left.render(code, options, { renderedSurroundingElement }); this.right.render(code, options); } - - private deoptimizeInstanceof(context?: HasEffectsContext) { - if ( - this.operator === 'instanceof' && - (this.included || - (this.expressionsDependingOnNegativeInstanceof.size && - this.right.hasEffectsOnInteractionAtPath( - TEST_INSTANCEOF_PATH, - NODE_INTERACTION_UNKNOWN_ASSIGNMENT, - context || createHasEffectsContext() - ))) - ) { - for (const expression of this.expressionsDependingOnNegativeInstanceof) { - expression.deoptimizeCache(); - this.context.requestTreeshakingPass(); - } - this.expressionsDependingOnNegativeInstanceof.clear(); - } - } } diff --git a/src/ast/nodes/shared/ClassNode.ts b/src/ast/nodes/shared/ClassNode.ts index 44c44629aab..06f54ee3db7 100644 --- a/src/ast/nodes/shared/ClassNode.ts +++ b/src/ast/nodes/shared/ClassNode.ts @@ -13,7 +13,6 @@ import { type ObjectPath, type PathTracker, SHARED_RECURSION_TRACKER, - TestInstanceof, UNKNOWN_PATH, UnknownKey } from '../../utils/PathTracker'; @@ -96,8 +95,6 @@ export default class ClassNode extends NodeBase implements DeoptimizableEntity { : this.superClass?.hasEffectsOnInteractionAtPath(path, interaction, context)) || false ); - } else if (path[0] === TestInstanceof) { - return false; } else { return this.getObjectEntity().hasEffectsOnInteractionAtPath(path, interaction, context); } diff --git a/src/ast/nodes/shared/FunctionBase.ts b/src/ast/nodes/shared/FunctionBase.ts index 1ec9efa35dc..a1517cb7e08 100644 --- a/src/ast/nodes/shared/FunctionBase.ts +++ b/src/ast/nodes/shared/FunctionBase.ts @@ -14,13 +14,7 @@ import { NodeInteractionWithThisArg } from '../../NodeInteractions'; import ReturnValueScope from '../../scopes/ReturnValueScope'; -import { - type ObjectPath, - PathTracker, - TestInstanceof, - UNKNOWN_PATH, - UnknownKey -} from '../../utils/PathTracker'; +import { type ObjectPath, PathTracker, UNKNOWN_PATH, UnknownKey } from '../../utils/PathTracker'; import BlockStatement from '../BlockStatement'; import * as NodeType from '../NodeType'; import RestElement from '../RestElement'; @@ -101,9 +95,6 @@ export default abstract class FunctionBase extends NodeBase { interaction: NodeInteraction, context: HasEffectsContext ): boolean { - if (path[0] === TestInstanceof) { - return false; - } if (path.length > 0 || interaction.type !== INTERACTION_CALLED) { return this.getObjectEntity().hasEffectsOnInteractionAtPath(path, interaction, context); } diff --git a/src/ast/nodes/shared/ObjectEntity.ts b/src/ast/nodes/shared/ObjectEntity.ts index aef1164b87d..efbc2026427 100644 --- a/src/ast/nodes/shared/ObjectEntity.ts +++ b/src/ast/nodes/shared/ObjectEntity.ts @@ -11,7 +11,6 @@ import { ObjectPath, ObjectPathKey, PathTracker, - TestInstanceof, UNKNOWN_INTEGER_PATH, UNKNOWN_PATH, UnknownInteger, @@ -282,7 +281,6 @@ export class ObjectEntity extends ExpressionEntity { context: HasEffectsContext ): boolean { const [key, ...subPath] = path; - if (key === TestInstanceof) return true; if (subPath.length || interaction.type === INTERACTION_CALLED) { const expressionAtPath = this.getMemberExpression(key); if (expressionAtPath) { diff --git a/src/ast/utils/PathTracker.ts b/src/ast/utils/PathTracker.ts index 0c552d2242f..5b0679a6187 100644 --- a/src/ast/utils/PathTracker.ts +++ b/src/ast/utils/PathTracker.ts @@ -4,24 +4,15 @@ import type { Entity } from '../Entity'; export const UnknownKey = Symbol('Unknown Key'); export const UnknownNonAccessorKey = Symbol('Unknown Non-Accessor Key'); export const UnknownInteger = Symbol('Unknown Integer'); -/** - * A special key that does not actually reference a property but can be used to - * test if the right hand side of "instanceof" is either included or not a class - * or function via - * .hasEffectsOnInteractionAtPath([TestInstanceof], {type: INTERACTION_ASSIGNED,...}, ...) - */ -export const TestInstanceof = Symbol('Test instanceof'); export type ObjectPathKey = | string | typeof UnknownKey | typeof UnknownNonAccessorKey - | typeof UnknownInteger - | typeof TestInstanceof; + | typeof UnknownInteger; export type ObjectPath = ObjectPathKey[]; export const EMPTY_PATH: ObjectPath = []; export const UNKNOWN_PATH: ObjectPath = [UnknownKey]; -export const TEST_INSTANCEOF_PATH: ObjectPath = [TestInstanceof]; // For deoptimizations, this means we are modifying an unknown property but did // not lose track of the object or are creating a setter/getter; // For assignment effects it means we do not check for setter/getter effects @@ -34,7 +25,6 @@ const EntitiesKey = Symbol('Entities'); interface EntityPaths { [pathSegment: string]: EntityPaths; [EntitiesKey]: Set; - [TestInstanceof]?: EntityPaths; [UnknownInteger]?: EntityPaths; [UnknownKey]?: EntityPaths; [UnknownNonAccessorKey]?: EntityPaths; @@ -82,7 +72,6 @@ export const SHARED_RECURSION_TRACKER = new PathTracker(); interface DiscriminatedEntityPaths { [pathSegment: string]: DiscriminatedEntityPaths; [EntitiesKey]: Map>; - [TestInstanceof]?: DiscriminatedEntityPaths; [UnknownInteger]?: DiscriminatedEntityPaths; [UnknownKey]?: DiscriminatedEntityPaths; [UnknownNonAccessorKey]?: DiscriminatedEntityPaths; diff --git a/test/form/samples/instanceof/_config.js b/test/form/samples/instanceof/_config.js deleted file mode 100644 index 272579e8d05..00000000000 --- a/test/form/samples/instanceof/_config.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - description: 'removes instanceof checks if the right side is not included' -}; diff --git a/test/form/samples/instanceof/_expected.js b/test/form/samples/instanceof/_expected.js deleted file mode 100644 index 291a9f7748c..00000000000 --- a/test/form/samples/instanceof/_expected.js +++ /dev/null @@ -1,15 +0,0 @@ -console.log('retained'); - -console.log('retained'); - -class Used1 {} -const Intermediate = Used1; -const used1 = new Used1(); - -if (used1 instanceof Intermediate) console.log('retained'); -else console.log('does not matter'); - -class Used2 {} - -if (new Used2() instanceof Intermediate) console.log('retained'); -else console.log('does not matter'); diff --git a/test/form/samples/instanceof/main.js b/test/form/samples/instanceof/main.js deleted file mode 100644 index 718d04ff40a..00000000000 --- a/test/form/samples/instanceof/main.js +++ /dev/null @@ -1,21 +0,0 @@ -class Unused1 {} - -if ({} instanceof Unused1) console.log('removed'); -else console.log('retained'); - -function Unused2() {} - -if ({} instanceof Unused2) console.log('removed'); -else console.log('retained'); - -class Used1 {} -const Intermediate = Used1; -const used1 = new Used1(); - -if (used1 instanceof Intermediate) console.log('retained'); -else console.log('does not matter'); - -class Used2 {} - -if (new Used2() instanceof Intermediate) console.log('retained'); -else console.log('does not matter'); diff --git a/test/function/samples/instanceof/used-parameter/_config.js b/test/function/samples/instanceof/used-parameter/_config.js new file mode 100644 index 00000000000..1312a923985 --- /dev/null +++ b/test/function/samples/instanceof/used-parameter/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'retains instanceof for function parameters' +}; diff --git a/test/function/samples/instanceof/used-parameter/main.js b/test/function/samples/instanceof/used-parameter/main.js new file mode 100644 index 00000000000..61c8724c78d --- /dev/null +++ b/test/function/samples/instanceof/used-parameter/main.js @@ -0,0 +1,12 @@ +let effect = false; + +class Foo {} + +function checkInstance(instance) { + if (instance instanceof Foo) { + effect = true; + } +} + +checkInstance(new Foo()); +assert.ok(effect);