Make sure the initializer of hoisted variables is deoptimized (#4149)

* Make sure the initializer of hoisted variables is deoptimized

* Also deoptimize var initializers when force deoptimizing via flag

* Do not check types on Windows

.github/workflows/tests.yml

@@ -85,6 +85,6 @@ jobs:
       - name: Install dependencies
         run: npm ci --ignore-scripts
       - name: Run tests
-        run: npm test
+        run: npm run ci:test:only
         env:
           CI: true

package.json

@@ -11,7 +11,7 @@
   "scripts": {
     "build": "shx rm -rf dist && git rev-parse HEAD > .commithash && rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
     "build:cjs": "shx rm -rf dist && rollup --config rollup.config.ts --configPlugin typescript --configTest && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
-    "build:bootstrap": "dist/bin/rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
+    "build:bootstrap": "node dist/bin/rollup --config rollup.config.ts --configPlugin typescript && shx cp src/rollup/types.d.ts dist/rollup.d.ts && shx chmod a+x dist/bin/rollup",
     "ci:lint": "npm run lint:nofix",
     "ci:test": "npm run build:cjs && npm run build:bootstrap && npm run test:all",
     "ci:test:only": "npm run build:cjs && npm run build:bootstrap && npm run test:only",

src/ast/nodes/Identifier.ts

@@ -9,12 +9,13 @@ import { HasEffectsContext, InclusionContext } from '../ExecutionContext';
 import { NodeEvent } from '../NodeEvents';
 import FunctionScope from '../scopes/FunctionScope';
 import { EMPTY_PATH, ObjectPath, PathTracker } from '../utils/PathTracker';
+import { UNDEFINED_EXPRESSION } from '../values';
 import GlobalVariable from '../variables/GlobalVariable';
 import LocalVariable from '../variables/LocalVariable';
 import Variable from '../variables/Variable';
 import * as NodeType from './NodeType';
 import SpreadElement from './SpreadElement';
-import { ExpressionEntity, LiteralValueOrUnknown, UNKNOWN_EXPRESSION } from './shared/Expression';
+import { ExpressionEntity, LiteralValueOrUnknown } from './shared/Expression';
 import { ExpressionNode, NodeBase } from './shared/Node';
 import { PatternNode } from './shared/Pattern';
 
@@ -48,12 +49,11 @@ export default class Identifier extends NodeBase implements PatternNode {
 		const { treeshake } = this.context.options;
 		switch (kind) {
 			case 'var':
-				variable = this.scope.addDeclaration(
-					this,
-					this.context,
-					treeshake && treeshake.correctVarValueBeforeDeclaration ? UNKNOWN_EXPRESSION : init,
-					true
-				);
+				variable = this.scope.addDeclaration(this, this.context, init, true);
+				if (treeshake && treeshake.correctVarValueBeforeDeclaration) {
+					// Necessary to make sure the init is deoptimized. We cannot call deoptimizePath here.
+					this.scope.addDeclaration(this, this.context, UNDEFINED_EXPRESSION, true);
+				}
 				break;
 			case 'function':
 				// in strict mode, functions are only hoisted within a scope but not across block scopes

src/ast/scopes/BlockScope.ts

@@ -1,6 +1,7 @@
 import { AstContext } from '../../Module';
 import Identifier from '../nodes/Identifier';
-import { ExpressionEntity, UNKNOWN_EXPRESSION } from '../nodes/shared/Expression';
+import { ExpressionEntity } from '../nodes/shared/Expression';
+import { UNDEFINED_EXPRESSION } from '../values';
 import LocalVariable from '../variables/LocalVariable';
 import ChildScope from './ChildScope';
 
@@ -12,7 +13,9 @@ export default class BlockScope extends ChildScope {
 		isHoisted: boolean
 	): LocalVariable {
 		if (isHoisted) {
-			return this.parent.addDeclaration(identifier, context, UNKNOWN_EXPRESSION, isHoisted);
+			this.parent.addDeclaration(identifier, context, init, isHoisted);
+			// Necessary to make sure the init is deoptimized. We cannot call deoptimizePath here.
+			return this.parent.addDeclaration(identifier, context, UNDEFINED_EXPRESSION, isHoisted);
 		} else {
 			return super.addDeclaration(identifier, context, init, false);
 		}

src/ast/scopes/CatchScope.ts

@@ -16,8 +16,7 @@ export default class CatchScope extends ParameterScope {
 			existingParameter.addDeclaration(identifier, init);
 			return existingParameter;
 		}
-		// as parameters are handled differently, all remaining declarations are
-		// hoisted
+		// as parameters are handled differently, all remaining declarations are hoisted
 		return this.parent.addDeclaration(identifier, context, init, isHoisted);
 	}
 }

test/form/samples/deoptimize-var-in-hoisted-scopes/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'deoptimizes var variables in hoisted scopes'
+};

test/form/samples/deoptimize-var-in-hoisted-scopes/_expected.js

@@ -0,0 +1,15 @@
+const obj1 = { flag: false };
+{
+	var foo = obj1;
+	foo.flag = true;
+}
+if (obj1.flag) console.log('retained');
+
+const obj2 = { flag: false };
+try {
+	throw new Error();
+} catch {
+	var foo = obj2;
+	foo.flag = true;
+}
+if (obj2.flag) console.log('retained');

test/form/samples/deoptimize-var-in-hoisted-scopes/main.js

@@ -0,0 +1,15 @@
+const obj1 = { flag: false };
+{
+	var foo = obj1;
+	foo.flag = true;
+}
+if (obj1.flag) console.log('retained');
+
+const obj2 = { flag: false };
+try {
+	throw new Error();
+} catch {
+	var foo = obj2;
+	foo.flag = true;
+}
+if (obj2.flag) console.log('retained');

test/function/samples/correct-var-before-declaration-deopt/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+	description: 'adds necessary deoptimizations when using treeshake.correctVarBeforeDeclaration',
+	options: {
+		treeshake: { correctVarValueBeforeDeclaration: true }
+	}
+};

test/function/samples/correct-var-before-declaration-deopt/_expected.js

@@ -0,0 +1,15 @@
+const obj1 = { flag: false };
+{
+	var foo = obj1;
+	foo.flag = true;
+}
+if (obj1.flag) console.log('retained');
+
+const obj2 = { flag: false };
+try {
+	throw new Error();
+} catch {
+	var foo = obj2;
+	foo.flag = true;
+}
+if (obj2.flag) console.log('retained');

test/function/samples/correct-var-before-declaration-deopt/main.js

@@ -0,0 +1,7 @@
+const obj = { flag: false };
+var foo = obj;
+foo.flag = true;
+assert.ok(obj.flag ? true : false, 'init deoptimization');
+
+assert.ok(bar ? false : true, 'value deoptimization');
+var bar = true;