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

improve handling of self-referential classes #1504

Merged
merged 1 commit into from
Mar 14, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions lib/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -2271,6 +2271,24 @@ var AST_Class = DEFNODE("Class", "name extends properties", function AST_Class(p
}
});
},
is_self_referential: function() {
const this_id = this.name && this.name.definition().id;
let found = false;
let class_this = true;
this.visit_nondeferred_class_parts(new TreeWalker((node, descend) => {
if (found) return true;
if (node instanceof AST_This) return (found = class_this);
if (node instanceof AST_SymbolRef) return (found = node.definition().id === this_id);
if (node instanceof AST_Lambda && !(node instanceof AST_Arrow)) {
const class_this_save = class_this;
class_this = false;
descend();
class_this = class_this_save;
return true;
}
}));
return found;
},
}, AST_Scope /* TODO a class might have a scope but it's not a scope */);

var AST_ClassProperty = DEFNODE("ClassProperty", "static quote", function AST_ClassProperty(props) {
Expand Down
14 changes: 7 additions & 7 deletions lib/compress/drop-side-effect-free.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,13 @@ def_drop_side_effect_free(AST_Arrow, return_null);

def_drop_side_effect_free(AST_Class, function (compressor) {
const with_effects = [];

if (this.is_self_referential() && this.has_side_effects(compressor)) {
return this;
}

const trimmed_extends = this.extends && this.extends.drop_side_effect_free(compressor);
if (trimmed_extends)
with_effects.push(trimmed_extends);
if (trimmed_extends) with_effects.push(trimmed_extends);

for (const prop of this.properties) {
if (prop instanceof AST_ClassStaticBlock) {
Expand All @@ -166,11 +170,7 @@ def_drop_side_effect_free(AST_Class, function (compressor) {
}
} else {
const trimmed_prop = prop.drop_side_effect_free(compressor);
if (trimmed_prop) {
if (trimmed_prop.contains_this()) return this;

with_effects.push(trimmed_prop);
}
if (trimmed_prop) with_effects.push(trimmed_prop);
}
}

Expand Down
21 changes: 8 additions & 13 deletions lib/compress/drop-unused.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
AST_SymbolFunarg,
AST_SymbolRef,
AST_SymbolVar,
AST_This,
AST_Toplevel,
AST_Unary,
AST_Var,
Expand Down Expand Up @@ -127,7 +126,6 @@ AST_Scope.DEFMETHOD("drop_unused", function(compressor) {
return node.expression;
}
};
var this_def = null;
var in_use_ids = new Map();
var fixed_ids = new Map();
if (self instanceof AST_Toplevel && compressor.top_retain) {
Expand All @@ -139,6 +137,7 @@ AST_Scope.DEFMETHOD("drop_unused", function(compressor) {
}
var var_defs_by_id = new Map();
var initializations = new Map();
var self_referential_classes = new Set();

// pass 1: find out which symbols are directly used in
// this scope (not in nested scopes).
Expand All @@ -152,6 +151,10 @@ AST_Scope.DEFMETHOD("drop_unused", function(compressor) {
});
}
if (node === self) return;
if (node instanceof AST_Class && node.has_side_effects(compressor)) {
if (node.is_self_referential()) self_referential_classes.add(node);
node.visit_nondeferred_class_parts(tw);
}
if (node instanceof AST_Defun || node instanceof AST_DefClass) {
var node_def = node.name.definition();
const in_export = tw.parent() instanceof AST_Export;
Expand All @@ -161,22 +164,11 @@ AST_Scope.DEFMETHOD("drop_unused", function(compressor) {
}
}

if (node instanceof AST_DefClass && node.has_side_effects(compressor)) {
const save_this_def = this_def;
this_def = node_def;
node.visit_nondeferred_class_parts(tw);
this_def = save_this_def;
}

map_add(initializations, node_def.id, node);
return true; // don't go in nested scopes
}
// In the root scope, we drop things. In inner scopes, we just check for uses.
const in_root_scope = scope === self;
if (node instanceof AST_This && this_def && in_root_scope) {
in_use_ids.set(this_def.id, this_def);
return true;
}
if (node instanceof AST_SymbolFunarg && in_root_scope) {
map_add(var_defs_by_id, node.definition().id, node);
}
Expand Down Expand Up @@ -225,6 +217,9 @@ AST_Scope.DEFMETHOD("drop_unused", function(compressor) {
init.walk(tw);
});
});
self_referential_classes.forEach(function (cls) {
cls.walk(tw);
});
// pass 3: we should drop declarations not in_use
var tt = new TreeTransformer(
function before(node, descend, in_list) {
Expand Down
Loading
Loading