425827 - function does not always return a value warning confused by nested function

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Bob Clary [:bc] (inactive)]

Attachment: testcase

function foo()
{
  return 0;

  function bar()
  {
  }
}
foo();

Warning: function foo does not always return a value
Source File: doesnotreturnavalue.html
Line: 9, Column: 2
Source Code:
}

Comment 1

[Ryan VanderMeulen [:RyanVM]]

js> function foo()
{
  return 0;

  function bar()
  {
  }
}
js> foo();
0

No warnings in the error console when I load the testcase either.

Comment 2

[Jared Jacobs]

Attachment: screenshot of warning in console

I see this warning today in Firefox 28.0 for Mac when I click on the attached testcase. Regression?

It looks like I’m unable to reopen the issue. Would someone else, please? Thanks.

Comment 3

[Bob Clary [:bc] (inactive)]

jmjacobs, you probably have javascript.options.strict set to true? I was able to reproduce it in an Linux Nightly/31 on a build with that option set but not without it.

Looking at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions_and_function_scope/Strict_mode it is not clear to me how strict mode should handle this script.

Waldo, do you think this should be reopened?

Comment 4

[Jared Jacobs]

Yes, I have javascript.options.strict set to true. I must have done it a long time ago and forgotten.

Regardless, I’m certain that the warning should not be printed. Neither strict mode (as defined by Mozilla) nor ES6 forbid or even discourage inner function declarations as the last statements in their containing functions. Function declaration hoisting is never dangerous. Only variable hoisting is considered dangerous, since the value is not bound to the name until execution reaches the declaration.

The statement ”strict mode prohibits function statements not at the top level of a script or function” in the strict mode reference doc you linked to just forbids putting function declarations within blocks, loops, case statements, etc.

Comment 5

[Jeff Walden [:Waldo]]

Strict mode (the spec concept) and the unfortunately-named javascript.options.strict are different concepts.  :-\  Strict mode is a specified behavior.  The strict option is our own half-baked set of heuristics -- sometimes fully principled, sometimes not (as here).  Strict mode should not have anything to say about failure ("failure") of any function to return a value.

This is still valid, but these days (I didn't know this, had to debug it myself) you have to pass -w to the shell to get a warning with options("strict"):

[jwalden@find-waldo-now src]$ dbg/js/src/js -w
js> options("strict"); void 0
js> function f() { return 0; function g() {} }
typein:2:13 strict warning: function f does not always return a value:
typein:2:13 strict warning: function f() { return 0; function g() {} }
typein:2:13 strict warning: .............^

Alternatively there's options("werror"), but that's by its very nature (converting unspecified warning conditions into errors) not-to-spec, so I hesitate to suggest it too much lest people think it's something they should use at all.

The relevant code that would need changing to address this is HasFinalReturn in Parser.cpp.  Honestly, tho, I'm not sure I much like this warning if it's going to try to hackishly determine falling-off-the-endness without some sort of dominator analysis or something to show it's actually possible.  But I have better things to do than argue the case as far I'd need to actually get that to happen, I'm pretty sure.

Comment 6

[Jeff Walden [:Waldo]]

Fixed by removal of the relevant warning in 1046964.