[ES6] remove AST_ArrowParametersOrSeq

[alexlamsl]

#968 (comment)

[kzc]

Can you copy the implementation of the expression function from master to aid future merges? They are functionally identical.

https://github.com/mishoo/UglifyJS2/blob/7d3b941e6e12883e9631655d69ae99b1f1dde3db/lib/parse.js#L1560-L1574

Patch to apply to this PR:

--- lib/parse.js-1997	2017-05-23 21:46:39.000000000 -0400
+++ lib/parse.js	2017-05-23 21:46:55.000000000 -0400
@@ -2751,8 +2751,7 @@
             next();
             commas = true;
         }
-        if (exprs.length == 1) return exprs[0];
-        return new AST_Sequence({
+        return exprs.length == 1 ? exprs[0] : new AST_Sequence({
             start       : start,
             expressions : exprs,
             end         : peek()

Or, if you prefer, make master match harmony's implementation of this function.

[alexlamsl]

Thanks for catching this - version on master looks good.

[kzc]

The good thing about having extensive unit tests is that you can completely change the implementation with reasonable confidence.

[alexlamsl]

... only to get tripped and fall flat on the face by that false sense of security 😏

[alexlamsl]

Memory footprint along these updated paths for ES5 should now almost identical to master.

[kzc]

There's around 13K LOC in the in uglify library/CLI compared to 29K LOC of tests (in harmony). Is the coverage perfect? No, but it's not too shabby either. At least it prevents people from making obvious mistakes when (re)implementing or fixing something.

[kzc]

We still have to find and address that 25% performance deficit that harmony has compared to master.

[alexlamsl]

It certainly caught the (-7) ** 5 issue when I update the end point to (what I believe) the correct position. Intuition doesn't usually help with it comes to bug compatibility 😅

[alexlamsl]

We still have to find and address that 25% performance deficit that harmony has compared to master.

My guy feeling is we are still at the "make it work" stage for the harmony branch.

[kzc]

harmony is closer to producing correct code than you may think. With both mangle and compress disabled it's pretty solid. mangle should now be reasonably safe with compress disabled. Most of the remaining compress issues are disabling certain ES5 optimizations in block scopes for let/const/function.

[alexlamsl]

So you believe the 25% performance loss is (mostly) due to the parse() phase?

I guess one way to narrow this down is by test/benchmark.js then pick out the worst offender, verify the loss with just bin/uglify, i.e. without mangle or compress, then proceed to narrow the gap in harmony for parsing of ES5 code.

I believe this is a reasonable strategy because ES6 code should mostly be ES5 with a sprinkle of magic dust.

[kzc]

So you believe the 25% performance loss is (mostly) due to the parse() phase?

Perhaps there's some minor performance loss in parse, but I think it's mainly due to the compress phase. But I haven't broken down the timings.

[kzc]

Once #1998 is resolved with #2000 it will be easier to determine why harmony is 25% slower than master on average.