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

Update to align with stage 3 conditions #38

Merged
merged 5 commits into from
Feb 26, 2021
Merged

Update to align with stage 3 conditions #38

merged 5 commits into from
Feb 26, 2021

Conversation

rbuckton
Copy link
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
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.

spec/index.html Outdated Show resolved Hide resolved
Co-authored-by: Michael Ficarra <github@michael.ficarra.me>
Copy link
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.

spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-class-definitions-patch.html Show resolved Hide resolved
spec/sec-class-definitions-patch.html Outdated Show resolved Hide resolved
spec/sec-ecmascript-specification-types-patch.html Outdated Show resolved Hide resolved
Copy link
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
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
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
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
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
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
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
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