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

Editorial: consolidate each SDO's definition #2271

Merged
merged 44 commits into from
Jan 15, 2021
Merged

Editorial: consolidate each SDO's definition #2271

merged 44 commits into from
Jan 15, 2021

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Jan 10, 2021

Fixes #1950. Fixes #2251.

This ensures every SDO is defined in a single place (modulo tweaks in Annex B, and excepting Early Errors and Evaluation). That way usages of SDOs can be automatically linked to their definition site, as is done for other abstract operations. This makes the spec much easier to navigate, I think.

That was generally just a matter of moving things around and some minor tweaking or renaming. Most SDOs had a natural place to live: ExportEntries can go in the Exports section, say. We introduced a new top-level "Syntax-Directed Operations" clause to hold some more broadly applicable operations.

However, in a few cases @michaelficarra and I judged that the result would have separated sections which logically ought to be grouped, which we resolved by introducing new SDOs. For example, InstantiateFunctionObject has the semantics for setting up all four kinds of function declaration, and we wanted each those semantics to be together with their respective declaration kind. So we introduced InstantiateOrdinaryFunctionObject, InstantiateGeneratorFunctionObject, InstantiateAsyncGeneratorFunctionObject, and InstantiateAsyncFunctionObject, and made the original InstantiateFunctionObject dispatch the appropriate one.

This is marked as a draft because there's still a few cases to do (specifically IsIdentifierRef, NamedEvaluation, and StringValue), as well as some necessary cleanup (for example, the while statement clause is now entirely empty). Edit: ready to go! Needs tc39/ecmarkup#285 upstreamed, though.

This does not yet allow you to trivially answer the question "what SDOs are defined over this production?", which will come as part of tc39/ecmarkup#276.

Commits should be individually reviewable.

@ljharb ljharb force-pushed the tweaks-for-nt-linking branch from 7e0adbd to 241c9c9 Compare January 11, 2021 20:55
Base automatically changed from tweaks-for-nt-linking to master January 11, 2021 21:29
@bakkot bakkot marked this pull request as ready for review January 12, 2021 06:51
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.

We went through this on the editor call, and it seems like a great improvement.

@ljharb ljharb requested review from michaelficarra, syg and a team January 13, 2021 23:04
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.

This is great.

Per the editor call I skipped/skimmed most of the "consolidate" commits as they were script-generated.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@@ -14017,7 +14017,7 @@ <h1>Rules of Automatic Semicolon Insertion</h1>
`throw` [no LineTerminator here] Expression[+In, ?Yield, ?Await] `;`

ArrowFunction[In, Yield, Await] :
ArrowParameters[?Yield, ?Await] [no LineTerminator here] `=&gt;` ConciseBody[?In]
Copy link
Contributor

Choose a reason for hiding this comment

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

We're inconsistent between using =&gt; and =>. Are there any =&gt;s left after this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should always write > as &gt; when not part of an HTML tag.

Copy link
Contributor Author

@bakkot bakkot Jan 14, 2021

Choose a reason for hiding this comment

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

Are there any =&gt;s left after this PR?

One, in OtherPunctuator, which uses &gt; consistently for all punctuators.

I think we should always write > as &gt; when not part of an HTML tag.

Strong disagree. It's less readable for no benefit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @bakkot on not using &gt;. I prefer =>.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@syg
Copy link
Contributor

syg commented Jan 14, 2021

As a followup, the new SDO clauses could use more prose in addition to the clause names briefly saying what SDOs are, what each clause is grouping by at a high-level. See existing prose in the Abstract Operations section, for example.

jmdyck
jmdyck previously requested changes Jan 14, 2021
Copy link
Collaborator

@jmdyck jmdyck left a comment

Choose a reason for hiding this comment

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

I notice that you put type="sdo" only on 'single-clause' SDOs. Wouldn't it also make sense on the remaining multi-clause SDOs?

Or is the 'type' attribute deemed to be an attribute of the aoid, not of the clause?

spec.html Show resolved Hide resolved
<h1>CreateImmutableBinding ( _N_, _S_ )</h1>
<p>The CreateImmutableBinding concrete method of an object Environment Record is never used within this specification.</p>
</emu-clause>
<emu-clause id="sec-static-semantics-declarationpart" type="sdo" aoid="DeclarationPart">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally, when a clause defines an aoid, the id attribute is just "sec-" + lowercase(aoid).

(This is just the first occurrence of many.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm OK with this particular inconsistency, I think.


<emu-clause id="sec-syntax-directed-operations-function-name-inference">
<h1>Function Name Inference</h1>
<emu-clause id="sec-static-semantics-hasname" oldids="sec-semantics-static-semantics-hasname,sec-function-definitions-static-semantics-hasname,sec-arrow-function-definitions-static-semantics-hasname,sec-generator-function-definitions-static-semantics-hasname,sec-async-generator-function-definitions-static-semantics-hasname,sec-class-definitions-static-semantics-hasname,sec-async-function-definitions-static-semantics-HasName,sec-async-arrow-function-definitions-static-semantics-HasName" type="sdo" aoid="HasName">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Insert blank line before <emu-clause>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make a lint rule for this and address in a followup.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay.

spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
spec.html Show resolved Hide resolved
@jmdyck
Copy link
Collaborator

jmdyck commented Jan 14, 2021

Oh, I forgot one: this PR moves up the defining productions for parameter-related nonterminals, so it should probably move the corresponding emu-prodrefs in Annex A.

@bakkot
Copy link
Contributor Author

bakkot commented Jan 14, 2021

Ah, nice catch. Done.

@bakkot
Copy link
Contributor Author

bakkot commented Jan 14, 2021

Wouldn't it also make sense on the remaining multi-clause SDOs?

Copying from IRC for posterity: the remaining such SDOs are Early Errors, Evaluation, MV, TV, TRV, and the evaluation semantics for RegExp patterns. I and ecmarkup are treating these as being unlike normal SDOs for one reason or another, so I'm not adding the type= attribute to them.

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
5 participants