Skip to content

Commit

Permalink
Remove feature and add regression test
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Jun 10, 2022
1 parent b3fb1b8 commit 001639d
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 118 deletions.
56 changes: 4 additions & 52 deletions src/ast/nodes/BinaryExpression.ts
Expand Up @@ -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 =
| '!='
Expand Down Expand Up @@ -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<DeoptimizableEntity>();

deoptimizeCache(): void {}

Expand All @@ -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;

Expand All @@ -127,19 +104,13 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE
) {
return true;
}
this.deoptimizeInstanceof(context);
return super.hasEffects(context);
}

hasEffectsOnInteractionAtPath(path: ObjectPath, { type }: NodeInteraction): boolean {
return type !== INTERACTION_ACCESSED || path.length > 1;
}

include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
this.deoptimizeInstanceof();
super.include(context, includeChildrenRecursively);
}

render(
code: MagicString,
options: RenderOptions,
Expand All @@ -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();
}
}
}
3 changes: 0 additions & 3 deletions src/ast/nodes/shared/ClassNode.ts
Expand Up @@ -13,7 +13,6 @@ import {
type ObjectPath,
type PathTracker,
SHARED_RECURSION_TRACKER,
TestInstanceof,
UNKNOWN_PATH,
UnknownKey
} from '../../utils/PathTracker';
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 1 addition & 10 deletions src/ast/nodes/shared/FunctionBase.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 0 additions & 2 deletions src/ast/nodes/shared/ObjectEntity.ts
Expand Up @@ -11,7 +11,6 @@ import {
ObjectPath,
ObjectPathKey,
PathTracker,
TestInstanceof,
UNKNOWN_INTEGER_PATH,
UNKNOWN_PATH,
UnknownInteger,
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 1 addition & 12 deletions src/ast/utils/PathTracker.ts
Expand Up @@ -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
Expand All @@ -34,7 +25,6 @@ const EntitiesKey = Symbol('Entities');
interface EntityPaths {
[pathSegment: string]: EntityPaths;
[EntitiesKey]: Set<Entity>;
[TestInstanceof]?: EntityPaths;
[UnknownInteger]?: EntityPaths;
[UnknownKey]?: EntityPaths;
[UnknownNonAccessorKey]?: EntityPaths;
Expand Down Expand Up @@ -82,7 +72,6 @@ export const SHARED_RECURSION_TRACKER = new PathTracker();
interface DiscriminatedEntityPaths {
[pathSegment: string]: DiscriminatedEntityPaths;
[EntitiesKey]: Map<unknown, Set<Entity>>;
[TestInstanceof]?: DiscriminatedEntityPaths;
[UnknownInteger]?: DiscriminatedEntityPaths;
[UnknownKey]?: DiscriminatedEntityPaths;
[UnknownNonAccessorKey]?: DiscriminatedEntityPaths;
Expand Down
3 changes: 0 additions & 3 deletions test/form/samples/instanceof/_config.js

This file was deleted.

15 changes: 0 additions & 15 deletions test/form/samples/instanceof/_expected.js

This file was deleted.

21 changes: 0 additions & 21 deletions test/form/samples/instanceof/main.js

This file was deleted.

3 changes: 3 additions & 0 deletions test/function/samples/instanceof/used-parameter/_config.js
@@ -0,0 +1,3 @@
module.exports = {
description: 'retains instanceof for function parameters'
};
12 changes: 12 additions & 0 deletions 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);

0 comments on commit 001639d

Please sign in to comment.