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

Gather data and decide if block-scoped functions should be redeclarable #320

Closed
syg opened this Issue Jan 23, 2016 · 8 comments

Comments

Projects
None yet
5 participants
@syg
Member

syg commented Jan 23, 2016

Firefox, Chrome, and Edge have all run into web-compat breakages regarding block-scope function redeclaration. Per ES2015, the following is an early error.

{
  function f() { }
  function f() { }
}

Many existing sites, however, have variants of the above snippet and break. See the bugs for Firefox here, for Chrome here, and for Edge here.

It is my understanding the vendors here agree to back off on early errors for now and implement semantics described by Dave Herman here, preferably with a deprecation warning visible in the console.

I would still like to see the early errors implemented, but it is clear there are major breakages and we need data to see if the spec needs changing.

FWIW, some prominent sites that have been broken have already rolled out fixes.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Jan 23, 2016

Member

@syg can you expand on what sites are broken and which have been fixed? This will help with a gut-check for how widespread the issue is.

FWIW, Microsoft/ChakraCore#144 is a bug against Chakra for not implementing the standard semantics. We actually haven't gotten around to fixing the bug yet and so don't have any compat experience to share yet :(

Member

bterlson commented Jan 23, 2016

@syg can you expand on what sites are broken and which have been fixed? This will help with a gut-check for how widespread the issue is.

FWIW, Microsoft/ChakraCore#144 is a bug against Chakra for not implementing the standard semantics. We actually haven't gotten around to fixing the bug yet and so don't have any compat experience to share yet :(

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Jan 23, 2016

Member

@bterlson For Firefox we saw Cisco Spark breakage, which reportedly has been fixed upstream already. The compat fix I landed in Firefox has telemetry probes, so once that hits Nightly we might get a better idea, though the Nightly population is small in comparison to release.

I think Chrome saw foxnews.com video player breakage. If V8's compat fix doesn't have telemetry in it I would certainly recommend they be added.

Member

syg commented Jan 23, 2016

@bterlson For Firefox we saw Cisco Spark breakage, which reportedly has been fixed upstream already. The compat fix I landed in Firefox has telemetry probes, so once that hits Nightly we might get a better idea, though the Nightly population is small in comparison to release.

I think Chrome saw foxnews.com video player breakage. If V8's compat fix doesn't have telemetry in it I would certainly recommend they be added.

@concavelenz

This comment has been minimized.

Show comment
Hide comment
@concavelenz

concavelenz Jan 23, 2016

I assume this is sloppy mode only?

concavelenz commented Jan 23, 2016

I assume this is sloppy mode only?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Jan 23, 2016

Member

Yes, sloppy only!

Member

bterlson commented Jan 23, 2016

Yes, sloppy only!

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Jan 23, 2016

Member

The reason this wasn't addressed in es6 the Annex B was because snippets like this:

{function f(){console.log("f1")}
 f();  // "f1" in SpiderMonkey, "f2" in most others
 function f(){console.log("f2")}
}

was not interoperable across browsers.

However, things like the following are interoperable but not covered by Annex B:

{function f(){console.log("f1")}
 function f(){console.log("f2")}
 f();   //"f2" in all browsers
}

if (true) {
  function g(){console.log("g1")}
  function g(){console.log("g2")}
}
g();   //"g2" in all browsers

It seems to me that there is a fairly minimal addition to Annex B3.3 (sloppy mode) that can cover these cases:

  1. The 1st early error check in 14.1.2 for FunctionBody:FunctionStatementList should not generate an early error for multiple function declarations using the same name. Instead it should treat it as a warning worthy condition.
  2. During BlockDeclarationInstantiation only the last occurring FunctionDeclaration for any name should be processed. Such a final declaration is treated as if it was the sole declaration of that name in the block. The other Annex B3.3 compatibility affordances also apply to the final declaration.
Member

allenwb commented Jan 23, 2016

The reason this wasn't addressed in es6 the Annex B was because snippets like this:

{function f(){console.log("f1")}
 f();  // "f1" in SpiderMonkey, "f2" in most others
 function f(){console.log("f2")}
}

was not interoperable across browsers.

However, things like the following are interoperable but not covered by Annex B:

{function f(){console.log("f1")}
 function f(){console.log("f2")}
 f();   //"f2" in all browsers
}

if (true) {
  function g(){console.log("g1")}
  function g(){console.log("g2")}
}
g();   //"g2" in all browsers

It seems to me that there is a fairly minimal addition to Annex B3.3 (sloppy mode) that can cover these cases:

  1. The 1st early error check in 14.1.2 for FunctionBody:FunctionStatementList should not generate an early error for multiple function declarations using the same name. Instead it should treat it as a warning worthy condition.
  2. During BlockDeclarationInstantiation only the last occurring FunctionDeclaration for any name should be processed. Such a final declaration is treated as if it was the sole declaration of that name in the block. The other Annex B3.3 compatibility affordances also apply to the final declaration.
@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Jan 25, 2016

Member

We know of a couple: Not only Cisco WebEx, but also the Akamai Media Player (used by Fox News's website, for example). I'd guess that these come from the same underlying pattern of copying and pasting a function definition between files, and then having a "module system" concatenate multiple files together into the same sloppy-mode block-scoped region.

Member

littledan commented Jan 25, 2016

We know of a couple: Not only Cisco WebEx, but also the Akamai Media Player (used by Fox News's website, for example). I'd guess that these come from the same underlying pattern of copying and pasting a function definition between files, and then having a "module system" concatenate multiple files together into the same sloppy-mode block-scoped region.

@bterlson bterlson added the needs data label Feb 18, 2016

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 24, 2016

Member

@bterlson @concavelenz I'm not sure this should be strict mode only. It should be Annex B.3.3 only, but those set of behaviors aren't limited to strict mode, right?

Edit: It most certainly should be strict mode only. I was blind; ignore this message!

Member

syg commented Feb 24, 2016

@bterlson @concavelenz I'm not sure this should be strict mode only. It should be Annex B.3.3 only, but those set of behaviors aren't limited to strict mode, right?

Edit: It most certainly should be strict mode only. I was blind; ignore this message!

@syg

This comment has been minimized.

Show comment
Hide comment
@syg

syg Feb 25, 2016

Member

@littledan Proposed spec change here if you'd like to take a look.

Member

syg commented Feb 25, 2016

@littledan Proposed spec change here if you'd like to take a look.

syg added a commit to syg/ecma262 that referenced this issue Feb 26, 2016

syg added a commit to syg/ecma262 that referenced this issue Feb 27, 2016

littledan added a commit to littledan/ecma262 that referenced this issue Mar 4, 2016

Normative: Allow duplicate FunctionDeclarations in a block
A previously overlooked aspect of intersection semantics for
 #sec-block-level-function-declarations-web-legacy-compatibility-semantics
is the ability to have multiple function declarations with the same name
in the same block. This patch relaxes that constraint in sloppy mode.
Because it relaxes a static semantics rule, and replaces rather than
simply adds behavior, it is in the main spec text rather than Annex B.
The patch also puts the non-normative text in Annex B in an emu-note.

Fixes tc39#320.

littledan added a commit to littledan/ecma262 that referenced this issue Mar 4, 2016

Normative: Allow duplicate FunctionDeclarations in a block
A previously overlooked aspect of intersection semantics for
 #sec-block-level-function-declarations-web-legacy-compatibility-semantics
is the ability to have multiple function declarations with the same name
in the same block. This patch relaxes that constraint in sloppy mode.
Because it relaxes a static semantics rule, and replaces rather than
simply adds behavior, it is in the main spec text rather than Annex B.
The patch also puts the non-normative text in Annex B in an emu-note.

Fixes tc39#320.

littledan added a commit to littledan/ecma262 that referenced this issue Mar 9, 2016

Normative: Allow duplicate FunctionDeclarations in a block
A previously overlooked aspect of intersection semantics for
 #sec-block-level-function-declarations-web-legacy-compatibility-semantics
is the ability to have multiple function declarations with the same name
in the same block. This patch relaxes that constraint in sloppy mode.
Because it relaxes a static semantics rule, and replaces rather than
simply adds behavior, it is in the main spec text rather than Annex B.
The patch also puts the non-normative text in Annex B in an emu-note.

Fixes tc39#320.

@bterlson bterlson closed this in #453 Jun 2, 2016

bterlson added a commit that referenced this issue Jun 2, 2016

Normative: Allow duplicate FunctionDeclarations in a block (#453)
A previously overlooked aspect of intersection semantics for
 #sec-block-level-function-declarations-web-legacy-compatibility-semantics
is the ability to have multiple function declarations with the same name
in the same block. This patch relaxes that constraint in sloppy mode.
Because it relaxes a static semantics rule, and replaces rather than
simply adds behavior, it is in the main spec text rather than Annex B.
The patch also puts the non-normative text in Annex B in an emu-note.

Fixes #320.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment