fix corner case in unused

[alexlamsl]

fixes #4912

[kzc]

What was the relevance of hoist_vars and the unused var anything = 1; in the original reduced test case?

[alexlamsl]

Technically your test case is not the same bug − it only fails in master not v3.13.5

That var anything = 1 is there to thwart collapse_vars from folding var init = console.log.init = function() {} into init.prototype by transforming it with hoist_vars

[kzc]

Technically your test case is not the same bug − it only fails in master not v3.13.5

Son of a gun - you're right. I thought I originally had a test case that failed on both v3.13.5 and master - but I must have changed it.

When I diffed the jquery-2.1.3.js output between v3.13.4 and v3.13.5 I just saw that alias of the prototype property being dropped. In the most recent version of jquery I also saw a different alias dropped but that case appeared to be correct.

I attempted to make a test/release script for jquery but it failed to exhibit the bug. It also relied on headless browsers that do not work on my machine, so I abandoned the effort.

[alexlamsl]

Son of a gun - you're right.

No worries − that's most likely due to #4890 (comment), i.e. I've extended a bug alongside the functionality 😓

When I diffed the jquery-2.1.3.js output between v3.13.4 and v3.13.5 I just saw that alias of the prototype property being dropped.

After making the same observation, I've stubbed it out:

this.addEventListener = function() {
	return this;
};
this.document = {
	addEventListener: function() {
		return this;
	},
	appendChild: function() {
		return this;
	},
	cloneNode: function() {
		return this;
	},
	createDocumentFragment: function() {
		return this;
	},
	createElement: function() {
		return this;
	},
	documentElement: function() {
		return this;
	},
	nodeType: 9,
	setAttribute: function() {
		return this;
	},
};
this.document.lastChild = this.document;
this.window = this;
this.location = { href: "foo" };
this.module = { exports: {} };

<!-- rest of jquery-2.1.3.js -->

console.log(module.exports.fn.init.prototype === module.exports.fn);

Such that:

$ cat test.js | node
true

$ uglify-js -v
uglify-js 3.13.4

$ uglify-js test.js -c hoist_vars | node
true

$ uglify-js -v
uglify-js 3.13.5

$ uglify-js test.js -c hoist_vars | node
false

And then let --reduce-test rip while I go enjoy dinner 😋

[kzc]

The browser stub is quite clever.

I tried to run your test case with -c hoist_vars --reduce-test and it produced an 1109 line test case after 40 minutes. And that code when run produced ReferenceError: window is not defined which is strange because the test case ended with:

// output: true
// 
// minify: false
// 
// options: {
//   "compress": {
//     "hoist_vars": true
//   },
//   "mangle": false
// }

Did you experience something different?

[alexlamsl]

Same experience − remember what we did before I refused to add globalThis to sandbox.run()? 😏

[kzc]

Ah, okay... it runs correctly in the sandbox:

$ node
Welcome to Node.js v14.16.0.
Type ".help" for more information.
> var sandbox = require("./test/sandbox.js");
undefined
> var code = require("fs").readFileSync("reduced.js").toString();
undefined
> sandbox.run_code(code)
'true\n'
> sandbox.run_code(require(".").minify(code, {compress:{hoist_vars:1}}).code)
'false\n'

remember what we did before I refused to add globalThis to sandbox.run()?

Replacing window with global works if you run it in this manner:

$ cat reduced.js | node
true

$ cat reduced.js | uglify-js -c hoist_vars | node
false

But not this way:

$ node reduced.js
reduced.js:115
    var gq = o.createDocumentFragment(), hq = o.createElement(), iq = o.createElement();
               ^
TypeError: Cannot read property 'createDocumentFragment' of undefined

How odd.

[kzc]

I had to feed the reduced test case through test/reduce again to get a small result in a few more minutes. Is there a CLI option to up the max iterations?

$ /usr/bin/time bin/uglifyjs reduced.js -c hoist_vars --reduce-test 
// Node.js v14.16.0 on darwin x64
// reduce test pass 1, iteration 0: 37354 bytes
// reduce test pass 1, iteration 25: 17188 bytes
// reduce test pass 1, iteration 50: 17114 bytes
// reduce test pass 1, iteration 75: 17114 bytes
// reduce test pass 1, iteration 100: 17114 bytes
// reduce test pass 1, iteration 125: 17114 bytes
// reduce test pass 1, iteration 150: 17114 bytes
// reduce test pass 1, iteration 175: 17114 bytes
// reduce test pass 1, iteration 200: 17104 bytes
// reduce test pass 1, iteration 225: 17104 bytes
// reduce test pass 1, iteration 250: 17104 bytes
// reduce test pass 1, iteration 275: 17104 bytes
// reduce test pass 1, iteration 300: 17051 bytes
// reduce test pass 1, iteration 325: 17051 bytes
// reduce test pass 1, iteration 350: 17043 bytes
// reduce test pass 1, iteration 375: 17043 bytes
// reduce test pass 1, iteration 400: 17043 bytes
// reduce test pass 1, iteration 425: 16883 bytes
// reduce test pass 1, iteration 450: 16694 bytes
// reduce test pass 1, iteration 475: 16617 bytes
// reduce test pass 1, iteration 500: 16615 bytes
// reduce test pass 1, iteration 525: 16117 bytes
// reduce test pass 1, iteration 550: 14779 bytes
// reduce test pass 1, iteration 575: 14255 bytes
// reduce test pass 1, iteration 600: 9328 bytes
// reduce test pass 1, iteration 625: 7811 bytes
// reduce test pass 1, iteration 650: 5101 bytes
// reduce test pass 1, iteration 675: 959 bytes
// reduce test pass 1: 939 bytes
// reduce test pass 2: 350 bytes
// reduce test pass 3: 245 bytes
// (beautified)

this.module = {};

(function(a, b) {
    module.exports = 1 ? b() : function() {
        return b;
    };
})(global, function() {
    var q = function() {};
    q.fn = {};
    q.extend = function() {
        ic = 0;
    };
    var D = q.fn.init = function() {};
    D.prototype = 0;
    return q;
});

console.log(module.exports.fn.init.prototype);
// output: 0
// 
// minify: {}
// 
// options: {
//   "compress": {
//     "hoist_vars": true
//   },
//   "mangle": false
// }
      159.22 real       168.37 user        12.26 sys

So 43 minutes in total to reduce a 9240 line test case. It could be faster, but not bad.

Anyway, I'm still astonished that node removal and replacing stuff with 0s and 1s works at all.

[alexlamsl]

But not this way:

When you do node test.js it will be magically wrapped by an effective function(module, exports) { ... }, i.e. somewhat equivalent to toplevel=true

Is there a CLI option to up the max iterations?

Nope − but I don't really see the point for doing so. Cascading command line is hardly a lot of effort, and I often do that for ufuzz test cases when passes got flagged.

Anyway, I'm still astonished that node removal and replacing stuff with 0s and 1s works at all.

It takes care of the most basic but otherwise tedious details. I intervened between the first and second pass by getting rid of the module detection IIFE, i.e. making it console.log((function( window... )().init.prototype) since it's now trivial to make that transform without losing the culprit.