Skip to content

Commit

Permalink
stop create_symbol symbol generator from creating symbols which confl…
Browse files Browse the repository at this point in the history
…ict in inner scopes. Closes #851
  • Loading branch information
fabiosantoscode committed Oct 18, 2020
1 parent 8a42649 commit 3357170
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 2 deletions.
7 changes: 6 additions & 1 deletion lib/compress/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4270,9 +4270,14 @@ AST_Scope.DEFMETHOD("hoist_properties", function(compressor) {
const defs = new Map();
const assignments = [];
value.properties.forEach(({ key, value }) => {
const scope = find_scope(hoister);
const symbol = self.create_symbol(sym.CTOR, {
source: sym,
scope: find_scope(hoister),
scope,
conflict_scopes: new Set([
scope,
...sym.definition().references.map(ref => ref.scope)
]),
tentative_name: sym.name + "_" + key
});

Expand Down
28 changes: 27 additions & 1 deletion lib/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,13 @@ AST_Scope.DEFMETHOD("conflicting_def", function (name) {
);
});

AST_Scope.DEFMETHOD("conflicting_def_shallow", function (name) {
return (
this.enclosed.find(def => def.name === name)
|| this.variables.has(name)
);
});

AST_Scope.DEFMETHOD("add_child_scope", function (scope) {
// `scope` is going to be moved into `this` right now.
// Update the required scopes' information
Expand Down Expand Up @@ -526,23 +533,42 @@ AST_Scope.DEFMETHOD("add_child_scope", function (scope) {
}
});

function find_scopes_visible_from(scopes) {
const found_scopes = new Set();

for (const scope of new Set(scopes)) {
(function bubble_up(scope) {
if (scope == null || found_scopes.has(scope)) return;

found_scopes.add(scope);

bubble_up(scope.parent_scope);
})(scope);
}

return [...found_scopes];
}

// Creates a symbol during compression
AST_Scope.DEFMETHOD("create_symbol", function(SymClass, {
source,
tentative_name,
scope,
conflict_scopes = [scope],
init = null
} = {}) {
let symbol_name;

conflict_scopes = find_scopes_visible_from(conflict_scopes);

if (tentative_name) {
// Implement hygiene (no new names are conflicting with existing names)
tentative_name =
symbol_name =
tentative_name.replace(/(?:^[^a-z_$]|[^a-z0-9_$])/ig, "_");

let i = 0;
while (this.conflicting_def(symbol_name)) {
while (conflict_scopes.find(s => s.conflicting_def_shallow(symbol_name))) {
symbol_name = tentative_name + "$" + i++;
}
}
Expand Down
27 changes: 27 additions & 0 deletions test/compress/hoist_props.js
Original file line number Diff line number Diff line change
Expand Up @@ -1098,3 +1098,30 @@ does_not_hoist_objects_with_computed_props: {
}
expect_stdout: "PASS"
}

issue_851_hoist_to_conflicting_name: {
options = {
hoist_props: true,
reduce_vars: true,
toplevel: true
}
input: {
const BBB = { CCC: "PASS" }

if (id(true)) {
const BBB_CCC = BBB.CCC
console.log(BBB_CCC)
}
}

expect: {
const BBB_CCC$0 = "PASS";

if (id(true)) {
const BBB_CCC = BBB_CCC$0;
console.log(BBB_CCC);
}
}

expect_stdout: "PASS"
}

0 comments on commit 3357170

Please sign in to comment.