Skip to content
This repository was archived by the owner on Sep 8, 2021. It is now read-only.

Update to align with stage 3 conditions#38

Merged
rbuckton merged 5 commits intomasterfrom
stage3
Feb 26, 2021
Merged

Update to align with stage 3 conditions#38
rbuckton merged 5 commits intomasterfrom
stage3

Conversation

@rbuckton
Copy link
Copy Markdown
Collaborator

During the January TC39 meeting, the following changes are required to reach consensus for Stage 3:

In addition, I've updated the specification text to be a delta from the Static Class Features proposal.

In addition the following are required:

  • Additional review by the ECMA-262 editors
  • Additional review by @littledan

@rbuckton
Copy link
Copy Markdown
Collaborator Author

@littledan: You requested that I investigate leveraging Block rather than a custom parse node for ClassStaticBlock (i.e., `static` Block). I'm not sure this is feasible, as function bodies require additional semantics that aren't supported by the Static and Runtime semantics of Block, otherwise we would also just be reusing Block for productions like FunctionDeclaration, et. al.

I have, however, leveraged FunctionCreate and MakeMethod, and removed the custom class static block environment record in an effort to simplify the specification text.

Co-authored-by: Michael Ficarra <github@michael.ficarra.me>
Copy link
Copy Markdown
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, I think all my comments thus far are editorial.

Copy link
Copy Markdown
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending shu's comments

rbuckton and others added 2 commits February 19, 2021 16:54
Co-authored-by: Shu-yu Guo <shu@rfrn.org>
@rbuckton
Copy link
Copy Markdown
Collaborator Author

@syg: I've made updates based on your suggestions. Please let me know if there's anything else you see that I should address.

@littledan: When you have some time, please provide your feedback.

Copy link
Copy Markdown
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is perfect--it encodes the semantics we agreed on, and removes the duplication that I was most concerned about. Thanks for the update, and apologies for my delay.

spec/biblio.json Outdated
@@ -0,0 +1,5 @@
{
"https://tc39.es/ecma262/": [
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should link to the diff for now, so it is not a dead link?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing it to https://arai-a.github.io/ecma262-compare/?pr=1668 didn't fix the link. As soon as I open the diff it ignores the #sec-class-definitions-static-semantics-containsarguments added by ecmarkup.

Using https://arai-a.github.io/ecma262-compare/history/PR/1668/94b1af23d6ca075c6635bcd5b96c6450ec580f51 seems to work, though, but will get out of date if new commits are added.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can kind of hack around it by using this biblio entry:

{
    "https://arai-a.github.io/ecma262-compare/?pr=1668&id=sec-class-definitions-static-semantics-containsarguments": [
        {"type": "clause", "id": "sec-class-definitions-static-semantics-containsarguments", ... }
    ]
}

Since that diff tool uses id=.

@rbuckton
Copy link
Copy Markdown
Collaborator Author

I updated the biblio entry above, and also generated a fresh ecma262biblio.json (would be nice if this were automatic) and fixed some algorithm references where the names have changed (FunctionCreate->OrdinaryFunctionCreate).

Copy link
Copy Markdown
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants