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: Expand the "Syntax-Directed Operations" section #3210

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Oct 29, 2023

and establish some conventions for referring to Parse Nodes.


commit 1:

Move the definition of "source text matched by" to 5.1.4 The Syntactic Grammar, because it isn't specific to SDOs. It can be (and is, barely) used by early error rules and abstract operations too.


commit 2:

Split off a subsection for "Implicit Definitions for Chain Productions"

Occasionally, someone points out that the spec is missing some SDO definitions, and we direct them to this paragraph. Having it be a named section may slightly increase the chance that they will see it, and if not, it at least gives us a more specific id to point them to.


commit 3:

Split off a subsection for "Invoking Syntax-Directed Operations"


commit 4:

Add a paragraph about the dispatching mechanism for SDOs.
(The shape of the Parse Node determines which algorithm is executed.)

Also, capitalize "Parse Node".


commit 5:

Create a subsection for "Referring to Parse Nodes"


commit 6:

Early error rules use these phrases too.


commit 7 etc:

Change node-references to conform to the conventions.

@@ -900,13 +900,14 @@ <h1>Syntax-Directed Operations</h1>

<emu-clause id="sec-invoking-syntax-directed-operations">
<h1>Invoking Syntax-Directed Operations</h1>
<p>Syntax-directed operations are invoked with a parse node and, optionally, other parameters by using the conventions on steps <emu-xref href="#step-sdo-invocation-example-1"></emu-xref>, <emu-xref href="#step-sdo-invocation-example-2"></emu-xref>, and <emu-xref href="#step-sdo-invocation-example-3"></emu-xref> in the following algorithm:</p>
<p>Syntax-directed operations are invoked with a Parse Node and, optionally, other parameters by using the conventions on steps <emu-xref href="#step-sdo-invocation-example-1"></emu-xref>, <emu-xref href="#step-sdo-invocation-example-2"></emu-xref>, and <emu-xref href="#step-sdo-invocation-example-3"></emu-xref> in the following algorithm:</p>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<p>Syntax-directed operations are invoked with a Parse Node and, optionally, other parameters by using the conventions on steps <emu-xref href="#step-sdo-invocation-example-1"></emu-xref>, <emu-xref href="#step-sdo-invocation-example-2"></emu-xref>, and <emu-xref href="#step-sdo-invocation-example-3"></emu-xref> in the following algorithm:</p>
<p>Syntax-directed operations are invoked with a subject Parse Node and, optionally, other parameters by using the conventions on steps <emu-xref href="#step-sdo-invocation-example-1"></emu-xref>, <emu-xref href="#step-sdo-invocation-example-2"></emu-xref>, and <emu-xref href="#step-sdo-invocation-example-3"></emu-xref> in the following algorithm:</p>

to align with the use of the phrase "subject Parse Node" below

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 agree with the intent, but I'm worried that a reader might think that "a subject Parse Node" was a reference to a term defined elsewhere. We could put 'subject' in italics or quotes, to convey that this is its defining occurrence (sort of), but I think it'd be clearer to say something like:

invoked with a Parse Node (referred to below as 'the subject Parse Node')

Also, the sentence was getting a bit complicated, so I broke it into two.

By the way, I'm not sure that "subject" is the best word. In my code, I've also used "focus" and "target".

<ul>
<li>If each associated production has exactly one right-hand occurrence of the nonterminal, then the algorithm can refer to it with “the” or without a prefix (e.g., “the |StatementList|” or “|StatementList|”). If the same nonterminal also appears on the LHS of the production, the algorithm can also use “the derived” (e.g., “the derived |UnaryExpression|”) to emphasize a reference to the right-hand nonterminal.</li>
<li>If a production has multiple right-hand occurrences of the nonterminal, they are distinguished by ordinals (e.g., “the first |DecimalDigits|” or “the fourth |HexDigit|”).</li>
<li>If a production has an optional nonterminal (e.g., “|StatementList?|”), the algorithm must check if the corresponding Parse Node is present before using it (e.g., “If the |StatementList| is present, return the VarDeclaredNames of |StatementList|.”)</li>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<li>If a production has an optional nonterminal (e.g., “|StatementList?|”), the algorithm must check if the corresponding Parse Node is present before using it (e.g., “If the |StatementList| is present, return the VarDeclaredNames of |StatementList|.”)</li>
<li>If a production has an optional nonterminal (e.g., “|StatementList?|”), the algorithm must check if the corresponding Parse Node is present before using it (e.g., “If |StatementList| is present, return VarDeclaredNames of |StatementList|.”)</li>

The first "the" should be avoided. The second "the" we're really inconsistent about, but I'd prefer to not encourage it with the example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The first "the" should be avoided.

I'm pretty sure that hasn't been established. (If it has, please link.)

I'm inclined to the opposite opinion: eliding "the" before a nonterminal should be avoided.

Alternatively, we could use a scheme of superscripts such as you suggested in issue #1768. The editors might want to decide on that before this PR goes ahead, to avoid churn.


The second "the" we're really inconsistent about, but I'd prefer to not encourage it with the example.

Okay, done.

Would you like a PR that removes "the" from before the SDO-name in invocations? There's about 400 occurrences.

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure that hasn't been established. (If it has, please link.)

It hasn't.

Alternatively, we could use a scheme of superscripts such as you suggested in issue #1768. The editors might want to decide on that before this PR goes ahead, to avoid churn.

Nah, even if we eventually went with that eventually, I'd prefer to first do this intermediate step.

Would you like a PR that removes "the" from before the SDO-name in invocations? There's about 400 occurrences.

If you don't mind, yes. That would help us a lot in discussing the change.

<li>If all of the associated productions have the same left-hand side nonterminal, then that nonterminal is used with “this” (e.g., “this |ExportDeclaration|”).</li>
<li>Otherwise, “this Parse Node” is used.</li>
</ul>
<p>To refer to the children of the subject Parse Node: In practice, algorithms only reference children that are instances of nonterminals.</p>
Copy link
Member

Choose a reason for hiding this comment

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

I'd swap these.

Suggested change
<p>To refer to the children of the subject Parse Node: In practice, algorithms only reference children that are instances of nonterminals.</p>
<p>In practice, algorithms only reference children that are instances of nonterminals. To refer to the children of the subject Parse Node:</p>

spec.html Outdated
Comment on lines 916 to 920
<p>To refer to the subject Parse Node:</p>
<ul>
<li>If all of the associated productions have the same left-hand side nonterminal, then that nonterminal is used with “this” (e.g., “this |ExportDeclaration|”).</li>
<li>Otherwise, “this Parse Node” is used.</li>
</ul>
Copy link
Member

Choose a reason for hiding this comment

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

The way this is written sounds like editorial guidance, which is unnecessary in the spec. The goal here is just to list notation that they may run across. Can you reword to simplify it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does kind of sound like editorial guidance, but it's really more a description of editorial variations.

I could reword it, but I'm doubtful that would simplify it. I'll think about it.

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

LGTM otherwise. Some nice improvements here.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Mar 21, 2024

(force-pushed to resolve merge conflicts)

Copy link
Member

@michaelficarra michaelficarra 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 join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants