Skip to content
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

Normative: Create global properties for sloppy function hosting as vars #888

Merged
merged 1 commit into from
Jul 18, 2018

Conversation

littledan
Copy link
Member

This patch removes a case where a synthetic undefined was visible
as a result of legacy sloppy-mode function hoisting.

This patch addresses #753

This patch removes a case where a synthetic *undefined* was visible
as a result of legacy sloppy-mode function hoisting.

This patch addresses tc39#753
@anba
Copy link
Contributor

anba commented Apr 12, 2017

I wondered why we decided to use CanDeclareGlobalFunction/CreateGlobalFunctionBinding instead of CanDeclareGlobalVar/CreateGlobalVarBinding, and look what I found -> https://bugs.ecmascript.org/show_bug.cgi?id=4428#c1. 😄

I think @syg should take a look at this change to confirm that this matches what he implemented in SpiderMonkey.

@rwaldron rwaldron added needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Sep 26, 2017
@syg
Copy link
Contributor

syg commented Sep 26, 2017

I'm pretty sure the PR matches my implementation. Sorry I completely forgot to respond to this.

@rwaldron rwaldron added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Sep 26, 2017
@hzoo hzoo mentioned this pull request Sep 30, 2017
11 tasks
@littledan
Copy link
Member Author

I think TC39 was happy with this sort of thing ultimately, right?

@littledan
Copy link
Member Author

I believe we got consensus here. Aside from addressing code review feedback on the tests, does anything else need to be fixed up for this patch to land?

@rwaldron
Copy link
Contributor

rwaldron commented May 11, 2018

Spec tests landed: tc39/test262#969

@ljharb ljharb added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels May 12, 2018
@ljharb ljharb requested review from bterlson and bmeck May 12, 2018 00:15
@ljharb ljharb assigned ljharb and unassigned bterlson Jul 18, 2018
@ljharb ljharb force-pushed the fix-function-annex-b branch from d485211 to caf89a1 Compare July 18, 2018 18:50
@ljharb ljharb merged commit caf89a1 into tc39:master Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text web reality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants