output optimal representations of NaN & Infinity #1723
Conversation
- move these optimisations out from `Compressor` to `OutputStream` - fixes behaviour inconsistency when running uglified code from global or module levels due to redefinition
|
@qfox @kzc this should nail the problem we had with $ cat test.js
var NaN = 2;
console.log(NaN, Infinity * 0);
$ bin/uglifyjs test.js -c -o min.js
$ cat min.js
var NaN=2;console.log(NaN,0/0);
$ node test.js
2 NaN
$ node min.js
2 NaN
$ cat test.js | node
NaN NaN
$ cat min.js | node
NaN NaN |
|
Cool. Shouldn't |
|
@qfox |
|
Should I add builtin global function names like |
|
@qfox no harm in trying to fuzz those as well 👍 |
|
I'm not so sure this change should be enabled in compress by default. The rationale for outputting In jsperf testing https://jsperf.com/test-nan-vs-0-0 Likewise, https://jsperf.com/infinity-vs-1-0-v2-0 At least the Edit: |
|
Shouldn't that be |
|
While I think the performance implication is important, in a sense it is a correctness issue as before this PR the uglified code could misbehave in certain scenario, e.g. when wrapped inside an IIFE: test.js!function() {
var NaN = 2;
console.log(NaN, Infinity * 0);
}();$ cat test.js | node
2 NaN
$ bin/uglifyjs -V
uglify-js 2.8.18
$ bin/uglifyjs test.js -c collapse_vars=0,reduce_vars=0 | node
2 2
$ git checkout master
$ bin/uglifyjs test.js -c collapse_vars=0,reduce_vars=0 | node
2 NaN |
The test was indeed wrong, but accurately described. Here's the correct test: https://jsperf.com/infinity-vs-1-0-v2-0
|
That's just a plain old bug. A different issue altogether. |
I'm confused - do you mean we shouldn't fix this, or have you got a fix that does not suffer the performance issue? |
|
The parens bug was introduced when changing I think It is my view that this transform should be not enabled by default and if you must add it, it should be behind a new flag. |
Pretty sure #1723 (comment) would fail on Chrome as well? |
|
$ bin/uglifyjs test.js -c collapse_vars=0,reduce_vars=0
!function(){var NaN=2;console.log(NaN,NaN)}(); |
That's a plain old bug. A separate issue. |
I must be pretty thick right now - what do you suggest as the correct course of action? What I meant to say is, given the choice of:
I'd prefer the first one. Obviously if there is a better fix for the same bug that does not incur performance penalty on Chrome, I'm all for it. I just can't think of one right now. 😓 Shall I open an issue with this performance penalty thing so we can keep track and fix it properly? |
|
@alexlamsl Obviously we all want the bug fixed on all engines. Thank you for fixing it. I am just arguing that |
|
Spec compliance aside, programs that redefine or override |
|
When you think about it, the very act of transforming code in intricate and convoluted ways just to have a smaller version of itself that behaves the same at runtime has got to be on the top ten list of inane pursuits. |
|
If this is your attempt to sum up my life, I think you're getting pretty close... 😈 |
It's my bread and butter and I love it :D They're just giant puzzles, really. |
That's it exactly. Download bandwidth is a secondary concern. |
Those people are missing out. It's like js1k; looking at a demo in no way describes the enjoyment of creating one. |
- migrate transformation logic from `OutputStream` to `Compressor` - always turn `undefined` into `void 0` (unless `unsafe`) - always keep `NaN` except when avoiding local variable redefinition - introduce `keep_infinity` to suppress `1/0` transform, except when avoiding local variable redefinition supersedes mishoo#1723 fixes mishoo#1730
- migrate transformation logic from `OutputStream` to `Compressor` - always turn `undefined` into `void 0` (unless `unsafe`) - always keep `NaN` except when avoiding local variable redefinition - introduce `keep_infinity` to suppress `1/0` transform, except when avoiding local variable redefinition supersedes mishoo#1723 fixes mishoo#1730
- migrate transformation logic from `OutputStream` to `Compressor` - always turn `undefined` into `void 0` (unless `unsafe`) - always keep `NaN` except when avoiding local variable redefinition - introduce `keep_infinity` to suppress `1/0` transform, except when avoiding local variable redefinition supersedes mishoo#1723 fixes mishoo#1730
- migrate transformation logic from `OutputStream` to `Compressor` - always turn `undefined` into `void 0` (unless `unsafe`) - always keep `NaN` except when avoiding local variable redefinition - introduce `keep_infinity` to suppress `1/0` transform, except when avoiding local variable redefinition supersedes #1723 fixes #1730
Clarify that "keep_infinity" compress option is only useful for older versions of Chrome. The initial performance assessment that concluded `Infinity` was faster than `1/0` was based on Chrome 57. mishoo#1723 (comment) Chrome 63 suffers no such degradation. https://jsperf.com/infinity-vs-1-0-v2-0
CompressortoOutputStreamFrom #1697 (comment)