track returned state in function body

[vankop]

What kind of change does this PR introduce?

feature

closes #14347

Did you add tests for your changes?

yes

Does this PR introduce a breaking change?

no

What needs to be documented once your changes are merged?

nothing

Notes regarding feature design

Added 2 properties to parser current scope (parser.scope)

• terminated marks that further parsing of nearest BlockStatement should be terminated. Has 3 states 'return'|'throw'|undefined, where undefined = false, 'return'|'throw' types of termination
  returned is inherited from child BlockStatement (or terminate node in case of if (true) return;) to nearest parent BlockStatement if executedPath=true
• executedPath marks that current statement will be executed if nearest BlockStatement will be reached, e.g.

function a() {  // <- executedPath=true
  return; // <- terminated=return
  // should terminate parsing of current BlockStatement
}

function a() {  // <- executedPath=true
  if (rand()) { // <- executedPath=false marks 'unknown' state
     return 1; // terminated=return
  }  // <- terminated=undefined we do not know will this be executed or not (child block executedPath=false)
  
  if (true) {  // <- executedPath=true
       throw 2;  // <- terminated=throw
  }
   // should terminate parsing of current BlockStatement
   require('c')
}

function a() {  // <- executedPath=true
  for (;;) {  // <- executedPath=false for all loops
     require('a') // should be required
     if (true) {  // <- executedPath=true
         return;  // <- terminated=return
     }
     // should terminate parsing of current BlockStatement
     require('c')
  } // <- terminated=undefined
  
  try { // <- executedPath=true same executedPath for each inner block
     throw 2; // <- terminated=throw
     // should not be parsed
     require()
  } catch(e) {  // executedPath=false for every catch
   
  } // terminated=undefined throw in try
  // continue parsing..
}

complex example

function a() {  // <- executedPath=true
  if (rand()) {  // <- executedPath=false
    if (true) {  // <- executedPath=true
        return;  // <- terminated=return
    }
    // should terminate parsing of current BlockStatement
    require('c')
  }  // <- executedPath=true, terminated=undefined
  
  if (true) {  // <- executedPath=true
       throw 2;  // <- terminated=throw
  }  // <- executedPath=true, terminated=throw

   // should terminate parsing of current BlockStatement
   require('c')
}

and one more 🔞

function a(name) { // executedPath=true
switch (name) { // executedPath=false for each switch
  case "a": { // executedPath=false from parent
     if (true) return; // executedPath=true, terminated=return
     // skip parsing current block
     require('a')
     return 1;
  } // terminated=undefined since `case: a` block is executedPath=false
  case "b":
     if (true) return; // executedPath=true, terminated=return
     // skip parsing current switch case, but not current block ⚠️
     // we should not mark current block as returned=true
     require('a');
     return 2;
  case "c":
    // should be parsed
     return require('c');
  case "d":
    // // should be parsed
     return require('c');
  default:
     throw new Error("Unexcepted test data");
}
}

Notes regarding feature implementation

• right now parser.hooks.terminate looks kind of useless, not sure about it..

TODO
drop skipped code. Any ideas how to do it better? we should do this from terminated!=undefined blocks till terminated=undefined blocks

[webpack-bot]

For maintainers only:

• This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
• This needs to be backported to webpack 4 (issue will be created when merged)

[webpack-bot]

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

[alexander-akait]

@vankop Can you rebase? Thank you

[TheLarkInn]

• Add test case for hoisted function after return
• Add test case for dynamic import after return
• Change test case to use require("fail") instead of importing in existing module (so it actually fails compiling when parsing that part)

[alexander-akait]

I like this solution, I'm only afraid of one thing, that some plugins could rely on it and rewrite something, and now we just will not handle them at all

[vankop]

but this code is unreachable.. so I dont see a reason to waste cpu =)

[alexander-akait]

Yeah, I agree with you, I'm just worried that someone could use similar logic, but we can merge and look at feedback, It's not difficult to do a revert