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

Wording RegExp DecimalEscape is unnecessarily complicated #379

Closed
hashseed opened this Issue Feb 10, 2016 · 0 comments

Comments

Projects
None yet
2 participants
@hashseed

hashseed commented Feb 10, 2016

In [0] for DecimalEscape, its evaluation is described: it can be either the NUL character or an integer.
In [1] for AtomEscape, the usage of that value in an atom is described: if it's a character, it describes that character. If it's an integer, it describes a back reference.
In [2] for ClassEscape, the usage of that value in a character class is described: if it's a character, it describes that character. If it's an integer, a syntax error is thrown.

It seems to me that the production for \0 and is unnecessarily entwined. In unicode mode, the former exclusively describes the character NUL and the latter a back reference (if matching capturing group exists), since capturing groups are 1-indexed. In non-unicode mode, both could also fall back to be a legacy octal literal, for which a separate syntax pattern already exists.

If we factor out \0 as NullEscape, we could

  • add it alongside DecimalEscape in the production of AtomEscape.
  • remove DecimalEscape in the production of ClassEscape.
  • remove step 2 in [1].
  • remove step 2 in [2].

It seems to me that this would make things a lot easier to understand, while preserving the way things work.

[0] http://tc39.github.io/ecma262/#sec-decimalescape
[1] http://tc39.github.io/ecma262/#sec-atomescape
[2] http://tc39.github.io/ecma262/#sec-classescape

anba added a commit to anba/ecma262 that referenced this issue Feb 23, 2016

@anba anba referenced this issue Feb 23, 2016

Closed

RegExp fixes #403

@bterlson bterlson closed this in ce5b503 Apr 15, 2016

bterlson added a commit that referenced this issue Apr 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment