Fix side effect and self-reference handling

src/ast/nodes/BinaryExpression.ts

@@ -3,7 +3,7 @@ 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 } from '../ExecutionContext';
+import { createHasEffectsContext, createInclusionContext } from '../ExecutionContext';
 import {
 	INTERACTION_ACCESSED,
 	NODE_INTERACTION_UNKNOWN_ASSIGNMENT,
@@ -14,7 +14,7 @@ import {
 	type ObjectPath,
 	type PathTracker,
 	SHARED_RECURSION_TRACKER,
-	TEST_INCLUSION_PATH
+	TEST_INSTANCEOF_PATH
 } from '../utils/PathTracker';
 import ExpressionStatement from './ExpressionStatement';
 import type { LiteralValue } from './Literal';
@@ -59,15 +59,13 @@ const binaryOperators: {
 	'+': (left: any, right: any) => left + right,
 	'-': (left: any, right: any) => left - right,
 	'/': (left: any, right: any) => left / right,
-	'<': (left, right) => (left as NonNullable<LiteralValue>) < (right as NonNullable<LiteralValue>),
+	'<': (left, right) => left! < right!,
 	'<<': (left: any, right: any) => left << right,
-	'<=': (left, right) =>
-		(left as NonNullable<LiteralValue>) <= (right as NonNullable<LiteralValue>),
+	'<=': (left, right) => left! <= right!,
 	'==': (left, right) => left == right,
 	'===': (left, right) => left === right,
-	'>': (left, right) => (left as NonNullable<LiteralValue>) > (right as NonNullable<LiteralValue>),
-	'>=': (left, right) =>
-		(left as NonNullable<LiteralValue>) >= (right as NonNullable<LiteralValue>),
+	'>': (left, right) => left! > right!,
+	'>=': (left, right) => left! >= right!,
 	'>>': (left: any, right: any) => left >> right,
 	'>>>': (left: any, right: any) => left >>> right,
 	'^': (left: any, right: any) => left ^ right,
@@ -93,9 +91,12 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE
 	): 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_INCLUSION_PATH,
+					TEST_INSTANCEOF_PATH,
 					NODE_INTERACTION_UNKNOWN_ASSIGNMENT,
 					createHasEffectsContext()
 				)
@@ -134,12 +135,9 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE
 		return type !== INTERACTION_ACCESSED || path.length > 1;
 	}
 
-	// TODO Lukas if the operator is instanceof, only include if necessary
 	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren) {
 		this.deoptimizeInstanceof();
-		if (!this.expressionsDependingOnNegativeInstanceof.size) {
-			super.include(context, includeChildrenRecursively);
-		}
+		super.include(context, includeChildrenRecursively);
 	}
 
 	render(
@@ -154,15 +152,17 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE
 	private deoptimizeInstanceof(context?: HasEffectsContext) {
 		if (
 			this.operator === 'instanceof' &&
-			this.expressionsDependingOnNegativeInstanceof.size &&
-			this.right.hasEffectsOnInteractionAtPath(
-				TEST_INCLUSION_PATH,
-				NODE_INTERACTION_UNKNOWN_ASSIGNMENT,
-				context || createHasEffectsContext()
-			)
+			(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();
 		}

src/ast/nodes/NewExpression.ts

@@ -28,8 +28,9 @@ export default class NewExpression extends NodeBase {
 			if (
 				(this.context.options.treeshake as NormalizedTreeshakingOptions).annotations &&
 				this.annotations
-			)
+			) {
 				return false;
+			}
 			return (
 				this.callee.hasEffects(context) ||
 				this.callee.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)

src/ast/nodes/shared/ClassNode.ts

@@ -13,6 +13,7 @@ import {
 	type ObjectPath,
 	type PathTracker,
 	SHARED_RECURSION_TRACKER,
+	TestInstanceof,
 	UNKNOWN_PATH,
 	UnknownKey
 } from '../../utils/PathTracker';
@@ -95,6 +96,8 @@ 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);
 		}

src/ast/nodes/shared/FunctionBase.ts

@@ -14,7 +14,13 @@ import {
 	NodeInteractionWithThisArg
 } from '../../NodeInteractions';
 import ReturnValueScope from '../../scopes/ReturnValueScope';
-import { type ObjectPath, PathTracker, UNKNOWN_PATH, UnknownKey } from '../../utils/PathTracker';
+import {
+	type ObjectPath,
+	PathTracker,
+	TestInstanceof,
+	UNKNOWN_PATH,
+	UnknownKey
+} from '../../utils/PathTracker';
 import BlockStatement from '../BlockStatement';
 import * as NodeType from '../NodeType';
 import RestElement from '../RestElement';
@@ -95,6 +101,9 @@ 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);
 		}

src/ast/nodes/shared/ObjectEntity.ts

@@ -11,7 +11,7 @@ import {
 	ObjectPath,
 	ObjectPathKey,
 	PathTracker,
-	TestInclusionKey,
+	TestInstanceof,
 	UNKNOWN_INTEGER_PATH,
 	UNKNOWN_PATH,
 	UnknownInteger,
@@ -282,7 +282,7 @@ export class ObjectEntity extends ExpressionEntity {
 		context: HasEffectsContext
 	): boolean {
 		const [key, ...subPath] = path;
-		if (key === TestInclusionKey) return false;
+		if (key === TestInstanceof) return true;
 		if (subPath.length || interaction.type === INTERACTION_CALLED) {
 			const expressionAtPath = this.getMemberExpression(key);
 			if (expressionAtPath) {

src/ast/utils/PathTracker.ts

@@ -6,21 +6,22 @@ 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 a variable is included via
- * .hasEffectsOnInteractionAtPath([TestInclusionKey], {type: INTERACTION_ASSIGNED,...}, ...)
+ * 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 TestInclusionKey = Symbol('Test Inclusion Key');
+export const TestInstanceof = Symbol('Test instanceof');
 export type ObjectPathKey =
 	| string
 	| typeof UnknownKey
 	| typeof UnknownNonAccessorKey
 	| typeof UnknownInteger
-	| typeof TestInclusionKey;
+	| typeof TestInstanceof;
 
 export type ObjectPath = ObjectPathKey[];
 export const EMPTY_PATH: ObjectPath = [];
 export const UNKNOWN_PATH: ObjectPath = [UnknownKey];
-export const TEST_INCLUSION_PATH: ObjectPath = [TestInclusionKey];
+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
@@ -33,7 +34,7 @@ const EntitiesKey = Symbol('Entities');
 interface EntityPaths {
 	[pathSegment: string]: EntityPaths;
 	[EntitiesKey]: Set<Entity>;
-	[TestInclusionKey]?: EntityPaths;
+	[TestInstanceof]?: EntityPaths;
 	[UnknownInteger]?: EntityPaths;
 	[UnknownKey]?: EntityPaths;
 	[UnknownNonAccessorKey]?: EntityPaths;
@@ -81,7 +82,7 @@ export const SHARED_RECURSION_TRACKER = new PathTracker();
 interface DiscriminatedEntityPaths {
 	[pathSegment: string]: DiscriminatedEntityPaths;
 	[EntitiesKey]: Map<unknown, Set<Entity>>;
-	[TestInclusionKey]?: DiscriminatedEntityPaths;
+	[TestInstanceof]?: DiscriminatedEntityPaths;
 	[UnknownInteger]?: DiscriminatedEntityPaths;
 	[UnknownKey]?: DiscriminatedEntityPaths;
 	[UnknownNonAccessorKey]?: DiscriminatedEntityPaths;

test/form/samples/instanceof/_expected.js

@@ -1,3 +1,15 @@
 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');

test/form/samples/instanceof/main.js

@@ -1,17 +1,21 @@
 class Unused1 {}
-class Used1 {}
-const Intermediate = Used1;
 
-if (new Intermediate() instanceof Unused1) console.log('removed');
+if ({} instanceof Unused1) console.log('removed');
 else console.log('retained');
 
-class Unused2 {}
-class WithEffect {
-	constructor() {
-		console.log('effect');
-	}
-}
+function Unused2() {}
 
-if (new WithEffect() instanceof Unused2)
-	console.log('does not matter, but effect should be retained');
+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');

test/form/samples/invalid-binary-expressions/_expected.js

@@ -37,3 +37,7 @@ if (null instanceof true) {
 if (null instanceof 'y') {
 	console.log('retained');
 }
+
+if (null instanceof {}) {
+	console.log('retained');
+}

test/function/samples/instanceof/left-hand-effect/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'retains side effects in the left hand side of instanceof'
+};

test/function/samples/instanceof/left-hand-effect/main.js

@@ -0,0 +1,14 @@
+let effect = false;
+
+class Bar {}
+class Foo {
+	constructor() {
+		effect = true;
+	}
+}
+
+if (new Foo() instanceof Bar) {
+	assert.fail('Wrong instance relation');
+}
+
+assert.ok(effect);

test/function/samples/instanceof/right-hand-effect/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'retains side effects in the right hand side of instanceof'
+};

test/function/samples/instanceof/right-hand-effect/main.js

@@ -0,0 +1,14 @@
+let effect = false;
+
+class Foo {}
+
+if (
+	new Foo() instanceof
+	class {
+		[(effect = true)]() {}
+	}
+) {
+	assert.fail('Wrong instance relation');
+}
+
+assert.ok(effect);

test/function/samples/instanceof/used/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'retains instanceof if it is true'
+};

test/function/samples/instanceof/used/main.js

@@ -0,0 +1,5 @@
+class Foo {}
+
+if (!(new Foo() instanceof Foo)) {
+	assert.fail('instanceof not resolved correctly');
+}