From 8dea52989cac954b3db39dce1ab71a9bb23aacbc Mon Sep 17 00:00:00 2001 From: Justin Ridgewell Date: Sun, 19 Sep 2021 14:50:00 -0400 Subject: [PATCH] Fix compress/eval of optional chains (#1062) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix compress/eval of optional chains This implements the recursion necessary to determine if a property access/call is part of an optional chain that has short-circuited. * fix evaluation of chain * Apply suggestions from code review Co-authored-by: Fábio Santos * Add test cases for future optional chain dropping * Only drop expression if optional chaining is used * Ensure nullish is never returned from evaluate Co-authored-by: Fábio Santos --- lib/compress/drop-side-effect-free.js | 21 +++-- lib/compress/evaluate.js | 27 ++++--- lib/compress/index.js | 12 +-- lib/compress/inference.js | 40 +++++++--- test/compress/drop-unused.js | 111 +++++++++++++++++++++++++- test/compress/evaluate.js | 25 +++++- 6 files changed, 184 insertions(+), 52 deletions(-) diff --git a/lib/compress/drop-side-effect-free.js b/lib/compress/drop-side-effect-free.js index 34fddbf75..60ee014af 100644 --- a/lib/compress/drop-side-effect-free.js +++ b/lib/compress/drop-side-effect-free.js @@ -73,13 +73,12 @@ import { AST_TemplateString, AST_This, AST_Unary, - AST_Undefined, } from "../ast.js"; import { make_node, return_null, return_this } from "../utils/index.js"; import { first_in_statement } from "../utils/first_in_statement.js"; import { pure_prop_access_globals } from "./native-objects.js"; -import { lazy_op, unary_side_effects, is_nullish } from "./inference.js"; +import { lazy_op, unary_side_effects, is_nullish_shortcircuited } from "./inference.js"; import { WRITE_ONLY, set_flag, clear_flag } from "./compressor-flags.js"; import { make_sequence, is_func_expr, is_iife_call } from "./common.js"; @@ -121,8 +120,8 @@ def_drop_side_effect_free(AST_Constant, return_null); def_drop_side_effect_free(AST_This, return_null); def_drop_side_effect_free(AST_Call, function (compressor, first_in_statement) { - if (this.optional && is_nullish(this.expression, compressor)) { - return make_node(AST_Undefined, this); + if (is_nullish_shortcircuited(this, compressor)) { + return this.expression.drop_side_effect_free(compressor, first_in_statement); } if (!this.is_callee_pure(compressor)) { @@ -298,21 +297,19 @@ def_drop_side_effect_free(AST_Array, function (compressor, first_in_statement) { }); def_drop_side_effect_free(AST_Dot, function (compressor, first_in_statement) { - if (this.optional) { - return is_nullish(this.expression, compressor) ? make_node(AST_Undefined, this) : this; + if (is_nullish_shortcircuited(this, compressor)) { + return this.expression.drop_side_effect_free(compressor, first_in_statement); } - if (this.expression.may_throw_on_access(compressor)) - return this; + if (this.expression.may_throw_on_access(compressor)) return this; return this.expression.drop_side_effect_free(compressor, first_in_statement); }); def_drop_side_effect_free(AST_Sub, function (compressor, first_in_statement) { - if (this.optional) { - return is_nullish(this.expression, compressor) ? make_node(AST_Undefined, this) : this; + if (is_nullish_shortcircuited(this, compressor)) { + return this.expression.drop_side_effect_free(compressor, first_in_statement); } - if (this.expression.may_throw_on_access(compressor)) - return this; + if (this.expression.may_throw_on_access(compressor)) return this; var expression = this.expression.drop_side_effect_free(compressor, first_in_statement); if (!expression) diff --git a/lib/compress/evaluate.js b/lib/compress/evaluate.js index 1a0f9c19a..a9c52e7c7 100644 --- a/lib/compress/evaluate.js +++ b/lib/compress/evaluate.js @@ -82,6 +82,9 @@ function def_eval(node, func) { node.DEFMETHOD("_eval", func); } +// Used to propagate a nullish short-circuit signal upwards through the chain. +export const nullish = Symbol("This AST_Chain is nullish"); + // If the node has been successfully reduced to a constant, // then its value is returned; otherwise the element itself // is returned. @@ -93,7 +96,7 @@ AST_Node.DEFMETHOD("evaluate", function (compressor) { var val = this._eval(compressor, 1); if (!val || val instanceof RegExp) return val; - if (typeof val == "function" || typeof val == "object") + if (typeof val == "function" || typeof val == "object" || val == nullish) return this; return val; }); @@ -338,11 +341,8 @@ const regexp_flags = new Set([ ]); def_eval(AST_PropAccess, function (compressor, depth) { - if (this.optional) { - const obj = this.expression._eval(compressor, depth); - if (obj == null) - return undefined; - } + const obj = this.expression._eval(compressor, depth); + if (obj === nullish || (this.optional && obj == null)) return nullish; if (compressor.option("unsafe")) { var key = this.property; if (key instanceof AST_Node) { @@ -397,16 +397,19 @@ def_eval(AST_PropAccess, function (compressor, depth) { def_eval(AST_Chain, function (compressor, depth) { const evaluated = this.expression._eval(compressor, depth); - return evaluated === this.expression ? this : evaluated; + return evaluated === nullish + ? undefined + : evaluated === this.expression + ? this + : evaluated; }); def_eval(AST_Call, function (compressor, depth) { var exp = this.expression; - if (this.optional) { - const callee = this.expression._eval(compressor, depth); - if (callee == null) - return undefined; - } + + const callee = exp._eval(compressor, depth); + if (callee === nullish || (this.optional && callee == null)) return nullish; + if (compressor.option("unsafe") && exp instanceof AST_PropAccess) { var key = exp.property; if (key instanceof AST_Node) { diff --git a/lib/compress/index.js b/lib/compress/index.js index 85b684d94..6cb333340 100644 --- a/lib/compress/index.js +++ b/lib/compress/index.js @@ -2012,10 +2012,6 @@ def_optimize(AST_Call, function(self, compressor) { } } - if (self.optional && is_nullish(fn, compressor)) { - return make_node(AST_Undefined, self); - } - var is_func = fn instanceof AST_Lambda; if (is_func && fn.pinned()) return self; @@ -4200,14 +4196,11 @@ def_optimize(AST_Sub, function(self, compressor) { ev = make_node_from_constant(ev, self).optimize(compressor); return best_of(compressor, ev, self); } - if (self.optional && is_nullish(self.expression, compressor)) { - return make_node(AST_Undefined, self); - } return self; }); def_optimize(AST_Chain, function (self, compressor) { - self.expression = self.expression.optimize(compressor); + if (is_nullish(self.expression, compressor)) return make_node(AST_Undefined, self); return self; }); @@ -4274,9 +4267,6 @@ def_optimize(AST_Dot, function(self, compressor) { ev = make_node_from_constant(ev, self).optimize(compressor); return best_of(compressor, ev, self); } - if (self.optional && is_nullish(self.expression, compressor)) { - return make_node(AST_Undefined, self); - } return self; }); diff --git a/lib/compress/inference.js b/lib/compress/inference.js index 4646bd733..0b2c98294 100644 --- a/lib/compress/inference.js +++ b/lib/compress/inference.js @@ -226,9 +226,8 @@ export function is_undefined(node, compressor) { ); } -// Find out if something is == null -// Used to optimize ?. and ?? -export function is_nullish(node, compressor) { +// Is the node explicitly null or undefined. +function is_null_or_undefined(node, compressor) { let fixed; return ( node instanceof AST_Null @@ -238,13 +237,29 @@ export function is_nullish(node, compressor) { && (fixed = node.definition().fixed) instanceof AST_Node && is_nullish(fixed, compressor) ) - // Recurse into those optional chains! - || node instanceof AST_PropAccess && node.optional && is_nullish(node.expression, compressor) - || node instanceof AST_Call && node.optional && is_nullish(node.expression, compressor) - || node instanceof AST_Chain && is_nullish(node.expression, compressor) ); } +// Find out if this expression is optionally chained from a base-point that we +// can statically analyze as null or undefined. +export function is_nullish_shortcircuited(node, compressor) { + if (node instanceof AST_PropAccess || node instanceof AST_Call) { + return ( + (node.optional && is_null_or_undefined(node.expression, compressor)) + || is_nullish_shortcircuited(node.expression, compressor) + ); + } + if (node instanceof AST_Chain) return is_nullish_shortcircuited(node.expression, compressor); + return false; +} + +// Find out if something is == null, or can short circuit into nullish. +// Used to optimize ?. and ?? +export function is_nullish(node, compressor) { + if (is_null_or_undefined(node, compressor)) return true; + return is_nullish_shortcircuited(node, compressor); +} + // Determine if expression might cause side effects // If there's a possibility that a node may change something when it's executed, this returns true (function(def_has_side_effects) { @@ -352,13 +367,12 @@ export function is_nullish(node, compressor) { return any(this.elements, compressor); }); def_has_side_effects(AST_Dot, function(compressor) { + if (is_nullish(this, compressor)) return false; return !this.optional && this.expression.may_throw_on_access(compressor) || this.expression.has_side_effects(compressor); }); def_has_side_effects(AST_Sub, function(compressor) { - if (this.optional && is_nullish(this.expression, compressor)) { - return false; - } + if (is_nullish(this, compressor)) return false; return !this.optional && this.expression.may_throw_on_access(compressor) || this.expression.has_side_effects(compressor) @@ -426,7 +440,7 @@ export function is_nullish(node, compressor) { return any(this.body, compressor); }); def_may_throw(AST_Call, function(compressor) { - if (this.optional && is_nullish(this.expression, compressor)) return false; + if (is_nullish(this, compressor)) return false; if (any(this.args, compressor)) return true; if (this.is_callee_pure(compressor)) return false; if (this.expression.may_throw(compressor)) return true; @@ -485,12 +499,12 @@ export function is_nullish(node, compressor) { return this.body.may_throw(compressor); }); def_may_throw(AST_Dot, function(compressor) { + if (is_nullish(this, compressor)) return false; return !this.optional && this.expression.may_throw_on_access(compressor) || this.expression.may_throw(compressor); }); def_may_throw(AST_Sub, function(compressor) { - if (this.optional && is_nullish(this.expression, compressor)) return false; - + if (is_nullish(this, compressor)) return false; return !this.optional && this.expression.may_throw_on_access(compressor) || this.expression.may_throw(compressor) || this.property.may_throw(compressor); diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js index 1fd32138e..93e2efc7c 100644 --- a/test/compress/drop-unused.js +++ b/test/compress/drop-unused.js @@ -2927,8 +2927,115 @@ unused_null_conditional_chain: { null?.maybe_call?.(3); } expect: { - undefined.i_will_throw; - undefined(1); } } +unused_null_conditional_chain_2: { + options = { + toplevel: true, + defaults: true, + passes: 5, + } + input: { + let i = 0; + (i++, null)?.unused; + (i++, null)?.[side_effect()]; + (i++, null)?.unused.i_will_throw; + (i++, null)?.call(1); + (i++, null)?.(2); + (i++, null)?.maybe_call?.(3); + } + expect: { + } +} + +unused_null_conditional_chain_3: { + options = { + toplevel: true, + defaults: true, + passes: 5, + } + input: { + (side_effect(), null)?.unused; + (side_effect(), null)?.[side_effect()]; + (side_effect(), null)?.unused.i_will_throw; + (side_effect(), null)?.call(1); + (side_effect(), null)?.(2); + (side_effect(), null)?.maybe_call?.(3); + } + expect: { + // TODO: Elminate everything after ?. + (side_effect(), null)?.unused, + (side_effect(), null)?.[side_effect()], + (side_effect(), null)?.unused.i_will_throw, + (side_effect(), null)?.call(1), + (side_effect(), null)?.(2), + (side_effect(), null)?.maybe_call?.(3); + } +} + +unused_null_conditional_chain_4: { + options = { + toplevel: true, + defaults: true, + passes: 5, + } + input: { + function nullish() { + side_effect(); + return null; + } + nullish()?.unused; + nullish()?.[side_effect()]; + nullish()?.unused.i_will_throw; + nullish()?.call(1); + nullish()?.(2); + nullish()?.maybe_call?.(3); + } + expect: { + // TODO: Elminate everything after ?. + function nullish() { + return side_effect(), null; + } + nullish()?.unused, + nullish()?.[side_effect()], + nullish()?.unused.i_will_throw, + nullish()?.call(1), + nullish()?.(2), + nullish()?.maybe_call?.(3); + } +} + +unused_null_conditional_chain_5: { + options = { + toplevel: true, + defaults: true, + passes: 5, + } + input: { + export const obj = { + side_effect() { + side_effect(); + return { null: null }; + } + } + obj.side_effect().null?.unused; + obj.side_effect().null?.[side_effect()]; + obj.side_effect().null?.unused.i_will_throw; + obj.side_effect().null?.call(1); + obj.side_effect().null?.(2); + obj.side_effect().null?.maybe_call?.(3); + } + expect: { + export const obj = { + side_effect: () => (side_effect(), {null: null}) + } + // TODO: Elminate everything after ?. + obj.side_effect().null?.unused, + obj.side_effect().null?.[side_effect()], + obj.side_effect().null?.unused.i_will_throw, + obj.side_effect().null?.call(1), + obj.side_effect().null?.(2), + obj.side_effect().null?.maybe_call?.(3); + } +} diff --git a/test/compress/evaluate.js b/test/compress/evaluate.js index cd8e4ecd9..a8b1a9535 100644 --- a/test/compress/evaluate.js +++ b/test/compress/evaluate.js @@ -1783,8 +1783,29 @@ null_conditional_chain_eval: { null?.maybe_call?.(3) } expect: { - (void 0).but_might_throw; - (void 0)(1); + } +} + +null_conditional_chain_eval_2: { + options = { + evaluate: true, + side_effects: true + } + input: { + null.deep?.unused + null.deep?.[side_effect()] + null.deep?.unused.but_might_throw + null.deep?.call(1) + null.deep?.(2) + null.deep?.maybe_call?.(3) + } + expect: { + null.deep?.unused + null.deep?.[side_effect()] + null.deep?.unused.but_might_throw + null.deep?.call(1) + null.deep?.(2) + null.deep?.maybe_call?.(3) } }