-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Normative: Make B.1.{1,2} normative #1867
Conversation
Note that this will require a change to test262 (to remove tests for the non-annex B behavior and move the annex B tests to the main directory). |
bf7b1e2
to
7e3ff42
Compare
(force-pushed to resolve merge conflicts and fix some mistakes) |
Where can I find this text rendered into an html page? (I think someone told me before but I can't find it.) |
If you click "show all checks" at the bottom of any PR, the "Details" link next to the netlify one gives you the rendered HTML. |
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
7e3ff42
to
81800b3
Compare
(force-pushed to resolve merge conflicts) |
This normative change is marked as neither |
@michaelficarra In #1595 we got consensus for moving parts of annex B into the main spec, but there is apparently not agreement about exactly which parts, leaving us in a slightly awkward state. |
Which parts do you think remain unresolved? |
@erights Are you implying that the changes in this PR all have consensus? |
Not necessarily. But I remember that we did once have consensus. From @bakkot 's comment, it sounded like there are particular things that we don't remember what we agreed on, or if we agreed. What are they? I might remember. |
@erights as i recall, we got consensus to move all syntax/parsing items into the main spec (but i don't recall if those were to be marked normative optional, or left required) - but all the other items were to be evaluated case-by-case. |
81800b3
to
6186f0d
Compare
force-pushed to resolve merge conflicts and handle NonOctalDecimalEscapeSequence added in PR #2054. |
6186f0d
to
d44b89b
Compare
Done. |
3d0c24c
to
7a79833
Compare
ba4abee
to
c4f8a8e
Compare
spec.html
Outdated
<ul> | ||
<li>It is a Syntax Error if the source code matching this production is strict mode code.</li> | ||
</ul> | ||
<emu-note>In non-strict code, this syntax is legacy.</emu-note> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<emu-note>In non-strict code, this syntax is legacy.</emu-note> | |
<emu-note>In non-strict code, this syntax is Legacy.</emu-note> |
since i think we want it to auto-link to the dfn added in #2125?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
spec.html
Outdated
<ul> | ||
<li>It is a Syntax Error if the source code matching this production is strict mode code.</li> | ||
</ul> | ||
<emu-note>In non-strict code, this syntax is legacy.</emu-note> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<emu-note>In non-strict code, this syntax is legacy.</emu-note> | |
<emu-note>In non-strict code, this syntax is Legacy.</emu-note> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
c4f8a8e
to
d3139c6
Compare
force-pushed to:
|
@@ -27035,10 +27178,10 @@ <h1>Forbidden Extensions</h1> | |||
The Syntactic Grammar must not be extended in any manner that allows the token `:` to immediately follow source text that matches the |BindingIdentifier| nonterminal symbol. | |||
</li> | |||
<li> | |||
When processing strict mode code, the syntax of |NumericLiteral| must not be extended to include <emu-xref href="#prod-annexB-LegacyOctalIntegerLiteral"></emu-xref> and the syntax of |DecimalIntegerLiteral| must not be extended to include <emu-xref href="#prod-annexB-NonOctalDecimalIntegerLiteral"></emu-xref> as described in <emu-xref href="#sec-additional-syntax-numeric-literals"></emu-xref>. | |||
When processing strict mode code, an implementation must not relax the early error rules of <emu-xref href="#sec-numeric-literals-early-errors"></emu-xref>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's odd that we list the restriction for numeric literals and templates but not strings, here, but I guess that was the status quo as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see issue #2432)
Since this seems to be close to merging, note to @ljharb that I'd like to handle squashing it down. |
@jmdyck I think this is ready, if you want to do the squashing now. |
(Part of Annex B reform, see PR tc39#1595.)
... and replace the former with the latter in TemplateCharacter. The spec prohibits applying the B.1.2 extensions to EscapeSequence when it appears in TemplateCharacter; this change accomplishes that in the grammar rather than in prose. (This makes it a little easier to 'mainline' the B.1.2 extensions in the next commit.)
(Part of Annex B reform, see PR tc39#1595.) B.1.2 makes 2 changes to the EscapeSequence production: (1) It adds the rhs `NonOctalDecimalEscapeSequence`. (2) It replaces the rhs: `0` [lookahead <! DecimalDigit] with: LegacyOctalEscapeSequence where the latter nonterminal generates `0` among lots of other things. Change 1 is straightforward, but change 2 is tricky. In the EscapeSequence production, we can't simply replace the `0` alternative with LegacyOctalEscapeSequence (as B.1.2 does), because the `0` alternative must be treated differently from everything else that LegacyOctalEscapeSequence derives. (The `0` alternative is allowed in contexts where everything else that LegacyOctalEscapeSequence derives is forbidden.) So instead, we redefine LegacyOctalEscapeSequence to exclude the `0` alternative. Specifically, the 'overlap' comes from: LegacyOctalEscapeSequence :: OctalDigit [lookahead ∉ OctalDigit] so we replace that with: LegacyOctalEscapeSequence :: `0` [lookahead ∈ {`8`, `9`}] NonZeroOctalDigit [lookahead ∉ OctalDigit] (See Issue tc39#1975 for more details.) Resolves tc39#1975.
d3139c6
to
f79dfd2
Compare
force-pushed to:
|
(Part of Annex B reform, see PR tc39#1595.)
... and replace the former with the latter in TemplateCharacter. The spec prohibits applying the B.1.2 extensions to EscapeSequence when it appears in TemplateCharacter; this change accomplishes that in the grammar rather than in prose. (This makes it a little easier to 'mainline' the B.1.2 extensions in the next commit.)
(Part of Annex B reform, see PR tc39#1595.) B.1.2 makes 2 changes to the EscapeSequence production: (1) It adds the rhs `NonOctalDecimalEscapeSequence`. (2) It replaces the rhs: `0` [lookahead <! DecimalDigit] with: LegacyOctalEscapeSequence where the latter nonterminal generates `0` among lots of other things. Change 1 is straightforward, but change 2 is tricky. In the EscapeSequence production, we can't simply replace the `0` alternative with LegacyOctalEscapeSequence (as B.1.2 does), because the `0` alternative must be treated differently from everything else that LegacyOctalEscapeSequence derives. (The `0` alternative is allowed in contexts where everything else that LegacyOctalEscapeSequence derives is forbidden.) So instead, we redefine LegacyOctalEscapeSequence to exclude the `0` alternative. Specifically, the 'overlap' comes from: LegacyOctalEscapeSequence :: OctalDigit [lookahead ∉ OctalDigit] so we replace that with: LegacyOctalEscapeSequence :: `0` [lookahead ∈ {`8`, `9`}] NonZeroOctalDigit [lookahead ∉ OctalDigit] (See Issue tc39#1975 for more details.) Resolves tc39#1975.
This PR comprises the first 2 commits of PR #1651, which I gather are "non-controversial".