Deoptimize try-catch less radically (#2918)

* Test against polyfill regressions

* Make try-statement deoptimization less aggressive by not following variables

* Update docs

docs/999-big-list-of-options.md

@@ -855,25 +855,12 @@ Type: `boolean`<br>
 CLI: `--treeshake.tryCatchDeoptimization`/`--no-treeshake.tryCatchDeoptimization`<br>
 Default: `true`
 
-By default, Rollup assumes that many builtin globals of the runtime behave according to the latest specs when tree-shaking and do not throw unexpected errors. In order to support e.g. feature detection workflows that rely on those errors being thrown, Rollup will by default deactivate tree-shaking inside try-statements. Furthermore, it will also deactivate tree-shaking inside functions that are called directly from a try-statement if Rollup can resolve the function. Set `treeshake.tryCatchDeoptimization` to `false` if you do not need this feature and want to have tree-shaking inside try-statements as well as inside functions called from those statements.
+By default, Rollup assumes that many builtin globals of the runtime behave according to the latest specs when tree-shaking and do not throw unexpected errors. In order to support e.g. feature detection workflows that rely on those errors being thrown, Rollup will by default deactivate tree-shaking inside try-statements. If a function parameter is called from within a try-statement, this parameter will be deoptimized as well. Set `treeshake.tryCatchDeoptimization` to `false` if you do not need this feature and want to have tree-shaking inside try-statements.
 
 ```js
-function directlyCalled1() {
-  // as this function is directly called from a try-statement, it will be
-  // retained unmodified for tryCatchDeoptimization: true including staements
-  // that would usually be removed 
-  Object.create(null);
-  notDirectlyCalled();
-}
-
-function directlyCalled2() {
-  Object.create(null);
-  notDirectlyCalled();
-}
-
-function notDirectlyCalled() {
-  // even if this function is retained, this will be removed as the function is
-  // never directly called from a try-statement
+function otherFn() {
+  // even though this function is called from a try-statement, the next line
+  // will be removed as side-effect-free
   Object.create(null);
 }
 
@@ -883,20 +870,23 @@ function test(callback) {
   	// inside try-statements for tryCatchDeoptimization: true
     Object.create(null);
 
-  	// directly resolvable calls will also be deoptimized
-    directlyCalled1();
+  	// calls to other function are retained as well but the body of this
+  	// function may again be subject to tree-shaking
+    otherFn();
 
     // if a parameter is called, then all arguments passed to that function
     // parameter will be deoptimized
     callback();
-
-    // all calls will be retained but only calls of the form
-    // "identifier(someArguments)" will also deoptimize the target
-    (notDirectlyCalled && notDirectlyCalled)();
   } catch {}
 }
 
-test(directlyCalled2);
+test(() => {
+  // will be ratained
+  Object.create(null)
+});
+
+// call will be retained but again, otherFn is not deoptimized
+test(otherFn);
 
 ```
 

package.json

@@ -76,7 +76,10 @@
     "buble": "^0.19.7",
     "chokidar": "^2.1.6",
     "console-group": "^0.3.3",
+    "core-js": "^3.1.3",
     "date-time": "^3.1.0",
+    "es5-shim": "^4.5.13",
+    "es6-shim": "^0.35.5",
     "eslint": "^5.16.0",
     "eslint-plugin-import": "^2.17.3",
     "execa": "^1.0.0",

src/ast/nodes/CallExpression.ts

@@ -23,7 +23,7 @@ import {
 import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import { ExpressionEntity } from './shared/Expression';
-import { ExpressionNode, INCLUDE_VARIABLES, IncludeChildren, NodeBase } from './shared/Node';
+import { ExpressionNode, INCLUDE_PARAMETERS, IncludeChildren, NodeBase } from './shared/Node';
 import SpreadElement from './SpreadElement';
 
 export default class CallExpression extends NodeBase implements DeoptimizableEntity {
@@ -204,11 +204,11 @@ export default class CallExpression extends NodeBase implements DeoptimizableEnt
 		if (includeChildrenRecursively) {
 			super.include(includeChildrenRecursively);
 			if (
-				includeChildrenRecursively === INCLUDE_VARIABLES &&
+				includeChildrenRecursively === INCLUDE_PARAMETERS &&
 				this.callee instanceof Identifier &&
 				this.callee.variable
 			) {
-				this.callee.variable.includeInitRecursively();
+				this.callee.variable.markCalledFromTryStatement();
 			}
 		} else {
 			this.included = true;

src/ast/nodes/TryStatement.ts

@@ -2,7 +2,7 @@ import { ExecutionPathOptions } from '../ExecutionPathOptions';
 import BlockStatement from './BlockStatement';
 import CatchClause from './CatchClause';
 import * as NodeType from './NodeType';
-import { INCLUDE_VARIABLES, IncludeChildren, StatementBase } from './shared/Node';
+import { INCLUDE_PARAMETERS, IncludeChildren, StatementBase } from './shared/Node';
 
 export default class TryStatement extends StatementBase {
 	block!: BlockStatement;
@@ -25,7 +25,7 @@ export default class TryStatement extends StatementBase {
 			this.included = true;
 			this.directlyIncluded = true;
 			this.block.include(
-				this.context.tryCatchDeoptimization ? INCLUDE_VARIABLES : includeChildrenRecursively
+				this.context.tryCatchDeoptimization ? INCLUDE_PARAMETERS : includeChildrenRecursively
 			);
 		}
 		if (this.handler !== null) {

src/ast/nodes/shared/Node.ts

@@ -20,8 +20,8 @@ export interface GenericEsTreeNode {
 	[key: string]: any;
 }
 
-export const INCLUDE_VARIABLES: 'variables' = 'variables';
-export type IncludeChildren = boolean | typeof INCLUDE_VARIABLES;
+export const INCLUDE_PARAMETERS: 'variables' = 'variables';
+export type IncludeChildren = boolean | typeof INCLUDE_PARAMETERS;
 
 export interface Node extends Entity {
 	annotations?: CommentDescription[];

src/ast/scopes/ParameterScope.ts

@@ -42,30 +42,27 @@ export default class ParameterScope extends ChildScope {
 	}
 
 	includeCallArguments(args: (ExpressionNode | SpreadElement)[]): void {
-		let hasInitBeenForceIncluded = false;
+		let calledFromTryStatement = false;
 		let argIncluded = false;
 		const restParam = this.hasRest && this.parameters[this.parameters.length - 1];
 		for (let index = args.length - 1; index >= 0; index--) {
 			const paramVars = this.parameters[index] || restParam;
 			const arg = args[index];
 			if (paramVars) {
-				hasInitBeenForceIncluded = false;
+				calledFromTryStatement = false;
 				for (const variable of paramVars) {
 					if (variable.included) {
 						argIncluded = true;
 					}
-					if (variable.hasInitBeenForceIncluded) {
-						hasInitBeenForceIncluded = true;
+					if (variable.calledFromTryStatement) {
+						calledFromTryStatement = true;
 					}
 				}
 			} else if (!argIncluded && arg.shouldBeIncluded()) {
 				argIncluded = true;
 			}
 			if (argIncluded) {
-				arg.include(hasInitBeenForceIncluded);
-				if (hasInitBeenForceIncluded && arg instanceof Identifier && arg.variable) {
-					arg.variable.includeInitRecursively();
-				}
+				arg.include(calledFromTryStatement);
 			}
 		}
 	}

src/ast/variables/LocalVariable.ts

@@ -25,8 +25,8 @@ const MAX_PATH_DEPTH = 7;
 
 export default class LocalVariable extends Variable {
 	additionalInitializers: ExpressionEntity[] | null = null;
+	calledFromTryStatement = false;
 	declarations: (Identifier | ExportDefaultDeclaration)[];
-	hasInitBeenForceIncluded = false;
 	init: ExpressionEntity | null;
 	module: Module;
 
@@ -203,12 +203,7 @@ export default class LocalVariable extends Variable {
 		}
 	}
 
-	includeInitRecursively() {
-		if (!this.hasInitBeenForceIncluded) {
-			this.hasInitBeenForceIncluded = true;
-			if (this.init && this.init !== UNKNOWN_EXPRESSION) {
-				this.init.include(true);
-			}
-		}
+	markCalledFromTryStatement() {
+		this.calledFromTryStatement = true;
 	}
 }

src/ast/variables/Variable.ts

@@ -91,7 +91,7 @@ export default class Variable implements ExpressionEntity {
 		}
 	}
 
-	includeInitRecursively() {}
+	markCalledFromTryStatement() {}
 
 	setRenderNames(baseName: string | null, name: string | null) {
 		this.renderBaseName = baseName;

test/form/samples/supports-core-js/_config.js

@@ -0,0 +1,8 @@
+module.exports = {
+	description: 'supports core-js',
+	options: {
+		// check against tree-shake: false when updating the polyfill
+		treeshake: true,
+		plugins: [require('rollup-plugin-node-resolve')(), require('rollup-plugin-commonjs')()]
+	}
+};