https://codereview.chromium.org/788043005/diff/220001/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/788043005/diff/220001/src/parser.cc#newcode4354 src/parser.cc:4354: } Should `-` be a “syntax character” as well ...
6 years, 2 months ago
(2015-01-08 12:29:07 UTC)
#4
https://codereview.chromium.org/788043005/diff/220001/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/788043005/diff/220001/src/parser.cc#newcode4354 src/parser.cc:4354: } On 2015/01/08 13:42:18, marja wrote: > On 2015/01/08 ...
6 years, 2 months ago
(2015-01-08 14:11:46 UTC)
#7
https://codereview.chromium.org/788043005/diff/220001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/788043005/diff/220001/src/parser.cc#newcode4354
src/parser.cc:4354: }
On 2015/01/08 13:42:18, marja wrote:
> On 2015/01/08 12:29:07, mathias wrote:
> > Should `-` be a “syntax character” as well because of its special meaning
> within
> > character classes (e.g. `/[a-b]/`)?
>
> The spec (draft rev 30) says:
>
> SyntaxCharacter :: one of
> ^ $ \ . * + ? ( ) [ ] { } |
>
> - loses its special meaning if it's the first or the last character in the
> character class, so [a-b-] matches a, b, and -.
>
> But idk, hard to say if it's an omission in the spec or intentional. I filed a
> bug to ask that: https://bugs.ecmascript.org/show_bug.cgi?id=3519
>
> If they update the spec, I'll update the implementation.
It's not a SyntaxCharacter and that's likely intentional: it has no special
status outside a character class, and inside you can put it first or last, so
you don't need to escape it in either case.
https://codereview.chromium.org/788043005/diff/220001/test/mjsunit/harmony/un...
File test/mjsunit/harmony/unicode-escapes-in-regexps.js (right):
https://codereview.chromium.org/788043005/diff/220001/test/mjsunit/harmony/un...
test/mjsunit/harmony/unicode-escapes-in-regexps.js:145: new
RegExp("\\u{110000}")
On 2015/01/08 13:42:19, marja wrote:
> On 2015/01/08 12:47:45, rossberg wrote:
> > Perhaps add the case
> >
> > assertThrows{"new RegExp('\\\\u{1f}')", SyntaxError)
>
> That doesn't throw even w/ the current implementation.
>
> Looks like the thing inside { } doesn't need to be a number, if it's not, it's
> just not treated as a count specifier.
Hm, that looks like a spec deviation then, as I don't see anything in the
grammar allowing it. But that's clearly a separate issue then.
mathias
LGTM. https://codereview.chromium.org/788043005/diff/190011/src/regexp.js File src/regexp.js (right): https://codereview.chromium.org/788043005/diff/190011/src/regexp.js#newcode241 src/regexp.js:241: if (harmony_unicode && this.unicode) result += 'u'; Let’s ...
6 years, 2 months ago
(2015-01-08 17:12:56 UTC)
#8
since the sticky flag was added to the regexp mirror, i'm adding the unicode flag ...
6 years, 1 month ago
(2015-01-09 12:12:30 UTC)
#10
since the sticky flag was added to the regexp mirror, i'm adding the unicode
flag too in the latest-1 patch set.
mathias
On 2015/01/09 12:12:30, marja wrote: > since the sticky flag was added to the regexp ...
6 years, 1 month ago
(2015-01-09 12:14:52 UTC)
#11
On 2015/01/09 12:12:30, marja wrote:
> since the sticky flag was added to the regexp mirror, i'm adding the unicode
> flag too in the latest-1 patch set.
Let’s update test/mjsunit/mirror-regexp.js too then?
\u{1F44D} Thanks for your amazing work on this, Marja! \u{1F44D}
Updated the test, though, it's not very exciting (just testing properties, and also passes without ...
6 years, 1 month ago
(2015-01-09 16:37:54 UTC)
#13
Updated the test, though, it's not very exciting (just testing properties, and
also passes without the corresponding code change - what I gathered from
yangguo@ is that the mirror regexps don't really affect anything for realz).
yangguo, could you review the mirror changes (impl + test)?
marja
Oh, and, mathias, thx, also, this is very cool: https://mathiasbynens.be/notes/es6-unicode-regex \u{263A}
6 years, 1 month ago
(2015-01-09 17:15:19 UTC)
#14
Issue 788043005: ES6 unicode escapes, part 2: Regexps.
(Closed)
Created 6 years, 2 months ago by marja
Modified 6 years, 1 month ago
Reviewers: rossberg, mathias, yangguo, Erik Corry Chromium.org
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Comments: 18