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 web compatibility concern #73

Closed
littledan opened this Issue Sep 28, 2015 · 38 comments

Comments

Projects
None yet
4 participants
@littledan
Member

littledan commented Sep 28, 2015

I have found that some websites don't work properly under Annex B 3.3. When interacting with Reddit comments, there is code with the following pattern (in the global scope):

try {
  function x() { }
} catch { }

x()

x isn't hoisted at all under Annex B, but it must for compatibility with the current version of Reddit.

I could imagine the annex having a second rule, which is when a sloppy-mode block-scoped function declaration is made in the global scope, the appropriate property of the global object is also set when the function declaration is reached (unconditionally, since this actually couldn't lead to an early error). I think this would fix that website, but would it cause any other problems?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 28, 2015

Member

This should already be covered I believe as this is certainly in the intersection semantics among browsers today. I think x is hoisted because it meets the requirements in step 1.a and 1.a.ii.

Member

bterlson commented Sep 28, 2015

This should already be covered I believe as this is certainly in the intersection semantics among browsers today. I think x is hoisted because it meets the requirements in step 1.a and 1.a.ii.

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Sep 28, 2015

Contributor

https://bugs.ecmascript.org/show_bug.cgi?id=4428 contains more information what needs to be changed in GlobalDeclarationInstantiation to enable B.3.3 for scripts.

Contributor

anba commented Sep 28, 2015

https://bugs.ecmascript.org/show_bug.cgi?id=4428 contains more information what needs to be changed in GlobalDeclarationInstantiation to enable B.3.3 for scripts.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 28, 2015

Member

Ha! I completely missed the point about this being in global code. Sorry I didn't read it thoroughly. The intention was always that the code above would not to be broken but spec doesn't reflect this atm. Will make updates (likely based on @anba's gist) unless someone sends me a PR for the same first :)

Member

bterlson commented Sep 28, 2015

Ha! I completely missed the point about this being in global code. Sorry I didn't read it thoroughly. The intention was always that the code above would not to be broken but spec doesn't reflect this atm. Will make updates (likely based on @anba's gist) unless someone sends me a PR for the same first :)

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Sep 29, 2015

Member

OK, this looks good to me. The only annoying part is the CanDeclareGlobalFunction check, which has to be at runtime, unlike the check inside functions. But it seems like reasonable semantics to me.

Member

littledan commented Sep 29, 2015

OK, this looks good to me. The only annoying part is the CanDeclareGlobalFunction check, which has to be at runtime, unlike the check inside functions. But it seems like reasonable semantics to me.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Sep 29, 2015

Member

By the way, a change is needed for eval too.

Member

littledan commented Sep 29, 2015

By the way, a change is needed for eval too.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Sep 29, 2015

Member

Also, it looks like the last two ReturnIfAbrupts can't be abrupt. Would it make sense to replace these with asserts that it's not an abrupt completion?

Member

littledan commented Sep 29, 2015

Also, it looks like the last two ReturnIfAbrupts can't be abrupt. Would it make sense to replace these with asserts that it's not an abrupt completion?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 29, 2015

Member

Which two ReturnIfAbrupt's? In current Annex B or @anba's gist?

I believe that CanDeclareGlobalFunction check occurs "statically" upon entry to the script (once you know the state of the global object for the script), which should be the same as inside functions. It's possible you know statically if there will be a runtime error as well, which I think your last comment is getting at... but I'm not convinced the SetMutableBinding call can't throw yet. Will think more.

Member

bterlson commented Sep 29, 2015

Which two ReturnIfAbrupt's? In current Annex B or @anba's gist?

I believe that CanDeclareGlobalFunction check occurs "statically" upon entry to the script (once you know the state of the global object for the script), which should be the same as inside functions. It's possible you know statically if there will be a runtime error as well, which I think your last comment is getting at... but I'm not convinced the SetMutableBinding call can't throw yet. Will think more.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Sep 29, 2015

Member

I meant in @anba 's gist.

Member

littledan commented Sep 29, 2015

I meant in @anba 's gist.

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Sep 30, 2015

Contributor

Also, it looks like the last two ReturnIfAbrupts can't be abrupt. Would it make sense to replace these with asserts that it's not an abrupt completion?

The ReturnIfAbrupt after benv.GetBindingValue() should be replaced with an assertion. In the gist and also in the ES2015 spec for B.3.3.
The other ReturnIfAbrupt (after genv.SetMutableBinding()) cannot be replaced with an assertion, because the global object could be a Proxy exotic object which throws.

Contributor

anba commented Sep 30, 2015

Also, it looks like the last two ReturnIfAbrupts can't be abrupt. Would it make sense to replace these with asserts that it's not an abrupt completion?

The ReturnIfAbrupt after benv.GetBindingValue() should be replaced with an assertion. In the gist and also in the ES2015 spec for B.3.3.
The other ReturnIfAbrupt (after genv.SetMutableBinding()) cannot be replaced with an assertion, because the global object could be a Proxy exotic object which throws.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Sep 30, 2015

Member

Could we have an Assert that it only throws if the global object is a Proxy? Then, in hosting environments where the global object is not a proxy (e.g., web browsers), implementors will know that a throw will not happen. Or maybe a note, if an assert wouldn't be appropriate?

Member

littledan commented Sep 30, 2015

Could we have an Assert that it only throws if the global object is a Proxy? Then, in hosting environments where the global object is not a proxy (e.g., web browsers), implementors will know that a throw will not happen. Or maybe a note, if an assert wouldn't be appropriate?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Sep 30, 2015

Member

Implementations are always free to elide spec steps that are not observable in their implementation. There are many cases like these and I'm not sure we want to be in the business of making notes for all of them.

Member

bterlson commented Sep 30, 2015

Implementations are always free to elide spec steps that are not observable in their implementation. There are many cases like these and I'm not sure we want to be in the business of making notes for all of them.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Sep 30, 2015

Member

@littledan that sort of analysis applies to pretty much every throw in the spec. If an implementation can prove that a condition can never occur in their implementation then they can ignore it.

Member

allenwb commented Sep 30, 2015

@littledan that sort of analysis applies to pretty much every throw in the spec. If an implementation can prove that a condition can never occur in their implementation then they can ignore it.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 8, 2015

Member

Under what conditions should annex B EvalDeclarationEvaluation actually create the var binding? The hand-wave "if it wouldn't otherwise cause an early error" works as a starting point for inside eval, but what about the leakage outside the eval? Seems like in order to faithfully match the static semantics we'd have to do a walk up the scope chain looking for lexical bindings of the same name and if found, not do the hoisting. Any way to avoid this?

Member

bterlson commented Oct 8, 2015

Under what conditions should annex B EvalDeclarationEvaluation actually create the var binding? The hand-wave "if it wouldn't otherwise cause an early error" works as a starting point for inside eval, but what about the leakage outside the eval? Seems like in order to faithfully match the static semantics we'd have to do a walk up the scope chain looking for lexical bindings of the same name and if found, not do the hoisting. Any way to avoid this?

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 8, 2015

Member

I see the options as:

  1. Hoist unconditionally from blocks (what browsers do now)
  2. Hoist conditionally (annoying to implement)
  3. Don't hoist (web-incompatible)

Option 1. would kinda be OK except it goes against the whole spirit of Annex B, to make bindings look like they are lexical sorta, even in sloppy mode. Option 2 is the "correct" one which I was also realizing would be a lot of work to implement properly (but you already have to do much of that work, just to throw exceptions in cases like (function() { { let x; eval('var x') } })()). I don't think 3 is an option--too much code on the web expects hoisting out of blocks from scripts, and many scripts are eval'd. Are there other options?

Member

littledan commented Oct 8, 2015

I see the options as:

  1. Hoist unconditionally from blocks (what browsers do now)
  2. Hoist conditionally (annoying to implement)
  3. Don't hoist (web-incompatible)

Option 1. would kinda be OK except it goes against the whole spirit of Annex B, to make bindings look like they are lexical sorta, even in sloppy mode. Option 2 is the "correct" one which I was also realizing would be a lot of work to implement properly (but you already have to do much of that work, just to throw exceptions in cases like (function() { { let x; eval('var x') } })()). I don't think 3 is an option--too much code on the web expects hoisting out of blocks from scripts, and many scripts are eval'd. Are there other options?

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 8, 2015

Member

Yeah I think option 2 is the right way to go. As an initial strawman, I say the behavior should be identical to however var decls inside eval in non-annex-B semantics works. Investigating this now.

Member

bterlson commented Oct 8, 2015

Yeah I think option 2 is the right way to go. As an initial strawman, I say the behavior should be identical to however var decls inside eval in non-annex-B semantics works. Investigating this now.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 8, 2015

Member

Actually I take it back, since the current policy seems to be to avoid throwing errors (and vars do). So I'm going to spec a scope walk similar to EDI step 5 and if I find any lexical bindings, simply not hoist the function out of eval. Annoying to implement, yes, but seems correct :)

Member

bterlson commented Oct 8, 2015

Actually I take it back, since the current policy seems to be to avoid throwing errors (and vars do). So I'm going to spec a scope walk similar to EDI step 5 and if I find any lexical bindings, simply not hoist the function out of eval. Annoying to implement, yes, but seems correct :)

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 9, 2015

Member

Basically it should be just like the gist, except make the properties enumerable because it's eval, whereas normal global would be non-enumerable (why???).

Member

littledan commented Oct 9, 2015

Basically it should be just like the gist, except make the properties enumerable because it's eval, whereas normal global would be non-enumerable (why???).

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 9, 2015

Member

That's if the variable environment is the global object. Need to also handle when the variable environment is not the global object and also the case where there's intermediate bindings between the eval call and the variable environment, eg. { let x; { eval('var x = 1') } }.

Hoping to have a draft of this by EOD!

Member

bterlson commented Oct 9, 2015

That's if the variable environment is the global object. Need to also handle when the variable environment is not the global object and also the case where there's intermediate bindings between the eval call and the variable environment, eg. { let x; { eval('var x = 1') } }.

Hoping to have a draft of this by EOD!

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 9, 2015

Member

I think the spec already handles cases like that one.

Member

littledan commented Oct 9, 2015

I think the spec already handles cases like that one.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 9, 2015

Member

Which one? I listed a few cases :)

Member

bterlson commented Oct 9, 2015

Which one? I listed a few cases :)

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 9, 2015

Member

You might be tempted to simply append the functions to the already-existing functionsToInitialize, but I don't think you can do this because the intersection semantics calls for only initializing the binding at EDI-time and setting the binding when the declaration is evaluated.

Member

bterlson commented Oct 9, 2015

You might be tempted to simply append the functions to the already-existing functionsToInitialize, but I don't think you can do this because the intersection semantics calls for only initializing the binding at EDI-time and setting the binding when the declaration is evaluated.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 9, 2015

Member

Also note that eval makes configurable properties on the global object (all of them, in eval and out, are enumerable afaict).

Member

bterlson commented Oct 9, 2015

Also note that eval makes configurable properties on the global object (all of them, in eval and out, are enumerable afaict).

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 12, 2015

Member

Do we have any evidence that function declarations in blocks within eval scripts actually exist on the web for the legacy interop use cases? If not, I don't think we should be trying to spec. an annex B behavior for them.

Remember, that annex B.3.3, exists to keep legacy code working that is already inoperable across browsers. We shouldn't be forcing implementations to add the complexity of eval'ed FiBs unless there is an actual legacy problem.

Member

allenwb commented Oct 12, 2015

Do we have any evidence that function declarations in blocks within eval scripts actually exist on the web for the legacy interop use cases? If not, I don't think we should be trying to spec. an annex B behavior for them.

Remember, that annex B.3.3, exists to keep legacy code working that is already inoperable across browsers. We shouldn't be forcing implementations to add the complexity of eval'ed FiBs unless there is an actual legacy problem.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 12, 2015

Member

@allenwb I'm not sure, but I feel like the burden of proof lies on us to decide that it's not a risk. In the absence of such data I think it's reasonable to include this as it's the minimal intersection of what implementations are already doing and will continue to do. Also there is a risk that implementations will be more accepting than the minimal intersection which seems bad... for example, Chrome is currently hosting eval'd function decls past lexical bindings.

Member

bterlson commented Oct 12, 2015

@allenwb I'm not sure, but I feel like the burden of proof lies on us to decide that it's not a risk. In the absence of such data I think it's reasonable to include this as it's the minimal intersection of what implementations are already doing and will continue to do. Also there is a risk that implementations will be more accepting than the minimal intersection which seems bad... for example, Chrome is currently hosting eval'd function decls past lexical bindings.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 13, 2015

Member

@bterlson We'd throw an exception in this case (as we did before), not silently accepting such a hoist. I have a test for this, though the whole sloppy mode lexical scoped thing is currently behind a flag. Throwing an exception violates the spec, but IIRC Edge does the same thing, so there!

@allenwb Reddit defines all of its functions in a try block, and expects them to be hoisted. Although Reddit includes this code using a script tag, you could easily imagine them refactoring their script to include another script executed with eval, using the same technique. I think it'd be rather surprising for users if the function hoisting behavior differed significantly for this case. This isn't a concrete example depending on that behavior, but I'd be really cautious about shipping something that removed that hoisting.

Member

littledan commented Oct 13, 2015

@bterlson We'd throw an exception in this case (as we did before), not silently accepting such a hoist. I have a test for this, though the whole sloppy mode lexical scoped thing is currently behind a flag. Throwing an exception violates the spec, but IIRC Edge does the same thing, so there!

@allenwb Reddit defines all of its functions in a try block, and expects them to be hoisted. Although Reddit includes this code using a script tag, you could easily imagine them refactoring their script to include another script executed with eval, using the same technique. I think it'd be rather surprising for users if the function hoisting behavior differed significantly for this case. This isn't a concrete example depending on that behavior, but I'd be really cautious about shipping something that removed that hoisting.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 13, 2015

Member

@littledan sorry I though it was a given that IE semantics are very odd in this situation :) Was trying to support the fact that we're not the only ones wanting a spec here. But I guess I failed at this since dev tools were lying to me about your semantics.

My thinking of how prevalent this is goes like the following - we have lots of evidence for var/const-in-eval expecting both that it hoists and creates a property on the global object. It would be a stretch to claim that only var/const are used for this purpose.

Member

bterlson commented Oct 13, 2015

@littledan sorry I though it was a given that IE semantics are very odd in this situation :) Was trying to support the fact that we're not the only ones wanting a spec here. But I guess I failed at this since dev tools were lying to me about your semantics.

My thinking of how prevalent this is goes like the following - we have lots of evidence for var/const-in-eval expecting both that it hoists and creates a property on the global object. It would be a stretch to claim that only var/const are used for this purpose.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 13, 2015

Member

@littledan I've lost the context of your reply to @bterlson. Under which conditions are you throwing?

I think there is agreement that functions in try-catch-finally blocks are already intended to be included in the B.3.3 semantics. But, I don't think user surprises is the relevant issue here. B.3.3 isn't intended as an enabler of new code that violates block scoping of function declarations. It exists solely to allow existing code to keep running. If the function-in-block-in-eval scope violation can't be found in existing code, then there is no need to specify or implement it.

Member

allenwb commented Oct 13, 2015

@littledan I've lost the context of your reply to @bterlson. Under which conditions are you throwing?

I think there is agreement that functions in try-catch-finally blocks are already intended to be included in the B.3.3 semantics. But, I don't think user surprises is the relevant issue here. B.3.3 isn't intended as an enabler of new code that violates block scoping of function declarations. It exists solely to allow existing code to keep running. If the function-in-block-in-eval scope violation can't be found in existing code, then there is no need to specify or implement it.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 13, 2015

Member

@allenwb Do you think functions-in-eval don't exist even though there is ample evidence of people putting other declaration forms in eval and expecting both hoisting behavior and global object property creation behavior? Also my feeling is if v8, sm, and chakra are going to have this behavior, it should be specified - agree?

@littledan FWIW, I've actually confirmed that my work canary (48.0.2531.0) behaves as I described: (function() { { let x; eval('function x() { console.log("c") }'); } x() })() will log 'c'. Perhaps this has been fixed recently though.

Member

bterlson commented Oct 13, 2015

@allenwb Do you think functions-in-eval don't exist even though there is ample evidence of people putting other declaration forms in eval and expecting both hoisting behavior and global object property creation behavior? Also my feeling is if v8, sm, and chakra are going to have this behavior, it should be specified - agree?

@littledan FWIW, I've actually confirmed that my work canary (48.0.2531.0) behaves as I described: (function() { { let x; eval('function x() { console.log("c") }'); } x() })() will log 'c'. Perhaps this has been fixed recently though.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 13, 2015

Member

@bterison the question is whether functions-in-block-in-eval-with-interoperable-out-of-scope-references exist. That seems like a probably much more limited use case than what you stated. Eval scoping rules are complex. So are the rules for legacy FiB compat. Combining them seems like a cross product of complexity. I agree that if a legacy semantics for this is going to be widely implemented, it needs to be specified. But if such legacy code doesn't actually exist, then it would be better, both now and for the future, to completely avoid that complexity.

I appreciate that we can't provide a negative and that it may be had to find real eval code that could be check. But it seem like we should at least try to look for actual uses before we assume that we have to add this complexity.

Member

allenwb commented Oct 13, 2015

@bterison the question is whether functions-in-block-in-eval-with-interoperable-out-of-scope-references exist. That seems like a probably much more limited use case than what you stated. Eval scoping rules are complex. So are the rules for legacy FiB compat. Combining them seems like a cross product of complexity. I agree that if a legacy semantics for this is going to be widely implemented, it needs to be specified. But if such legacy code doesn't actually exist, then it would be better, both now and for the future, to completely avoid that complexity.

I appreciate that we can't provide a negative and that it may be had to find real eval code that could be check. But it seem like we should at least try to look for actual uses before we assume that we have to add this complexity.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 13, 2015

Member

the question is whether functions-in-block-in-eval-with-interoperable-out-of-scope-references exist

Right, and since we have evidence that var-in-block-in-eval-with-interoperable-out-of-scope-references exist, it doesn't seem like a stretch to assert function-in-block-... exists as well. In an ideal world we'd have data ready for this, but data gathering is costly and I'm not sure I can justify it. Maybe v8 guys are willing to gather some data here? But, I think it's mostly moot as I bet we are all going to implement this.

What do you think about the semantics in this PR? Am I missing some complexity? As spec'd it doesn't seem too bad but if I'm missing a pile of complexity I might be convinced to leave it unspecified for now :)

Member

bterlson commented Oct 13, 2015

the question is whether functions-in-block-in-eval-with-interoperable-out-of-scope-references exist

Right, and since we have evidence that var-in-block-in-eval-with-interoperable-out-of-scope-references exist, it doesn't seem like a stretch to assert function-in-block-... exists as well. In an ideal world we'd have data ready for this, but data gathering is costly and I'm not sure I can justify it. Maybe v8 guys are willing to gather some data here? But, I think it's mostly moot as I bet we are all going to implement this.

What do you think about the semantics in this PR? Am I missing some complexity? As spec'd it doesn't seem too bad but if I'm missing a pile of complexity I might be convinced to leave it unspecified for now :)

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 13, 2015

Member

I'm not sure what you mean by var-in-block-in-eval... since var in a non-strict eval has always hosted to global/function level with an interoperable standard semantics.

I'll look deep at the PR. the sort of things I have in mind are direct/indirect eval differences, dynamically augmenting a inner block with an eval'ed function, etc.

Member

allenwb commented Oct 13, 2015

I'm not sure what you mean by var-in-block-in-eval... since var in a non-strict eval has always hosted to global/function level with an interoperable standard semantics.

I'll look deep at the PR. the sort of things I have in mind are direct/indirect eval differences, dynamically augmenting a inner block with an eval'ed function, etc.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 13, 2015

Member

I'm not sure what you mean by var-in-block-in-eval... since var in a non-strict eval has always hosted to global/function level with an interoperable standard semantics.

Isn't this also true of function decls inside direct eval?

Member

bterlson commented Oct 13, 2015

I'm not sure what you mean by var-in-block-in-eval... since var in a non-strict eval has always hosted to global/function level with an interoperable standard semantics.

Isn't this also true of function decls inside direct eval?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 13, 2015

Member

Isn't this also true of function decls inside direct eval?
I don't think that direct/indirect changes anything that is relevant here. And only at top level of the eval'ed script. FiB in eval is illegal/non-standard in ES<=5. So, you have to go back an look at what inter-operable intersection semantics actually exists among browsers. Are you sure it is the same for both eval and non-eval FiB?

Member

allenwb commented Oct 13, 2015

Isn't this also true of function decls inside direct eval?
I don't think that direct/indirect changes anything that is relevant here. And only at top level of the eval'ed script. FiB in eval is illegal/non-standard in ES<=5. So, you have to go back an look at what inter-operable intersection semantics actually exists among browsers. Are you sure it is the same for both eval and non-eval FiB?

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 14, 2015

Member

@bterlson Sorry for my inaccuracy; it's only with certain flags that it's fixed. Those flags aren't even staged right now due to web compatibility concerns. But we intend to fix that. You must be running with --es-staging (otherwise you have no let in sloppy mode) so it's not like we expose let with those semantics on the web.

About the evidence that you have that users expect const to hoist out of eval and add properties to the global function: this is off-topic, but that is frightening from the perspective of wanting to ship ES2015 const semantics, as opposed to our "legacy const" semantics. Do you have any more information that you could share about that?

@allenwb Hoisting out of blocks from eval is something that Chrome has supported for a long time, and from what bterlson is saying, sounds like IE has been supporting it as well. Do you know of browsers that didn't support this? As an implementor, my concern comes more from the direction of the conservative "how can we be sure that this won't break any websites" rather than "where can we find a very popular website which depends on this". If it's been in the intersection for a while, and we can find websites which touch on similar issues like Reddit, then that seems like strong enough circumstantial evidence to make me very cautious.

Member

littledan commented Oct 14, 2015

@bterlson Sorry for my inaccuracy; it's only with certain flags that it's fixed. Those flags aren't even staged right now due to web compatibility concerns. But we intend to fix that. You must be running with --es-staging (otherwise you have no let in sloppy mode) so it's not like we expose let with those semantics on the web.

About the evidence that you have that users expect const to hoist out of eval and add properties to the global function: this is off-topic, but that is frightening from the perspective of wanting to ship ES2015 const semantics, as opposed to our "legacy const" semantics. Do you have any more information that you could share about that?

@allenwb Hoisting out of blocks from eval is something that Chrome has supported for a long time, and from what bterlson is saying, sounds like IE has been supporting it as well. Do you know of browsers that didn't support this? As an implementor, my concern comes more from the direction of the conservative "how can we be sure that this won't break any websites" rather than "where can we find a very popular website which depends on this". If it's been in the intersection for a while, and we can find websites which touch on similar issues like Reddit, then that seems like strong enough circumstantial evidence to make me very cautious.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Oct 14, 2015

Member

@littledan The base problem is that legacy browser did not have a single common semantics for functions in blocks. So it was impossible for ES6 to define a semantics that preserves all existing FiB code across all browsers. TC39 (and in particular @bterlson) spent consider time analyzing this and determine a path out of that mess. Brian should be able to give you pointers to the research he did. The chosen approach is to only worry about pre-existing FiB code that actually worked in all legacy browsers. Brain's analysis identified the three code patterns listed in the preface to B.3.3 as the only observed patterns that worked interoperably in all browsers. So, those are what is specified in B.3.3.

For any browser or legacy JS implementation that supports FiBs, there are probably other FiB usage patterns that differ from the ES6 semantics and which are not covered by B.3.3. But (unless you can demonstrate otherwise) the semantics required for such other patterns is not legacy browser interoperable and hence not part of the interoperable web. As such, it's not TC39's problem to fix. Each existing implementation needs to decide whether or not it needs to continue to support such non-interoperable legacy code and how to go about doing so. Perhaps an implementation will need to add additional special cases similar to those in B.3.3. But, in doing so they need to be very care to not break the normal ES6 FiB semantics. In some cases, a backwards compatibility switch might be necessary.

However, we have discovered that some things are missing from the ES6 B.3.3:

  1. FiBs in Script code. this is an oversight that needs to be handled. It's semantics are essentially the same as that provided in B.3.3 for function level code. But there are (at least) two differences. It can't be statically determined (without analyzing all scripts) whether the function reference in use cases 2&3 actually exist. So, we have to conservatively assume that such a reference exists (in another Script) and hence all FiBs need to get hoisted (unless doing so would create an error condition). The other difference is that in determining whether hoisting would produce an error, the semantics of global function declarations need to be used rather than the semantics of FunctionDeclarationInstantiation.

  2. FiBs contained in indirect evals. This is essentially the same as the above case.

  3. FiB contained in direct evals at either the Script or function level. This is the case for which I don't believe we have either a legacy interoperability analysis or evidence that it actually exists in the wild. (Brian, does Edge current support this case? If not, have you heard of it causing a problem?). the handling of this should be similar to the B.3.3 algorithm and 1) above, but it requires a dynamic analysis as to whether hoisting should not occur because it would introduce an error condition. This is possible, but I don't think we should try to specify it unless we have convinced that it occurs interoperablely in significant web sites.

Member

allenwb commented Oct 14, 2015

@littledan The base problem is that legacy browser did not have a single common semantics for functions in blocks. So it was impossible for ES6 to define a semantics that preserves all existing FiB code across all browsers. TC39 (and in particular @bterlson) spent consider time analyzing this and determine a path out of that mess. Brian should be able to give you pointers to the research he did. The chosen approach is to only worry about pre-existing FiB code that actually worked in all legacy browsers. Brain's analysis identified the three code patterns listed in the preface to B.3.3 as the only observed patterns that worked interoperably in all browsers. So, those are what is specified in B.3.3.

For any browser or legacy JS implementation that supports FiBs, there are probably other FiB usage patterns that differ from the ES6 semantics and which are not covered by B.3.3. But (unless you can demonstrate otherwise) the semantics required for such other patterns is not legacy browser interoperable and hence not part of the interoperable web. As such, it's not TC39's problem to fix. Each existing implementation needs to decide whether or not it needs to continue to support such non-interoperable legacy code and how to go about doing so. Perhaps an implementation will need to add additional special cases similar to those in B.3.3. But, in doing so they need to be very care to not break the normal ES6 FiB semantics. In some cases, a backwards compatibility switch might be necessary.

However, we have discovered that some things are missing from the ES6 B.3.3:

  1. FiBs in Script code. this is an oversight that needs to be handled. It's semantics are essentially the same as that provided in B.3.3 for function level code. But there are (at least) two differences. It can't be statically determined (without analyzing all scripts) whether the function reference in use cases 2&3 actually exist. So, we have to conservatively assume that such a reference exists (in another Script) and hence all FiBs need to get hoisted (unless doing so would create an error condition). The other difference is that in determining whether hoisting would produce an error, the semantics of global function declarations need to be used rather than the semantics of FunctionDeclarationInstantiation.

  2. FiBs contained in indirect evals. This is essentially the same as the above case.

  3. FiB contained in direct evals at either the Script or function level. This is the case for which I don't believe we have either a legacy interoperability analysis or evidence that it actually exists in the wild. (Brian, does Edge current support this case? If not, have you heard of it causing a problem?). the handling of this should be similar to the B.3.3 algorithm and 1) above, but it requires a dynamic analysis as to whether hoisting should not occur because it would introduce an error condition. This is possible, but I don't think we should try to specify it unless we have convinced that it occurs interoperablely in significant web sites.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Oct 15, 2015

Member

Well, I can only ship things in Chrome if they don't break too many websites which previously worked in Chrome. This could include websites which don't currently work in IE. I don't think we'll include a backwards compatibility switch; that would go against the 1JS spirit (where would this switch live anyway? how could we turn it on, given how slow the 'use strict' uptake is already?). And any extra hoisting/semantics will be, of course, observable and therefore not preserving the behavior in the spec. I hope we can work together as TC39 to find semantics which Chrome (and all other browsers) can ship. Brian's pull request seems to be a good step in that direction.

Member

littledan commented Oct 15, 2015

Well, I can only ship things in Chrome if they don't break too many websites which previously worked in Chrome. This could include websites which don't currently work in IE. I don't think we'll include a backwards compatibility switch; that would go against the 1JS spirit (where would this switch live anyway? how could we turn it on, given how slow the 'use strict' uptake is already?). And any extra hoisting/semantics will be, of course, observable and therefore not preserving the behavior in the spec. I hope we can work together as TC39 to find semantics which Chrome (and all other browsers) can ship. Brian's pull request seems to be a good step in that direction.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Oct 15, 2015

Member

So a key point is that my initial analysis into FIB did NOT contain any source in eval AT ALL in the dataset. This was a limitation of the methodology I used (basically, scraping inline scripts and scripts linked via a script tag). I think the intuition that if people are using var decls in try/catched eval and expecting them to be visible outside of the try/catch, they will also likely be using function decls in a similar manner.

Edge does support FIBIE (function-in-block-in-eval) and as far as I can tell all the implementations roughly align on the semantics I propose in this PR. I think the high order bit here is quite simply that we (browser vendors) are going to be shipping these semantics so something like this PR should go in to guide our efforts.

Member

bterlson commented Oct 15, 2015

So a key point is that my initial analysis into FIB did NOT contain any source in eval AT ALL in the dataset. This was a limitation of the methodology I used (basically, scraping inline scripts and scripts linked via a script tag). I think the intuition that if people are using var decls in try/catched eval and expecting them to be visible outside of the try/catch, they will also likely be using function decls in a similar manner.

Edge does support FIBIE (function-in-block-in-eval) and as far as I can tell all the implementations roughly align on the semantics I propose in this PR. I think the high order bit here is quite simply that we (browser vendors) are going to be shipping these semantics so something like this PR should go in to guide our efforts.

@bterlson

This comment has been minimized.

Show comment
Hide comment
@bterlson

bterlson Nov 13, 2015

Member

The Annex B EDI and GDI semantics are checked in so I will close this now. Separately, #162 needs to get resolved. Open new issues for anything else.

Member

bterlson commented Nov 13, 2015

The Annex B EDI and GDI semantics are checked in so I will close this now. Separately, #162 needs to get resolved. Open new issues for anything else.

@bterlson bterlson closed this Nov 13, 2015

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