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: bug re Annex B production for AtomEscape #1672

Closed
jmdyck opened this issue Aug 18, 2019 · 3 comments
Closed

Normative: bug re Annex B production for AtomEscape #1672

jmdyck opened this issue Aug 18, 2019 · 3 comments
Assignees
Labels

Comments

@jmdyck
Copy link
Collaborator

jmdyck commented Aug 18, 2019

The main-body definition of AtomEscape says (in part):

AtomEscape[U, N] :: CharacterEscape[?U]

but the Annex-B definition of AtomEscape says:

AtomEscape[U, N] :: CharacterEscape[~U, ?N]

So if AtomEscape is 'invoked' with +U, main-body uses CharacterEscape[+U] whereas Annex-B uses CharacterEscape[~U].

This is presumably a bug, since Annex B claims that it doesn't change any [+U] syntax.


It looks like the bug was introduced in 251dcef (for PR #403), where (using the old notation for grammatical parameters)

AtomEscape[U] ::
  [+U] CharacterEscape[U]
  [+U] CharacterClassEscape
  [~U] CharacterClassEscape
  [~U] CharacterEscape

was changed to:

AtomEscape[U] ::
  CharacterClassEscape
  CharacterEscape

but should have been changed to:

AtomEscape[U] ::
  CharacterClassEscape
  CharacterEscape[?U]

(Compare to the same commit's parallel change to the ClassEscape production, where CharacterEscape does get the [?U].)

@anba, please confirm.

@mathiasbynens
Copy link
Member

@jmdyck's reasoning makes sense to me. I'd love to hear from @anba in case I'm missing something.

@bakkot
Copy link
Contributor

bakkot commented Nov 29, 2020

I agree with @jmdyck. Having CharacterEscape parsed with ~U here would mean /\z/u would be legal (NB z is not a valid escape), as derived from CharacterEscape[U] :: IdentityEscape[?U] -> IdentityEscape[U] :: [~U] SourceCharacterIdentityEscape. That definitely is not the intent, and indeed all major engines reject /\z/u.

ljharb pushed a commit to jmdyck/ecma262 that referenced this issue Dec 2, 2020
Specifically, change:
    AtomEscape[U, N] :: CharacterEscape[~U, ?N]
to:
    AtomEscape[U, N] :: CharacterEscape[?U, ?N]

This fixes a bug that PR tc39#403 introduced way back in 2016.

Resolves issue tc39#1672 (which see for some discussion).
@bakkot
Copy link
Contributor

bakkot commented Dec 2, 2020

Fixed by #2235.

@bakkot bakkot closed this as completed Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants