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

Normative: Make B.1.{1,2} normative #1867

Merged
merged 3 commits into from
Aug 15, 2021

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Feb 9, 2020

This PR comprises the first 2 commits of PR #1651, which I gather are "non-controversial".

@ljharb ljharb added the normative change Affects behavior required to correctly evaluate some ECMAScript source text label Feb 9, 2020
@ljharb ljharb requested review from michaelficarra, syg, ljharb, bakkot and a team February 9, 2020 16:30
@bakkot
Copy link
Contributor

bakkot commented Feb 10, 2020

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).

@jmdyck jmdyck force-pushed the make_B_literals_normative branch 2 times, most recently from bf7b1e2 to 7e3ff42 Compare February 24, 2020 02:26
@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 24, 2020

(force-pushed to resolve merge conflicts and fix some mistakes)

@littledan
Copy link
Member

cc @waldemarhorwat

@erights
Copy link

erights commented Feb 24, 2020

Where can I find this text rendered into an html page?

(I think someone told me before but I can't find it.)

@evilpie
Copy link
Contributor

evilpie commented Feb 24, 2020

@bakkot
Copy link
Contributor

bakkot commented Feb 24, 2020

Where can I find this text rendered into an html page?

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.

@erights
Copy link

erights commented Feb 24, 2020

Thanks!

Copy link

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jmdyck
Copy link
Collaborator Author

jmdyck commented Apr 25, 2020

(force-pushed to resolve merge conflicts)

@michaelficarra
Copy link
Member

This normative change is marked as neither has consensus nor needs consensus. What gives?

@bakkot
Copy link
Contributor

bakkot commented Oct 8, 2020

@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.

@erights
Copy link

erights commented Oct 8, 2020

@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?

@michaelficarra
Copy link
Member

@erights Are you implying that the changes in this PR all have consensus?

@erights
Copy link

erights commented Oct 8, 2020

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.

@ljharb
Copy link
Member

ljharb commented Oct 8, 2020

@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.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Oct 17, 2020

force-pushed to resolve merge conflicts and handle NonOctalDecimalEscapeSequence added in PR #2054.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jun 10, 2021

Can you open an issue so we can track it and address it with a normative PR once we have consensus to do so?

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>
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
<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?

Copy link
Collaborator Author

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>
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
<emu-note>In non-strict code, this syntax is legacy.</emu-note>
<emu-note>In non-strict code, this syntax is Legacy.</emu-note>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@jmdyck jmdyck force-pushed the make_B_literals_normative branch from c4f8a8e to d3139c6 Compare July 24, 2021 01:27
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jul 24, 2021

force-pushed to:

  • rebase to master, resolving conflicts from PR 2469;
  • add squashable commits to change "legacy" to "Legacy"

@@ -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>.
Copy link
Contributor

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(see issue #2432)

@bakkot bakkot added the editor call to be discussed in the next editor call label Jul 28, 2021
@jmdyck
Copy link
Collaborator Author

jmdyck commented Jul 29, 2021

Since this seems to be close to merging, note to @ljharb that I'd like to handle squashing it down.

@bakkot
Copy link
Contributor

bakkot commented Aug 11, 2021

@jmdyck I think this is ready, if you want to do the squashing now.

... 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 &lt;! 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 &notin; OctalDigit]

so we replace that with:

    LegacyOctalEscapeSequence ::
        `0` [lookahead &isin; {`8`, `9`}]
        NonZeroOctalDigit [lookahead &notin; OctalDigit]

(See Issue tc39#1975 for more details.)
Resolves tc39#1975.
@jmdyck
Copy link
Collaborator Author

jmdyck commented Aug 12, 2021

force-pushed to:

  • rebase to master
  • squash it down to 3 commits
  • polish the commit messages

@michaelficarra michaelficarra added ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed editor call to be discussed in the next editor call labels Aug 12, 2021
@ljharb ljharb merged commit f79dfd2 into tc39:master Aug 15, 2021
@jmdyck jmdyck deleted the make_B_literals_normative branch August 16, 2021 00:15
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
... 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.)
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
(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 &lt;! 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 &notin; OctalDigit]

so we replace that with:

    LegacyOctalEscapeSequence ::
        `0` [lookahead &isin; {`8`, `9`}]
        NonZeroOctalDigit [lookahead &notin; OctalDigit]

(See Issue tc39#1975 for more details.)
Resolves tc39#1975.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants