824112 - jsmin.py horribly unsound in so many ways

Status: RESOLVED DUPLICATE of bug 814795

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

For inline multiple-var declarations like

    var foo = v1, bar = v2

jsmin.py, which we run over non-debug selfhosted code, generates wrong code where foo and bar are aliased.

Sometimes, the time spent debugging is not well spent, and is lost forever, like tears in rain.

Comment 1

[Shu-yu Guo [:shu]]

I'm sure this minifier is going to bite other people too; this is just one of the many ways it is unsound.

So let's discuss. Should we remove it? Should we replace it with a better minifier?

Comment 2

[Shu-yu Guo [:shu]]

And additional clarification on how the minifier introduces incorrect aliasing. It doesn't understand compound declarations where the variables are assigned to at all. So, if you have something like:

   var i = 0, j = 1;

Only |i| gets renamed, with |j| untouched. What if the minifier already renamed something else to |j|, you ask? Well too bad!

Comment 3

[Norbert Lindenberg]

As the comment in jsmin.py itself says: "there are many valid JavaScript programs that will be ruined by it".

The YUICompressor home page, besides providing an industrial-strength minifier itself, also links to some other minifiers:
http://yui.github.com/yuicompressor/

Comment 4

[Shu-yu Guo [:shu]]

I don't think requiring Java in the build process is going to fly, though.

Comment 5

[Till Schneidereit [:till]]

Just a quick comment from sunny South Africa:

I think we can use pretty much any minifier we want, now that we don't have any special syntax, anymore.

The other reason for the V8 team's usage of their own minifier is that they want to keep the line numbers correct. Since we don't minify for debug builds, I don't think that we need that property, either. (Even if/when we get around to making self-hosted code visible in the debugger in some sensible way.)

There are quite a few Python-based JS minifiers which should neatly fit into our build process.

Comment 6

[Shu-yu Guo [:shu]]

My preference is to not minify at all.

Comment 7

[Niko Matsakis [:nmatsakis]]

Could we measure the difference in startup time due to minification?  I'd expect it to be minimal, but then you never know.

Comment 8

[Till Schneidereit [:till]]

(In reply to Shu-yu Guo [:shu] from comment #6)
> My preference is to not minify at all.

Are the downloadable packages compressed for all platforms? If not, this might add to the download size considerably if we self-host substantial parts of the builtins library.

Comment 9

[Shu-yu Guo [:shu]]

(In reply to Till Schneidereit [:till] from comment #8)
> (In reply to Shu-yu Guo [:shu] from comment #6)
> > My preference is to not minify at all.
> 
> Are the downloadable packages compressed for all platforms? If not, this
> might add to the download size considerably if we self-host substantial
> parts of the builtins library.

We already have source compression capabilities for saving function sources. Maybe we can just use that?

Comment 10

[Till Schneidereit [:till]]

> We already have source compression capabilities for saving function sources.
> Maybe we can just use that?

That's a good point. source compression just uses zlib, so integration into the build script should be straight-forward.

Comment 11

[Norbert Lindenberg]

Source compression doesn't remove comments though, and seeing how the code review for 769872 is going, there will be lots of comments.

Comment 12

[Till Schneidereit [:till]]

As just discussed with shu, we should probably just go with compression.

We're going to use fairly advanced features in our self-hosted code (at any single point in time, i guess), so using even a now-solid minifier poses a high risk of breaking the build with the introduction of entirely valid JS code.

(In reply to Norbert Lindenberg from comment #11)
> Source compression doesn't remove comments though, and seeing how the code
> review for 769872 is going, there will be lots of comments.

True. Comments removal is surprisingly tricky, however. Strings and Regular Expressions make it non-trivial already, and Template Literals[1][2][3] are going to make it even harder.

[1]: spec: http://people.mozilla.org/~jorendorff/es6-draft.html#sec-11.1.9
[2]: old proposal: http://wiki.ecmascript.org/doku.php?id=harmony:quasis
[3]: tracked in bug 688857