727139 - inner function declaration in if branch not found

Status: RESOLVED DUPLICATE of bug 585536

(Core :: JavaScript Engine, defect)

Description

[Ray Blaak]

Attachment: functionBug.html - click on links to try out working vs failed functions

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0.1) Gecko/20100101 Firefox/10.0.1
Build ID: 20120208060813

Steps to reproduce:

I have encountered a JavaScript bug in FF 10.0.1. Basically if I declare an inner function in an if branch, it cannot be referenced above its declaration. If I declare it at the "top level" of an enclosing function it works fine.

This issue only happens on FireFox. The other browsers (Chrome, Opera, Safari, IE) all execute things fine.

E.g. this works:
      function doTopLevelFunction()
      {
        if (true)
        {
          localFunc();
        }

        function localFunc()
        {
          alert("called top level localFunc");
        }
      }

This does not:
      function doDeclaredBranchFunction()
      {
        if (true)
        {
          localFunc();

          function localFunc()
          {
            alert("called declared branch localFunc");
          }
        }
      }


Actual results:

I got this error:

  Error: localFunc is not defined
  Source File: file:///D:/.../functionBug.html
  Line: 34



Expected results:

It should have found and executed the function, i.e. shown the alert popup.

Comment 1

[Tim (fmdeveloper)]

Confirmed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.26) Gecko/20120128 Firefox/3.6.26 ( .NET CLR 3.5.30729; .NET4.0E) ID:20120128224517 and Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120214 Firefox/13.0a1 ID:20120214031227

Comment 2

[Boris Zbarsky [:bzbarsky]]

Per spec, that code should just throw a SyntaxError and fail to parse; the ES spec does not allow function declarations inside conditionals.

In practice, browsers end up supporting this, but in slightly different ways.  SpiderMonkey makes the invalid function declaration above an alias for:

  var localFunc = function () {
    alert("called declared branch localFunc");
  }

and since you have this code after the localFunc() call, you're trying to call undefined.

IE and Chrome do bizarre things with this case that involve hoisting the function out the conditional entirely in some cases.

I suggest not writing JavaScript that's invalid if you want to avoid browser inconsistencies.

I believe the next ES spec version will define behavior here, thought, and we have a bug tracking implementing whatever it defines.  This should be a duplicate of that bug.

Comment 4

[Ray Blaak]

I suppose you are correct to adhere to the spec (!), although I do wonder about defacto standards if all other browsers implement this. ActionScript also supports it. Firefox is standing alone here.

The reason I tend to use this style is to have callbacks close to usage. The restriction seems artificial, can force related code to be far away. E.g.

  if (doItNow) {
    server.sendRequest(args, onResponse);

    function onResponse(result) {
      // processing
    }
  }

I am surprised this is considered a difficult problem. The semantics of the inner declaration, as I understand them, is that it is equivalent to textually moving them to the top level of the enclosing function. In particular, the presence of the function is not affected by whether the condition block is executed or not (or is it?).

I would take the simple approach of processing all declared inner functions at once, treating them as if they were at the same level. Couldn't this be easily done?

SpiderMonkey's approach in particular seems incorrect, exactly because it also does not allow forward usages before the declaration.

The simple, almost syntactic transformation seems to avoid all the problems.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> I suppose you are correct to adhere to the spec

Well, more precisely we extended the spec in a particular way and others did it in other ways.

> if all other browsers implement this

Other browsers implement totally different things that happen to agree on this particular snippet on code but disagree on other snippets of code.

> The restriction seems artificial

Perhaps.  Since everyone implementing function declarations in if statements gave them completely different behavior, perhaps the issue is not as obvious as you think?

> The semantics of the inner declaration, as I understand them, is that it is equivalent
> to textually moving them to the top level of the enclosing function.

Again, per spec the semantics are that this is a syntax error.

Per proposed spec changes the semantics are that the function is moved to the top of the _block_.

> In particular, the presence of the function is not affected by whether the condition
> block is executed or not (or is it?).

It is in Firefox and some other browsers.  It's not in IE in certain modes and in some situations (I think they're inconsistent about it) and some other browsers.  Per the proposed spec changes the function declaration would only be available inside the condition block, so would certainly depend on whether the condition block is executed.

> I would take the simple approach of processing all declared inner functions at once,
> treating them as if they were at the same level. Couldn't this be easily done?

It could, but it would break existing content that does:

  if (condition) {
    function f() { /* something */ }
    f();
  } else {
    function f() { /* something else */ }
    f();
  }

In your case, I'd recommend just putting the function declarations at the top of the block if you want to declare functions inside the block; that should work in every single browser without issues.

Comment 6

[Ray Blaak]

(In reply to Boris Zbarsky (:bz) from comment #5)
> > The restriction seems artificial
> 
> Perhaps.  Since everyone implementing function declarations in if statements
> gave them completely different behavior, perhaps the issue is not as obvious
> as you think?

Most likely. My goal here is improving my understanding. I do note I only see different behaviour with Firefox and SpiderMonkey. The others are consistent.

> Per the proposed spec changes the function declaration
> would only be available inside the condition block, so would certainly
> depend on whether the condition block is executed.

I can certainly see how that can complicate things.

> > [couldn't we treat all inners at the same level?]
> 
> It could, but it would break existing content that does:
> 
>   if (condition) {
>     function f() { /* something */ }
>     f();
>   } else {
>     function f() { /* something else */ }
>     f();
>   }

Given that conditionals don't introduce new lexical scope anyway, isn't that equivalent to this code, which is just as broken? 
   if (condition) {
     f();
   } else {
     f();
   }

   function f() { /* something */ }
   function f() { /* something else */ }

Hmm. You get broken behaviour only if you follow SpiderMonkey's approach of a var assignment. For all of the other browsers I tried (Opera, Chrome, Safari, IE), the behaviour is in fact consistent with the simple approach I am adocating. E.g. all these duplicate f() code snippets are equivalent, including this one:

   if (condition) {
     f();
     function f() { /* something */ }
   } else {
     f();
     function f() { /* something else */ }
   }

That equivalence is in fact the cleanest approach in my opinion, given that there is no lexical scoping changes being considered here.

> In your case, I'd recommend just putting the function declarations at the
> top of the block if you want to declare functions inside the block; that
> should work in every single browser without issues.

That is certainly the practical approach I will have to take. Thanks for your responses.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> The others are consistent.

On this one particular code snippet, not in general.

> Given that conditionals don't introduce new lexical scope anyway

They will pretty soon (see "let", which has been supported in Spidermonkey for over 5 years now).

> isn't that equivalent to this code

Apparently not in general...

> all these duplicate f() code snippets are equivalent

Interesting.  This definitely did not use to be the case last time I tested this.

Comment 8

[Ray Blaak]

Attachment: update test showing the variations on duplicated inner function

As per the last comment, I explore the variations on a duplicated inner function of the same name, both inside and outside of the conditional, declared before and after usage.

Behaviour in the other browsers is identical (e.g. last declared function is used, no matter where declared), except for Firefox.