New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annex B.3.3 still doesn't actually allow you to redeclare sloppy functions in block #913

Open
syg opened this Issue May 8, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@syg
Member

syg commented May 8, 2017

(Thanks to @anba yet again.)

The conclusion to #320 was that we'd allow multiple same-named functions in block in sloppy mode.

Consider the following program:

(function () {
  {
    function g() {1}
    function g() {2}
  }
  print(g);
})();

On all of SpiderMonkey, V8, JSC, and Chakra, this prints function g() {2}. That is, all implementations agree that Annex B.3.3 semantics should apply to the inner, redeclared g.

The actual letter of Annex B.3.3, though, all say something like "If replacing the FunctionDeclaration f with a VariableStatement that has F as a BindingIdentifier would not produce any Early Errors ..."

The thing is, replacing either function g above with an actual var g actually would produce an Early Error, since the allowance we gave to multiple FIB is specifically tied to their FunctionDeclarations. That is,
the following program is an Early Error:

(function () {
  {
    function g() {1}
    var g; /*function g() {2}*/
  }
  print(g);
})();

So according to the current spec text, Annex B.3.3 does not apply to g. This is a spec oversight, since I'm pretty sure we specifically tried to allow this case.

syg added a commit to syg/ecma262 that referenced this issue May 8, 2017

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg May 8, 2017

Member

@bterlson Feedback on the proposed language in that commit?

Member

syg commented May 8, 2017

@bterlson Feedback on the proposed language in that commit?

@bakkot

This comment has been minimized.

Show comment
Hide comment
@bakkot

bakkot May 23, 2017

Contributor

@syg that commit seems fine to me insofar as we are willing to continue having hypotheticals in the spec, which is Not Great but works ok (and is after all what we're already doing).

Contributor

bakkot commented May 23, 2017

@syg that commit seems fine to me insofar as we are willing to continue having hypotheticals in the spec, which is Not Great but works ok (and is after all what we're already doing).

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Sep 6, 2017

Contributor

I don't think the proposed approach works for switch-statements when duplicate function declarations are present across multiple case-clauses:

function f() {
    switch (1) {
        case 1:
          function fb(){} // B.3.3.1 replaces this |var fb|.
        default:
          function fb(){} // The hypothetical block also needs to replace this function.
    }
    print(typeof fb); // Should print "function" per web-reality
}
f();
Contributor

anba commented Sep 6, 2017

I don't think the proposed approach works for switch-statements when duplicate function declarations are present across multiple case-clauses:

function f() {
    switch (1) {
        case 1:
          function fb(){} // B.3.3.1 replaces this |var fb|.
        default:
          function fb(){} // The hypothetical block also needs to replace this function.
    }
    print(typeof fb); // Should print "function" per web-reality
}
f();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment