585536 - Functions declared in block statements should be block-local and hoisted to top of block

Status: RESOLVED DUPLICATE of bug 1071646

(Core :: JavaScript Engine, defect)

Description

[David Anderson [:dvander] - inactive, e-mail if emergency]

<@brendan> it'll be block local for harmony -- maybe we should do that now
<@brendan> block local, hoisted

Comment 1

[Brendan Eich [:brendan]]

Need to get this done for ES5 future-proofing.

/be

Comment 3

[Brendan Eich [:brendan]]

Idea is at this stage to make the breaking change only if "use strict" is in force. We are very late in the release cycle to try breaking the default version.

/be

Comment 4

[Brendan Eich [:brendan]]

This should block es5strict and fx4 or we're in trouble with the Ecma purity police (I'm a volunteer deputy).

/be

Comment 5

[Brendan Eich [:brendan]]

I filed bug 609832 on banning functions in blocks from es5strict for fx4, so we can take the time to get this right for the release after.

/be

Comment 6

[Brendan Eich [:brendan]]

Removing from es5strict blocker status. Bug 609832 will suffice for ES5 strict mode conformance.

/be

Comment 7

[Jim Blandy :jimb]

What about things like this?

{ if (true) function f() { } }

That's in a block, but not directly. I'd assume this and similar things should be forbidden.

How about:

switch (x) { function f() { ... } case 1: ... }

This seems to be as meaningful as a function within any other block. However, the spec doesn't treat the body of a switch statement as a block. Another special case to remember to specify.

Comment 8

[Brendan Eich [:brendan]]

A function declaration not directly within a block is not even supported by some browsers, and it won't be specified in Harmony. We could probably ban it in all modes, now (I mean Firefox 5's earliest nightlies).

JS has a more structured switch a la Java, not C's statement with labels anywhere in its body (Duff's Device). So you can't write anything but case labels and one default: directly within the braced switch body (which does delimit block scope for any let bindings in the labeled statements). Of course fall through works or fails just as in C.

/be

Comment 9

[Dave Herman [:dherman]]

> That's in a block, but not directly. I'd assume this and similar things should
> be forbidden.

Yes. We should be forbidding all function declarations anywhere except at program or function top-level.

> How about:
> 
> switch (x) { function f() { ... } case 1: ... }
> 
> This seems to be as meaningful as a function within any other block. However,
> the spec doesn't treat the body of a switch statement as a block. Another
> special case to remember to specify.

No need for these special cases. One simple rule: only allowed at function or program top-level.

Dave

Comment 10

[Dave Herman [:dherman]]

> No need for these special cases. One simple rule: only allowed at function or
> program top-level.

Dumb Dave. I was thinking of the wrong bug. Disregard my comment, sorry.

Dave

Comment 11

[Jim Blandy :jimb]

(In reply to comment #10)
> Dumb Dave. I was thinking of the wrong bug. Disregard my comment, sorry.

No problem. The patch I submitted for bug 609832 ensures that we reject function definitions within all sorts of statements.

Comment 12

[Brendan Eich [:brendan]]

Fixing this bug should allow block-scoped functions in strict mode code.

/be

Comment 15

[Jim Blandy :jimb]

I am not actively working on this. De-assigning myself.

Comment 17

[Ray Blaak]

I am struck by this from comment 8:
> A function declaration not directly within a block is not even supported by
> some browsers, and it won't be specified in Harmony. We could probably ban
> it in all modes, now (I mean Firefox 5's earliest nightlies).

And from bug 788963:
> It just struck me that FF is the only one to implement this block-level
> behavior, seeing as JS has no such thing as block-level scope AFAIK (IF
> blocks do not create new scopes), unless it's something new in the ES5.

"Some browsers" == only FF as far as I know. 

Just simply hosting inner func declarations to the top level would be so much more polite, and would let one declare functions closer to their actual use, since that would effectively and finally become portable.

Comment 18

[Dmitriy Kropachev]

Attachment: try_catch test case

Hello Guys,

I have attached test case which represent js issue which is only actual for FF , other browsers (Ie7-10,CHROME,OPERA) doesn't meet issue there.

This test case is very simple:
try{
alert(B2());
function B2(){
	alert('DOES_NOT_WORK.JS: If this has appeared then there is no issue');
	}
}finally{}

Actually I have found bug https://bugzilla.mozilla.org/show_bug.cgi?id=593176 with almost simillar problem, which is marked as duplicate of this one, but I do not see direct connection there possibly because I am not experienced developer and do not understand many things written here.

Can someone answer if there was decided to fix such behavior or there was decided to leave it as it is ?

Thanks.

Comment 20

[Boris Zbarsky [:bzbarsky]]

So this is becoming more and more of a compat problem, especially on mobile.  We really need to change things here.  I assume the spec is still totally in limbo?

Comment 21

[David Mandelin [:dmandelin]]

Updated description to make it clear what's needed. bz, please correct it if I got it wrong.

Comment 22

[:Benjamin Peterson]

Can we come to some agreement about where definitions should be hoisted to? Other browsers seem to just lift everything to the (function) body level.

Comment 23

[Ray Blaak]

Lifting to the function top body level is certainly exactly what I am looking for. 

That would preserve the existing semantics, e.g. instead of forcing users to do it manually, which is the existing FF behaviour. It seems like a relatively simple approach that just solves the problem.

Comment 24

[:Benjamin Peterson]

This is quite a thorny issue. Hoisting to body level probably makes the most sense for web compatibility, but it makes no sense if you have "let" like SM. For example,

function f(a) {
    var i = 3;
    if (a) {
        let i = 4;
        function k() { return i; }
        return k;
    }
}
f(true)

would evaluate to 3 under hoisting to body level! Still, it's hard to stomach considering the web can't use |let|.

It seems declaring a non-body-level function should act as if the function was a |let| variable in its block. Even if we say that, there are questions open. Is redefining a function equivalent to redeclaring a let and thus banned?

function f(a) {
    var g = 42;
    if (a) {
        function g() {}
    }
    return g;
}

What does |f(true)| evaluate to?

I wish there was a spec!

Comment 25

[Ray Blaak]

If |let| is equivalent to creating an additional lambda scope, then any declarations within it proceed as before.

So f above is equivalent the following, which shows no problem with the local k:

function f(a) {
    var i = 3;
    if (a) {
        return let_i(4);
    }

    function let_i(i) {
        function k() { return i; }
        return k;
    }
}

The moral, to me at least, is that you hoist to the top of the nearest enclosing scope. Regular control-blocks are not scopes.

Comment 26

[Ray Blaak]

As for the other f example, you have the same question (and answer) with this example I would think:

function f(a) {
    var g = 42;
    if (a) {
    }
    function g() {}
    return g;
}

Also, what do other browsers do? The overall issue is a lack of compatability. Let's all solve it with the same semantics, and the web is a better place. There might not be a spec, but there is de-facto behaviour.

Comment 27

[:Benjamin Peterson]

There's not de facto behavior wrt let.

Comment 28

[Florian Bender]

Please make this "uplifting behaviour" (if you implement it) *non-strict-mode* only, please (and work with the other vendors to do the same). It looks like this needs to be spec-ed somehow (even if it's not ECMA-approved), so you can specify a different (non-legacy non-uplifting) behaviour for strict mode.

Comment 29

[Ray Blaak]

I am curious, what do other browsers do in strict mode? Do they hoist local functions? If they do, and they remain technically compliant, could not FF do the same?

Comment 30

[:Benjamin Peterson]

V8 lifts to the body level except in "harmony scoping mode" (which I don't think is ever enabled on the web) in which case it scopes at the block level.

Comment 31

[Florian Bender]

Please, lets not promote bad/confusing programming behaviour. Even if V8 hoists local functions in strict mode, *Monkey should not do the same. Strict mode is opt-in already, so IMO there is no need to expand this curious behaviour for compat.

Comment 32

[Jeff Walden [:Waldo]]

Function "declarations" not at the top level of a function or overall program are a syntax error in strict mode in SpiderMonkey, per bug 609832.  I don't think that's going to change, regardless what happens here.  If ECMA decides to change that at some point we would probably follow, but I'm pretty sure we're not changing without their say-so.

Comment 33

[Boris Zbarsky [:bzbarsky]]

See bug 811421 comment 8 for another example of this causing compat problems on mobile...

Comment 34

[Florian Bender]

(In reply to Boris Zbarsky (:bz) from comment #33)
> See bug 811421 comment 8 for another example of this causing compat problems
> on mobile...

(In reply to John Schoenick [:johns] from comment #8)
> - m.doctissimo.fr […] uses a non-standard JS syntax for functions that could
> be fairly easily fixed by the authors (bug 585536)

So this is something for TE.

Comment 35

[Jason Orendorff [:jorendorff]]

V8's behavior:

1. Normally, functions in statement context are hoisted to the enclosing function.
   Later function declarations overwrite earlier ones in the same scope.

   As with standard local functions, "executing" the function "statement"
   does nothing; these functions are created and bound when the script (or innermost
   enclosing function, if any) is called.

2. With both "use strict" and --harmony-scoping, functions in statement context are
   hoisted to the enclosing block. TC39 once wanted this to be the standard. I don't
   think it's standard-track anymore, and --harmony-scoping is not a web-visible
   option, so we shouldn't emulate it.

So we will ignore (2).

The problem is that fully switching to (1) will break existing Mozilla-only code that does stuff like:

    if (monkey) {
        function yum(x) { return x + " banana"; }
    } else {
        function yum(x) { return x + " cookie"; }
    }

Comment 36

[Ray Blaak]

> The problem is that fully switching to (1) will break existing Mozilla-only
> code that does stuff like:
> 
>     if (monkey) {
>         function yum(x) { return x + " banana"; }
>     } else {
>         function yum(x) { return x + " cookie"; }
>     }

I think that Mozilla has to decide how portable it wants to be. That code will not work on any other browser (will it?). Does it even work in NodeJS?

Comment 37

[Juriy "kangax" Zaytsev]

AFAIK, the only other implementations mimicking Mozilla's function statements are ancient Safari (up to 3.0.4). See http://kangax.github.com/nfe/#function-statements

Comment 38

[Florian Bender]

(In reply to Jason Orendorff [:jorendorff] from comment #35)
> The problem is that fully switching to (1) will break existing Mozilla-only 
> code that does stuff like:
>     if (monkey) {
>         function yum(x) { return x + " banana"; }
>     } else {
>         function yum(x) { return x + " cookie"; }
>     }

Well that can be fixed, can't it? ;)

Comment 39

[Jeff Walden [:Waldo]]

(In reply to Florian Bender from comment #38)
> Well that can be fixed, can't it? ;)

Um, yes.  Just as easily as sites depending on any other semantics can be changed, to not depend on function statement -- which in every engine that implement them as anything other than a syntax error, implement them as an extension.  (Function *declarations* are not the same as function *statements*.)  It's not helpful to simply say that code can be fixed, because if that were the simple answer, nobody would be using function statements at all.

Comment 40

[Florian Bender]

Jason was talking about Mozilla-only code – which can easily be fixed before changing the engine. Thus the argument agains his first choice is not really valid. That's all what I was saying.

Comment 41

[Jeff Walden [:Waldo]]

Mozilla-only code includes code in mozilla-central, which is fixable.  (Although there might be a lot of it.)  But it also includes code in the universe of addons controlled by a multitude of developers, not all actively updating their code, not all of whom we could even contact if we wanted to.  That's no more easily fixed than use of function statements on the web is.  Plus you have sites that have Mozilla-specific code paths that happen to rely on this -- perhaps in Mozilla-specific scripts and code.  (Big sites do have partially distinct implementations for different browser engines, so it's not out of the question.)

Comment 42

[Jason Orendorff [:jorendorff]]

In V8, functions are indeed transplanted in a way that flouts lexical scoping.

    try {
        throw 0;
    } catch (x) {
        function f() { return x++; }
    }
    print(f());

The function is transplanted to toplevel, so the "x" inside the function does not refer to the "x" that is lexically in scope. So this gives a ReferenceError.

I think this is too horrible to consider implementing in SpiderMonkey. Instead I am just going to implement exactly what the summary says and hoist to the enclosing block.

Ray Blaak suggests instead hoisting to the enclosing "scope", but that makes me feel sick inside. Blocks should act like scopes, it's the only way.

Drafting an email to es-discuss to ask TC39 to resolve this.

Comment 43

[Ray Blaak]

Except that in JS, blocks are not scopes, so we can't pretend that they are. Closures are scopes, and I guess catch blocks are scopes. Anything else?

In Opera, the example above gives an error that is consistent with simply hoisting it. The call to f() doesn't even work inside of the catch block, which is interesting. It seems like the rule is really to blindly hoist it to the top (of the current closure?).

E.g. this:

    try {
        throw 0;
    } catch (x) {
        function f() { return x++; }
        alert("inside of catch: " + f());
    }
    alert("outside of catch: " + f());

gives:

Uncaught exception: ReferenceError: Undefined variable: x
Error thrown at line 8, column 8 in [file...]
    throw 0;
Error initially occurred at unknown location in f() in [file...]
called from line 11, column 8 in [file...]
    alert("inside of catch: " + f());

Comment 44

[Jason Orendorff [:jorendorff]]

(In reply to Ray Blaak from comment #43)
> Except that in JS, blocks are not scopes, so we can't pretend that they are.

They are in ES6.

I think the current ES6 draft actually specifies "hoist to enclosing block". Need to read more closely.

Comment 45

[Ray Blaak]

This bug only makes sense in the context of regular JS. If blocks have lexical scopes then you cannot hoist anything at all.

With lexical scoping, these are two distinct functions, inaccessible (by name) outside of the scopes:

if (test) {
  var a = 1;
  function f() {return a}
} else {
  var a = 2; // a different var!
  function f() {return a}
}
f() // error, f not found

You have to approach

Comment 46

[Jason Orendorff [:jorendorff]]

There's not a distinction to be drawn between ES6 and "regular JS". ES6 is the spec for JS.

'var' hoists to the enclosing function or top-level scope (or, in strict eval code, the eval scope, which is its own special thing). The binding is hoisted, that is. Initialization only happens when the var-statement executes. ('var' is never block-scoped, the way you have it in comment 45 -- that's not what I meant to say.)

'let' and 'const' bindings hoist to the enclosing block or top-level scope.

Function declarations will behave like 'let' and 'const'. This is what ES6 specifies, and I think implementations will converge to that. We'll go first.

Comment 47

[Jason Orendorff [:jorendorff]]

olliej pointed out these:
    https://bugs.webkit.org/show_bug.cgi?id=27226#c4

He doesn't think Apple can change the behavior of block-level function-declarations in non-strict mode. My understanding of the discussion is that Apple won't implement the spec.

Comment 48

[Sam Tobin-Hochstadt [:samth]]

Just for posterity, here's the (lightly edited) IRC log.

* samth summons olliej
<jorendorff> it's funny, the bug for this https://bugzilla.mozilla.org/show_bug.cgi?id=585536 contains two or three people separately discovering that hoisting, in a language that has let, just makes no sense
<samth> and implement block scope for functions!
<dherman> jorendorff: what hoisting do you mean?
 I think it makes sense with block-hoisting
<jorendorff> var-like hoisting for functions, i mean.
<dherman> ok yeah
 agreed
<olliej> samth: noooooooo
<jorendorff> even without let!
 what it does right now in a catch-block or with statement
<olliej> samth: it’s non trivial for us currently
<jorendorff> oh it's just so wrong
<dherman> yep
<olliej> wait
<olliej> samth: do you mean outside of strict mode????
<samth> olliej: yes
<olliej> samth: because we _cannot_ change that behaviour
<jorendorff> ruh roh
<samth> olliej: wait a second ...
<olliej> samth: jorendorff: this has been discussed on numerous occasions, outside of strict mode, the behavior of nested function statements is not reconcilable
 samth: no one can change to anyone else’s behavior without breaking some set of content
<samth> olliej: this is not my recollection of what's been said in the room at tc39 meetings
<olliej> samth: no
<olliej> samth: the agreement was we could get sane behavior in strict mode and in modules 
<dherman> I think olliej is right
<olliej> samth: but outside of strict mode we’re kind of hosed
<dherman> that sounds right
<olliej> i think ES5 even says that explicitly
 i had a great set of examples somewhere
 just a sec
<dherman> yes, ES5 strict mode poisoned block-local functions
<jorendorff> i'm going to implement the spec anyway
* dherman approves
<dherman> :)
<olliej> dherman: samth jorendorff https://bugs.webkit.org/show_bug.cgi?id=27226#c4
<jorendorff> i don't think you even need the eval in the last example
 and it might even pass strict mode
<olliej> comments 4, 5 and 7
 4,5 give the ways where any engine changing non-strict behavior will break _some_ code

Comment 50

[Rob Campbell [:rc] (:robcee)]

fascinating.

Comment 51

[Jason Orendorff [:jorendorff]]

It's time to retire this bug.

All the different JS implementations are too constrained by existing content to be able to implement breaking changes and establish a single common behavior. Once we discovered that, the urgency here evaporated.

It's unfortunate, but we've hit this a million times in other areas, where browser-specific is totally baked into the Web. Each time we die a little inside, but we've got to move on.

Now for the tiny glimmer of remaining hope: There *is* a set of use cases that do work in all implementations, and TC39 is trying to standardize just those few common use cases, in an appendix to ES6. See <http://people.mozilla.org/~jorendorff/es6-draft.html#sec-block-level-function-declarations-web-legacy-compatibility-semantics>. I've filed a new bug, bug 1071646, to make sure we follow that spec.

Comment 52

[Jason Orendorff [:jorendorff]]

Right, the other good news: under "use strict", block-local functions will do the right thing---exactly what this bug's summary is asking for. Filed bug 1071877 for that.

Comment 56

[Jason Orendorff [:jorendorff]]

We're going to fix this after all.