Multiple passes produce incorrect code

[marcusdarmstrong]

Bug report or Feature request?

Bug.

Version (complete output of terser -V)
terser 3.16.1

Complete CLI command or minify() options used
terser -m -c passes=4 --toplevel test.js

terser input

exports.withStyles = withStyles;

function _inherits(superClass) {
  if (typeof superClass !== "function") {
    throw new TypeError("Super expression must be a function, not " + typeof superClass);
  }
  Object.create(superClass); 
}

function withStyles() {
  var a = EXTERNAL();
  return function(_a){
    _inherits(_a);
    function d() {}
  }(a);
}

terser output or error
terser -m -c passes=3 --toplevel test.js

exports.withStyles=function(){!function(n){throw new TypeError("Super expression must be a function, not undefined")}(EXTERNAL())};

terser -m -c passes=1 --toplevel test.js

exports.withStyles=function(){return function(t){!function(t){if("function"!=typeof t)throw new TypeError("Super expression must be a function, not "+typeof t);Object.create(t)}(t)}(EXTERNAL())};

Expected result
The multi-pass output should not produce invalid code.

Further information

When we moved to up our passes of terser in our production builds this week, we discovered that this triggered an invalid optimization in the code that is bundled by 'react-with-styles' combined with babel's _inherits helper. (See: airbnb/react-with-styles#151)

I've reduced this down as best I can to a minimal test case, as seen above. Best I can guess this is a failure in the IIFE argument pushdown pre-emptively marking a variable as unused.

It's also worth noting that setting the keep_fnames option opts out of this bug, at an obvious optimization cost, even though there's no use of function names in this code:

terser -m -c passes=3,keep_fnames --toplevel test.js

exports.withStyles=function t(){!function t(e){if("function"!=typeof e)throw new TypeError("Super expression must be a function, not "+typeof e);Object.create(e)}(EXTERNAL())};

[L2jLiga]

What I've found:

command:
terser -m -c passes=3 --toplevel test.js

Case 1: After removing function d(){} it works:

Input:

exports.withStyles = withStyles;

function _inherits(superClass) {
    if (typeof superClass !== "function") {
        throw new TypeError("Super expression must be a function, not " + typeof superClass);
    }
    Object.create(superClass);
}

function withStyles() {
    var a = EXTERNAL();
    return function(_a){
        _inherits(_a);
    }(a);
}

Output:

exports.withStyles=function(){!function(t){if("function"!=typeof t)throw new TypeError("Super expression must be a function, not "+typeof t);Object.create(t)}(EXTERNAL())};

Case 2: After moving function d() {} outside of anonymus function it also produces correct code:

Input:

exports.withStyles = withStyles;

function _inherits(superClass) {
    if (typeof superClass !== "function") {
        throw new TypeError("Super expression must be a function, not " + typeof superClass);
    }
    Object.create(superClass);
}

function withStyles() {
    var a = EXTERNAL();
    return function(_a){
        _inherits(_a);
    }(a);
    function d() {}
}

Output:

exports.withStyles=function(){!function(t){if("function"!=typeof t)throw new TypeError("Super expression must be a function, not "+typeof t);Object.create(t)}(EXTERNAL())};

Case 3: Just removing toplevel option make Terser produces right code

Input:

exports.withStyles = withStyles;

function _inherits(superClass) {
    if (typeof superClass !== "function") {
        throw new TypeError("Super expression must be a function, not " + typeof superClass);
    }
    Object.create(superClass);
}

function withStyles() {
    var a = EXTERNAL();
    return function(_a){
        _inherits(_a);
        function d() {}
    }(a);
}

Output:

function _inherits(t){if("function"!=typeof t)throw new TypeError("Super expression must be a function, not "+typeof t);Object.create(t)}function withStyles(){_inherits(EXTERNAL())}exports.withStyles=withStyles;

[roblan]

I'm not sure, but it looks a little like #294

[L2jLiga]

Workaround is to add evaluate=false to compress options, also it's workaround for #294

I'll try to fix it