Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compress/eval of optional chains #1062

Merged
merged 6 commits into from Sep 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 9 additions & 12 deletions lib/compress/drop-side-effect-free.js
Expand Up @@ -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";

Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)
Expand Down
27 changes: 15 additions & 12 deletions lib/compress/evaluate.js
Expand Up @@ -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.
Expand All @@ -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;
});
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 1 addition & 11 deletions lib/compress/index.js
Expand Up @@ -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;
Expand Down Expand Up @@ -4199,14 +4195,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;
});

Expand Down Expand Up @@ -4273,9 +4266,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;
});

Expand Down
40 changes: 27 additions & 13 deletions lib/compress/inference.js
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
111 changes: 109 additions & 2 deletions test/compress/drop-unused.js
Expand Up @@ -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);
}
}
25 changes: 23 additions & 2 deletions test/compress/evaluate.js
Expand Up @@ -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)
}
}

Expand Down