Skip to content

Commit a7c2057

Browse files
aryanraj45aryanraj45
committed
fix(javascript): clear parameter ranges once an analysis resolves
An analysis of a copied function counts its reads from zero but met the ranges an earlier analysis left on the scopes around it, so a read in the body could skip a binding there: P(a=q+q+q+q+q) copying h printed "out" for "in". Each analysis now clears its ranges after resolving. Adds the kinds the sweep reached: computed keys, writes, typeof, rest, constructors, setters, generators, const and class.
1 parent 4e566ff commit a7c2057

2 files changed

Lines changed: 92 additions & 7 deletions

File tree

‎lib/javascript/ScopeAnalyzer.js‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1794,6 +1794,10 @@ class MinifierScopeAnalyzer extends ScopeAnalyzer {
17941794
this.safari10 = Boolean(options.safari10);
17951795
/** @type {boolean} */
17961796
this.parameterScopes = Boolean(options.parameterScopes);
1797+
// Only this analysis's reads are counted against a range, so each is
1798+
// cleared once they resolve, before an analysis of a copy counts anew.
1799+
/** @type {MinifierScope[]} the scopes holding a range of parameter reads */
1800+
this.parameterRanges = [];
17971801
/** @type {MinifierScope} */
17981802
this.toplevel = new MinifierScope("global", program, null, true);
17991803
this.scope = this.toplevel;
@@ -2160,13 +2164,15 @@ class MinifierScopeAnalyzer extends ScopeAnalyzer {
21602164
lambda.functionName = /** @type {MinifierVariable} */ (id.definition);
21612165
}
21622166
const { params } = node;
2163-
if (this.parameterScopes) {
2167+
const ranged = this.parameterScopes && params.length !== 0;
2168+
if (ranged) {
21642169
lambda.parameterStart = this.referenceNodes.length;
2170+
this.parameterRanges.push(lambda);
21652171
}
21662172
for (let i = 0; i < params.length; i++) {
21672173
this._visitBinding(params[i], DECLARES_FUNARG);
21682174
}
2169-
if (this.parameterScopes) lambda.parameterEnd = this.referenceNodes.length;
2175+
if (ranged) lambda.parameterEnd = this.referenceNodes.length;
21702176
if (this.release !== undefined) this.release(node.body);
21712177
this._visitList(node.body.body);
21722178
this.scope = outer;
@@ -2427,13 +2433,13 @@ class MinifierScopeAnalyzer extends ScopeAnalyzer {
24272433
stack.push(current);
24282434
const outer = this._enterBlock("catch", current);
24292435
const catchScope = /** @type {MinifierScope} */ (this.scope);
2430-
if (this.parameterScopes) {
2436+
const ranged = this.parameterScopes && Boolean(current.param);
2437+
if (ranged) {
24312438
catchScope.parameterStart = this.referenceNodes.length;
2439+
this.parameterRanges.push(catchScope);
24322440
}
24332441
if (current.param) this._visitBinding(current.param, DECLARES_CATCH);
2434-
if (this.parameterScopes) {
2435-
catchScope.parameterEnd = this.referenceNodes.length;
2436-
}
2442+
if (ranged) catchScope.parameterEnd = this.referenceNodes.length;
24372443
if (this.release !== undefined) this.release(current.body);
24382444
this._visitList(current.body.body);
24392445
this.scope = outer;
@@ -2611,6 +2617,19 @@ class MinifierScopeAnalyzer extends ScopeAnalyzer {
26112617
}
26122618
}
26132619

2620+
/**
2621+
* Drops the ranges of parameter reads this analysis counted.
2622+
* @returns {void}
2623+
*/
2624+
_clearParameterRanges() {
2625+
const { parameterRanges } = this;
2626+
for (let i = 0; i < parameterRanges.length; i++) {
2627+
parameterRanges[i].parameterStart = -1;
2628+
parameterRanges[i].parameterEnd = -1;
2629+
}
2630+
parameterRanges.length = 0;
2631+
}
2632+
26142633
/**
26152634
* Resolves each name read, finds `eval`, and encloses what a catch
26162635
* parameter redefines, then moves the catch parameters old engines scope
@@ -2666,6 +2685,8 @@ class MinifierScopeAnalyzer extends ScopeAnalyzer {
26662685
}
26672686
}
26682687

2688+
this._clearParameterRanges();
2689+
26692690
// Passes 3 and 4: work around old engines' catch and loop scopes.
26702691
const { catchNames, catchScopes } = this;
26712692
for (let i = 0; i < catchNames.length; i++) {
@@ -3182,6 +3203,8 @@ class CompressorScopeAnalyzer extends MinifierScopeAnalyzer {
31823203
}
31833204
}
31843205

3206+
this._clearParameterRanges();
3207+
31853208
// Passes 3 and 4: work around old engines' catch and loop scopes.
31863209
const { catchNames, catchScopes } = this;
31873210
for (let i = 0; i < catchNames.length; i++) {

‎test/unitCases/syntax-printer.unittest.js‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,56 @@ const CORRECTED_CASES = [
705705
"var x = 1; function f(a = x, b = a) { var x = 2, a; return x + a + b; } console.log(f());",
706706
{ compress: false, mangle: true }
707707
],
708+
[
709+
"a parameter pattern's computed key reading a name its body declares",
710+
"var k = 'a'; function f({ [k]: y } = { a: 'out', b: 'in' }) { var k = 'b'; return y; } console.log(f());",
711+
{ compress: false, mangle: true }
712+
],
713+
[
714+
"a parameter default assigning a name its body declares",
715+
"var x = 'out'; function f(_ = (x = 'set')) { var x = 'in'; return x; } console.log(f(), x);",
716+
{ compress: false, mangle: true }
717+
],
718+
[
719+
"a parameter default updating a name its body declares",
720+
"var n = 1; function f(_ = n++) { var n = 10; return n; } console.log(f(), n);",
721+
{ compress: false, mangle: true }
722+
],
723+
[
724+
"a parameter default testing the type of a function its body declares",
725+
"function f(_ = typeof h) { function h() {} return _; } console.log(f());",
726+
{ compress: {}, mangle: false }
727+
],
728+
[
729+
"a rest parameter's default reading a name its body declares",
730+
"var x = 'out'; var a; function f(...[_ = a = () => x]) { var x = 'in'; } f(); console.log(a());",
731+
{ compress: false, mangle: true }
732+
],
733+
[
734+
"a constructor's parameter default reading a name its body declares",
735+
"var x = 'out'; var a; class C { constructor(_ = a = () => x) { var x = 'in'; } } new C(); console.log(a());",
736+
{ compress: false, mangle: true }
737+
],
738+
[
739+
"a setter's parameter default reading a name its body declares",
740+
"var x = 'out'; var a; var o = { set p(_ = a = () => x) { var x = 'in'; } }; o.p = undefined; console.log(a());",
741+
{ compress: false, mangle: true }
742+
],
743+
[
744+
"a generator's parameter default reading a name its body declares",
745+
"var x = 'out'; var a; function* f(_ = a = () => x) { var x = 'in'; yield x; } console.log(f().next().value, a());",
746+
{ compress: false, mangle: true }
747+
],
748+
[
749+
"a parameter default reading a name its body declares with `const`",
750+
"var x = 'out'; var a; function f(_ = a = () => x) { const x = 'in'; } f(); console.log(a());",
751+
{ compress: false, mangle: true }
752+
],
753+
[
754+
"a parameter default reading a class its body declares",
755+
"var C = 'out'; var a; function f(_ = a = () => C) { class C {} } f(); console.log(a());",
756+
{ compress: false, mangle: true }
757+
],
708758
[
709759
"a parameter default's closure reading a name its body declares, compressed",
710760
"function g() { var x = 'out'; var a, b; function f(_ = a = () => x) { var x = 'in'; b = () => x; } f(); console.log(a(), b()); } g();",
@@ -4083,14 +4133,26 @@ describe("syntax-printer", () => {
40834133
});
40844134

40854135
describe("a parameter list's scope", () => {
4136+
it("should resolve a copied function's reads by no range an earlier analysis counted", async () => {
4137+
const { minify } = await load();
4138+
// The copy of `h` is analysed again, its reads counted from zero.
4139+
const input =
4140+
"function P(a = q + q + q + q + q) { var y = arguments.length > 1 ? 'x' : 'in'; function h() { return y; } return [1].map(function () { return h(); })[0]; } var q = 1, y = 'out'; console.log(P());";
4141+
const { code } = await minify(input, { compress: {}, mangle: false });
4142+
4143+
expect(runProgram(/** @type {string} */ (code))).toBe(runProgram(input));
4144+
});
4145+
40864146
it("should still read in a parameter list what the list and its function name declare", async () => {
40874147
const { minify } = await load();
40884148
for (const input of [
40894149
"var a; function f(p = 'p', q = a = () => p) { var r = 1; } f(); console.log(a());",
40904150
"var a; var f = function self(_ = a = () => typeof self) { var z; }; f(); console.log(a());",
40914151
"var a; function f(_ = a = () => arguments.length) { var y; } f(1, 2); console.log(a());",
40924152
"var a; try { throw ['e']; } catch ([e, _ = a = () => e]) { let z; } console.log(a());",
4093-
"var x = 'out'; function f(y = x) { var x = 'in'; return y; } console.log(f());"
4153+
"var x = 'out'; function f(y = x) { var x = 'in'; return y; } console.log(f());",
4154+
"function f() { var x = 'body'; function g(_ = () => x) { return _(); } return g(); } var x = 'global'; console.log(f());",
4155+
"var a; function h() { try { throw []; } catch ([_ = a = () => x]) { var x = 'fn'; } return a(); } console.log(h());"
40944156
]) {
40954157
for (const options of [
40964158
{ compress: false, mangle: true },

0 commit comments

Comments
 (0)