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

CharacterRange abstract operation tests should be phase: runtime and not phase: early #1193

Closed
jirkamarsik opened this issue Aug 23, 2017 · 5 comments

Comments

@jirkamarsik
Copy link

The Early Errors rule for regular expression literals states that the literals must be parsed using the Pattern goal symbol of the regular expression grammar. This might trigger early SyntaxErrors when the regular expression in the literal is malformed. However, there are some "malformed" regular expressions which still pass the grammar and its early errors conditions, e.g. /[\s-\d]/u, where the bounds of a character range are character sets, not single characters. Such expressions also lead to SyntaxErrors, but only during the evaluation of the regular expression literal, when interpreting the pattern as a RegExpMatcher using the Pattern Semantics.

Therefore, I believe the tests in the following files to be incorrect as of their current state:

The phase of the SyntaxError should be runtime instead of early and therefore they shouldn't be prefixed with the throw statements. The RegExp validation is performed in Runtime Semantics: Character Range and not in Static Semantics.

NB: In ES5.1, SyntaxErrors thrown by the runtime semantics of constructing a RegExp object actually did cause early errors, but this behaviour disappeared from the spec in ES6.

NB2: These tests contradict the test built-ins/RegExp/property-escapes/character-class.js), in which the regular expression literal /[\p{Hex}-\uFFFF]/u is expected to throw a SyntaxError at runtime.

@jirkamarsik
Copy link
Author

The same issue now also manifests in built-ins/RegExp/property-escapes/character-class.js (as of commit e44d737). The abstract operation CharacterRange should throw SyntaxErrors at runtime (during RegExp literal evaluation) and not earlier.

@leobalter
Copy link
Member

Thanks for the raising these issues!

This topic is not really in my best field of experience, I might need some feedback from @anba, @jugglinmike, or @mathiasbynens here.

In any case, PRs are most welcome!

@anba
Copy link
Contributor

anba commented Sep 12, 2017

Spec PR to change RegExps per web-reality, i.e. throw when invalid ranges are present, is up at tc39/ecma262#984

@jirkamarsik
Copy link
Author

@anba, yes, moving the SyntaxErrors out of the runtime of RexExpCreate and putting them directly into the grammar (as in your PR) seems a lot cleaner. Thanks!

@jirkamarsik
Copy link
Author

OK, the spec has been changed upstream, so I am closing this issue.
tc39/ecma262#984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants