Fixed problems with hoisting

[andreabergia]

This PR fixes some problems with function hoisting in nested scopes. Consider this:

function outer() {
  inner();

  function inner() {}
  do {
    inBlock();
    function inBlock() {} 
  } while (false);

  var expr = function() {}
}

The hoisting rules say that this is sorta equivalent to:

function outer() {
  // Hoisting declaraitons
  var inner;
  var inBlock;
  var expr;
  inner = function inner() {}

  inner();

  do {
    // Hoisting
    inBlock = function inBlock() {} 

    inBlock();
  } while (false);

  expr = function() {}
}

Rhino correctly handles hoisting at the top level of the function (i.e. for inner), but it does not handle it correctly for nested blocks (i.e. for inBlock). So in the example above, the following exception gets thrown:

TypeError: inBlock is not a function, it is undefined

This PR fixes the problem. The trick is that, whenever the IR for a block is generated, nested functions are hoisted on top of it.

[p-bakker]

Based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function#block-level_function_declaration, I don't think any assumptions are to be made as to whether functions declarations within blocks are hoisted or not, at least in non-strict mode.

When in Strict mode however, the function should be hoisted, but only to the top of it's containing block, so not the outer function

Also see related PR/discussion #970

[andreabergia]

Based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/function#block-level_function_declaration, I don't think any assumptions are to be made as to whether functions declarations within blocks are hoisted or not, at least in non-strict mode.

When in Strict mode however, the function should be hoisted, but only to the top of it's containing block, so not the outer function

Also see related PR/discussion #970

We discovered this because of a problem reported by one of our customers. What we implemented is coherent with the other engines that we have tested (v8, jsc, spidermonkey, quickjs) from what we can tell.

Functions declared inside blocks are not hoisted to the top of the function; the equivalent var declaration is, though. But, the initialization is done only in the block.

Example:

if (typeof 'print' === 'undefined') {
  print = console.log.bind(console);
}

(function test() {
  print(typeof inner);
  if (true) {
    print(typeof inner);
    function inner() {}
  }
})();

This prints: undefined function in v8, spidermonkey, jsc.
In rhino before this patch it prints undefined undefined, but it is coherent with other engines after this PR.

[gbrail]

I'm all for doing it. I wonder why there aren't any test262 tests that fail because of this behavior, though. Are there other tests we can pull from the V8 test suite (We have also used the "mjsunit" tests from the V8 repo somewhere) so that we can make sure that we're not off on our own here? Otherwise I think this is a good change!

[gbrail]

This looks fine to me. Thanks for doing it!

[andreabergia]

I'm all for doing it. I wonder why there aren't any test262 tests that fail because of this behavior, though. Are there other tests we can pull from the V8 test suite (We have also used the "mjsunit" tests from the V8 repo somewhere) so that we can make sure that we're not off on our own here? Otherwise I think this is a good change!

Sorry, didn't get around to check that.
I'm a bit surprised too that there were no test262 cases, honestly.

[rbri]

Sorry, didn't get around to check that.
I'm a bit surprised too that there were no test262 cases, honestly.

Same here but at least there was a test in HtmlUnit/core-js and also a fix for that. Have removed our fix and all the tests are still passing. Thanks @andreabergia for all your work (hope for more)

HtmlUnit@e725bba
HtmlUnit/htmlunit-core-js@f6df509