Uglifyjs: ++ + can be +++

[S-YOU]

Not sure this is regression or intentional, but recently uglify output following:

var i=1,j=2,k=i+++j;
to
var i=1,j=2,k=i++ +j;

but i++ +j and i+++j is the same

[kzc]

Good question.

Google Closure Compiler and Babili also convert k=i+++j; to k=i++ +j;

Makes me suspect some popular browser has an issue with parsing precedence or incorrectly flags i+++j as an error.

[kzc]

Here's where Uglify emits the space:

https://github.com/mishoo/UglifyJS2/blob/8d74f3437352e22b3fd18ce602a4378170ec6598/lib/output.js#L260

It's just a consequence of the logic to prevent other operators from incorrectly running together.

I suppose the cases i+++j and i---j could be special cased here:

https://github.com/mishoo/UglifyJS2/blob/8d74f3437352e22b3fd18ce602a4378170ec6598/lib/output.js#L1152

with an override for print() to not emit the space in those two cases.

Probably the extra complexity is not worth the hassle for this relatively infrequent situation.

Edit: such code might also slow down output in the general case as it's a hot code path. Would have to perform timings if someone implemented this optimization to ensure it doesn't degrade performance significantly.

[S-YOU]

Thanks for the bringing up the details and explanations. That make sense.
Should I leave this open or closed as Won't fix?

[kzc]

It can be left open. If someone puts together a pull request with appropriate tests and verifies that performance isn't significantly impacted processing 1MB+ JS files (< 0.5% slowdown) it would be considered.

[avdg]

So the mechanics involved in this issue is that a tokenizer must be greedy, so it as much possible symbols to form a token. ++ is longer than +, so when there is doubt, the interpreter must pick the ++ over +. So for the opposite case, a + followed by ++. It can't be shortened, so a space must be inserted in that case.

And as @kzc said, as long as (95%+ of) the other interpreters are doing this. It should be all fine.

[avdg]

Checking is not really that complex in this case. But instead of looking at the last character, it must check for the last 2 characters for the ++, +, -- and - operators to make it possible to distinguish them.

[avdg]

Won't write a patch yet, because I already have a big pr queue for the moment.

[avdg]

Although this is a patch aimed for master, where I don't have too many patches for actually... I might change my mind...

[kzc]

Just to put this in perspective on how rare this is in the wild, I just ran uglify on 11MB of popular JS libraries and apps and only found 3 occurrences of this. Only 3 bytes could potentially be saved in 11MB of common javascript.

[mishoo]

@kzc it's true that it doesn't save much.

If I remember correctly, the space is added intentionally. I can't recall what problems were without it though, but feel free to dig the git history :-) I vote for “won't fix”, let's not create potential problems for improbable savings.

[avdg]

Meh... I digged in anyway...

rewriting https://github.com/mishoo/UglifyJS2/blob/8d74f3437352e22b3fd18ce602a4378170ec6598/lib/output.js#L260 to
/([^\+\-]|^)[\+\-]$/.test(last) would do the job, but meh only 3 bytes for an untested approuch.

That said, the regexp won't work, it will match wrong things... (although var last won't contain that content ever normally, but I prefer double safe)

Edit: yeah, I won't implement it for now... I'm not comfortable in the regexp to implement it unless the edge cases are investigated too... (like +++ +-, etc...)