Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

V8 implementation: does unicode property escapes behavior in character classes range differ from other classes intentionally? #29

Closed
vsemozhetbyt opened this issue Jul 8, 2017 · 9 comments

Comments

@vsemozhetbyt
Copy link

V8 5.8 — 6.1

> 'a-'.match(/[\s-\w]/gu)
SyntaxError: Invalid regular expression: /[\s-\w]/: Invalid character class

> 'a-'.match(/[\p{Script=Latin}-\p{Script=Cyrillic}]/gu)
[ 'a', '-' ]

Is this by design or should it be posted as a bug?

@mathiasbynens
Copy link
Member

mathiasbynens commented Jul 8, 2017

Good catch. On first glance, it seems like this should throw — so this would be a bug.

@mathiasbynens
Copy link
Member

cc @hashseed @schuay

@hashseed
Copy link

hashseed commented Jul 8, 2017

Probably a bug in the regexp parser. Will take a look soon.

@mathiasbynens
Copy link
Member

This V8 bug is now fixed: https://chromium.googlesource.com/v8/v8.git/+/7924985f9fce4ca6ff8b132f55131bd3e28ed849%5E%21

@vsemozhetbyt
Copy link
Author

Thank you!

@mathiasbynens
Copy link
Member

The spec already handles this case in step 1 of the CharacterRange abstract operation.

If A does not contain exactly one character or B does not contain exactly one character, throw a SyntaxError exception.

However, it seems like this doesn’t cover property escapes that expand to only a single character (although I don’t think those exist). Should the proposal explicitly throw for those too?

@mathiasbynens mathiasbynens reopened this Jul 27, 2017
@hashseed
Copy link

I think the behavior should not differ depending on what exactly a property class contains.

@littledan
Copy link
Member

littledan commented Jul 27, 2017

I agree; the current spec is good.

EDIT: Sorry, I was misreading. I think it's important that whether a Unicode property escape contains multiple code points or not should not affect the grammar, as this may change over versions.

@mathiasbynens
Copy link
Member

With tc39/ecma262#984 merged, this is now fixed upstream.

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

No branches or pull requests

4 participants