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

Early errors for invalid regular expressions #1050

Closed
TimothyGu opened this Issue Dec 20, 2017 · 7 comments

Comments

Projects
None yet
5 participants
@TimothyGu
Member

TimothyGu commented Dec 20, 2017

As it currently stands, the spec categorizes most invalid regular expression literals that don't interfere with parsing (e.g. /(/) as runtime errors. (See grammar productions for RegularExpressionLiteral; regular expression parsing errors only occur during Evaluation of such literals, through RegExpCreate and RegExpInitialize steps 7 and 8.) However, it seems that many implementations treat them as early errors, as discovered in acornjs/acorn#640.

Specifically, the following script parses in V8, but not in SpiderMonkey or JavaScriptCore. I am not sure how it performs on ChakraCore.

if (false) (/(/);

Alternatively, this could be used as a test:

new Function('return /(/');
@RReverser

This comment has been minimized.

Show comment
Hide comment
@RReverser

RReverser Dec 20, 2017

Contributor

Alternatively, this could be used as a test:

Or () => /(/

Contributor

RReverser commented Dec 20, 2017

Alternatively, this could be used as a test:

Or () => /(/

@RReverser

This comment has been minimized.

Show comment
Hide comment
@RReverser

RReverser Dec 20, 2017

Contributor

Tested with Chakra on MacOS, it also treats this as an early parsing error. So it's only V8 that delays it to the runtime.

Contributor

RReverser commented Dec 20, 2017

Tested with Chakra on MacOS, it also treats this as an early parsing error. So it's only V8 that delays it to the runtime.

@mathiasbynens

This comment has been minimized.

Show comment
Hide comment
@mathiasbynens

This comment has been minimized.

Show comment
Hide comment
@mathiasbynens

mathiasbynens Dec 20, 2017

Member

@TimothyGu

Specifically, the following script parses in V8, but not in SpiderMonkey or JavaScriptCore. I am not sure how it performs on ChakraCore.

$ eshost -e '(() => { if (false) /(/ })()'
#### Chakra
SyntaxError: Expected ')' in regular expression

#### JavaScriptCore
SyntaxError: Invalid regular expression: missing )

#### SpiderMonkey
SyntaxError: unterminated parenthetical:

#### V8 --harmony
undefined

#### V8
undefined

As a side comment, eshost-cli + jsvu make it really easy to test across all engines.

Member

mathiasbynens commented Dec 20, 2017

@TimothyGu

Specifically, the following script parses in V8, but not in SpiderMonkey or JavaScriptCore. I am not sure how it performs on ChakraCore.

$ eshost -e '(() => { if (false) /(/ })()'
#### Chakra
SyntaxError: Expected ')' in regular expression

#### JavaScriptCore
SyntaxError: Invalid regular expression: missing )

#### SpiderMonkey
SyntaxError: unterminated parenthetical:

#### V8 --harmony
undefined

#### V8
undefined

As a side comment, eshost-cli + jsvu make it really easy to test across all engines.

@jmdyck

This comment has been minimized.

Show comment
Hide comment
@jmdyck

jmdyck Dec 20, 2017

Collaborator

As it currently stands, the spec categorizes most invalid regular expression literals that don't interfere with parsing (e.g. /(/) as runtime errors ...
regular expression parsing errors only occur during Evaluation of such literals

What about the following early error?

It is a Syntax Error if BodyText of RegularExpressionLiteral
cannot be recognized using the goal symbol Pattern
of the ECMAScript RegExp grammar specified in 21.2.1. 
Collaborator

jmdyck commented Dec 20, 2017

As it currently stands, the spec categorizes most invalid regular expression literals that don't interfere with parsing (e.g. /(/) as runtime errors ...
regular expression parsing errors only occur during Evaluation of such literals

What about the following early error?

It is a Syntax Error if BodyText of RegularExpressionLiteral
cannot be recognized using the goal symbol Pattern
of the ECMAScript RegExp grammar specified in 21.2.1. 
@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Dec 20, 2017

Member

Agreed with @jmdyck , this is already specified an early error. V8 has a bug open for this spec violation.

Member

littledan commented Dec 20, 2017

Agreed with @jmdyck , this is already specified an early error. V8 has a bug open for this spec violation.

@littledan littledan closed this Dec 20, 2017

@TimothyGu

This comment has been minimized.

Show comment
Hide comment
@TimothyGu

TimothyGu Dec 20, 2017

Member

@jmdyck Oops, I overlooked that one.

@littledan Thanks for confirming.

Member

TimothyGu commented Dec 20, 2017

@jmdyck Oops, I overlooked that one.

@littledan Thanks for confirming.

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