Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If statement optimization causes assignments to be executed out of order #3271

Closed
simonlampen opened this issue Oct 7, 2018 · 5 comments · Fixed by #3329
Closed

If statement optimization causes assignments to be executed out of order #3271

simonlampen opened this issue Oct 7, 2018 · 5 comments · Fixed by #3329

Comments

@simonlampen
Copy link

Bug report or feature request?

Bug

bisected version
Bisected and GOOD at 3.4.8 and BROKEN at 3.4.9

JavaScript input
input.js:

function string2buf(str) {  
    var i=0,
        buf = new Array(2),
        c = str.charCodeAt(0);
    if (c < 0x800) {
        /* two byte char */
        buf[i++] = 0xC0 | (c >>> 6);
        buf[i++] = 0x80 | (c & 0x3f);
    } else {
        /* three byte char */
        buf[i++] = 0xE0 | (c >>> 12);
        buf[i++] = 0x80 | (c >>> 6 & 0x3f);
        buf[i++] = 0x80 | (c & 0x3f);
    }
    return buf;
};
console.log(string2buf("é"));

Command line:
cls&&uglifyjs --mangle --compress -- input.js > output.js&&echo expected:&&node input.js&&echo actual:&&node output.js

expectation and failure
expected:
[ 195, 169 ]
actual:
[ 169, 195 ]

The if statements are optimised into conditional clauses but the assignments within the if clauses are caused to run out of order. In this example it causes elements to be added to an array out of order.

@fabiosantoscode
Copy link
Contributor

Bug does not occur in terser

@nylen
Copy link

nylen commented Nov 9, 2018

Releasing a new version and then disappearing is bad form. Please follow up with your community after the release.

If you don't have time to do that, better not to do the release in the first place.

@nylen
Copy link

nylen commented Nov 9, 2018

@fabiosantoscode do you have a test case for this issue? Might help you improve coverage a bit.

@fabiosantoscode
Copy link
Contributor

Nice suggestion @nylen, I've added a test now.

@nylen
Copy link

nylen commented Nov 13, 2018

Thanks - this is not the first time I've seen incredibly poor software development and community management practices on this repository, so I'm going to look into switching current and future projects to terser.

Please keep improving the test coverage, that is really important for critical web infrastructure like a JS minifier, and really nice to see!

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Mar 12, 2019
alexlamsl added a commit that referenced this issue Mar 12, 2019
fixes #3245
fixes #3257
fixes #3260
fixes #3269
fixes #3271
fixes #3278
fixes #3309
fixes #3319
fixes #3321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants