Editorial: Remove the |Keyword| nonterminal #1694
Conversation
ljharb
requested review from
mathiasbynens,
allenwb,
devsnek,
jmdyck,
ljharb and
a team
|
my understanding of the linked issue was that people generally preferred the current definitions in the spec, based on more formal definitions of "keyword" and "reserved word" from programming language theory. |
Hi @devsnek. I thought the discussion in that issue was pretty confused, so I put together the pull request to show what I mean. Here's a rendered copy:
This is shorter than how it's currently specified. The notes are better organized, and they give a more complete picture of what keywords there are in ES. |
|
@jorendorff we now have PR previews, so here's the link to the rendered version: https://deploy-preview-1694--ecma262-snapshots.netlify.com/#sec-reserved-words (click "show all checks" and then "details") |
|
@ljharb That is some sweet sorcery. Thank you! |
There was a problem hiding this comment.
Several concerns,
- The title of section 11.6 "Names and Keywords" probably should change if we don't talk about keywords in it. But, read on...
- The only real technical role of the ReservedWord lexical productions is enabling the Identifer production in 12.1 to be written as Identifer : IdentiferName but not ReservedWord rather than as additional static semantics rules as is done for the strict mode "future reserved words". The inclusion of await and yield in the ReserveWord set actually caused some confusion for me when reviewing this PR because they are explicitly handled in grammar productions. Then I realized they were included because whether or not they are treated as an Identifer is controlled by grammar productions. I originally came up with that grammar hack, so the fact that I was momentarily confused by this probably means other will also be confused. It would be good to try to make this clearer in the notes. It seems like we have the following categories of IdentifierName used as Identifer rules:
- Those that are always recognized as identifiers
- those that are never recognized by identifiers
- those that are contextually allowed as identifiers (eg, await and yield)
- those that are contextually disallowed as identifiers
- those that are allowed as identifiers but are unambiguously used as contextual keywords.
Or maybe we should keep a Keywords subsection where we explain these differences and normatively include what is currently (in both the current draft and this PR) in the note about not allowing UnicodeEscapeSequences in ReservedWords (and all keywords, including contextual ones).
- I was particularly concerned about this last point, for example when we say
ofin the syntactic grammar we mean exactly those code points without any escapes. This caused me to look at section 5.1.2 which is intended to define the relationship between the lexical and syntactic grammars. In particular how symbols of the lexical grammar are used in the syntactic grammar. I found it to be not so clear about these points and in some cases obsolete (for example, it still thinks that Identifer is defined by the lexical grammar). I think it would be a good idea to expand this PR to include making sure that 5.1 is consistent with 11.6
|
@allenwb Thank you. I read your comment closely, and I'll try to clarify as follows:
A fourth suggestion, "making sure 5.1 is consistent with 11.6", is a good idea but a bit beyond me, at least for today. However, I hope that the work in this PR, while incomplete, is a worthwhile step that doesn't make anything worse. |
I'll look into this possibility too. |
I think in would be useful to (new) implementors to be explicit about those different categories of keywords and which keywords fall into each category. Otherwise, they have to puzzle it out for themselves by looking at the grammar and the static semantics rules. |
This change adopts the classification given in <tc39#1694 (review)>. On consideration, `eval` and `arguments` are not special enough to merit their own bullet point in the list. They are mentioned separately afterwards.
@allenwb Agreed. I updated the NOTE with the five categories you listed; see https://deploy-preview-1694--ecma262-snapshots.netlify.com/#sec-reserved-words. Should I promote that whole note to normative text? I think that would work all right, and I can rename the section to "Keywords and Reserved Words". |
sounds good to me |
|
OK, this incorporates Allen's feedback and is ready for further review. |
|
As far as I know this is ready for review again, so if there's anything I should revise, let me know. |
This change adopts the classification given in <tc39#1694 (review)>. On consideration, `eval` and `arguments` are not special enough to merit their own bullet point in the list. They are mentioned separately afterwards.
ljharb
force-pushed
the
keyword-clarification
branch
from
ea41541 to
179920d
Compare
ljharb
changed the title
Fixes tc39#1678. |Keyword| was not used anywhere else in the spec. This also removes |FutureReservedWord|. The |ReservedWord| nonterminal still matches exactly the same set of words, so the behavior is unchanged. This change allows several related NOTEs to be gathered in one place. A new NOTE acknowledges that `enum` is unused, and explains why it's reserved anyway. Two existing NOTEs about conditional keywords are combined; never-reserved keywords like `from` and `of` are mentioned in the new NOTE, and the term "conditional keyword" is non-normatively defined. - Reword reference to sec-future-reserved-words which no longer exists. - Add oldids to sec-reserved-words for deleted clauses - Clarify "so far unused": the phrase "so far unused" seems to me like it might trip up some readers, so rephrase it less idiomatically and clarify that we're talking about the use of `enum` in the spec, as opposed to its use in scripts. - Rename section "Names and Keywords" to "Names and Reserved Words". - Clarify the NOTE listing categories of identifier names. This change adopts the classification given in tc39#1694 (review) - On consideration, `eval` and `arguments` are not special enough to merit their own bullet point in the list. They are mentioned separately afterwards. - Add a non-normative definition for "keyword", in a NOTE. A Definition is also given for "conditional keyword" / "contextual keyword". - Further clarify the status of `await` and `yield`. - Promote the classification of identifier names to normative text. - Clarify that keywords match verbatim SourceCharacter sequences. - Clarify in a NOTE that even though `els\u{65}` technically isn't a |ReservedWord|, it's effectively reserved anyway. Both of these were already the case, but unobvious. - Clarify the difference between two classes of IdentifierNames.
ljharb
force-pushed
the
keyword-clarification
branch
from
179920d to
712b03b
Compare
Closes #1678.