[regexp] back refs must not start/end in the middle of a surrogate pair

[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N

Committed: https://crrev.com/49fda47c5fa88cea9ce0a913c023da1d9204ffb1
Cr-Commit-Position: refs/heads/master@{#33540}

[Yang, 2016-01-19 10:36:02]

On 2016/01/19 10:30:57, Yang wrote:

This depends on https://codereview.chromium.org/1578253005/

[Yang, 2016-01-19 11:00:33]

Description was changed from

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N
==========

to

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N
==========

[Yang, 2016-01-19 11:00:34]

yangguo@chromium.org changed reviewers:
+ erik.corry@gmail.com

[Yang, 2016-01-22 07:50:17]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-22 07:50:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1601653006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1601653006/40001

[commit-bot: I haz the power, 2016-01-22 07:50:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-22 07:50:23]

Dry run: CLs for remote refs other than refs/pending/heads/master must contain
NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

[Yang, 2016-01-25 14:13:58]

On 2016/01/22 07:50:23, commit-bot: I haz the power wrote:
> Dry run: CLs for remote refs other than refs/pending/heads/master must contain
> NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

I simplified and extended the test case a little.

[erikcorry, 2016-01-26 16:25:06]

erikcorry@google.com changed reviewers:
+ erikcorry@google.com

[erikcorry, 2016-01-26 16:25:07]

lgtm

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/jsregexp.cc
File src/regexp/jsregexp.cc (right):

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/jsregexp.cc#...
src/regexp/jsregexp.cc:4393: }
You don't need this check (and thus you don't need the surrogate_check variable
either).

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
File src/regexp/regexp-macro-assembler.cc (right):

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
src/regexp/regexp-macro-assembler.cc:30: LoadCurrentCharacter(cp_offset, &ok,
true, 1);
True, 1 are default args and not needed.

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
src/regexp/regexp-macro-assembler.cc:34:
CheckCharacterNotInRange(kLeadSurrogateStart, kLeadSurrogateEnd, &ok);
CheckCharacterInRange(on_failure) ?

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
File test/mjsunit/harmony/unicode-regexp-backrefs.js (right):

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
test/mjsunit/harmony/unicode-regexp-backrefs.js:10: .replace(/l/g, "\ud801")
]You could replace . (non-newline) with [^] (anything) to make the test tougher.

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
test/mjsunit/harmony/unicode-regexp-backrefs.js:12: .replace(/t/g, "\udc01");
You don't use the lower case 't' anywhere.

[Yang, 2016-01-27 09:06:06]

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/jsregexp.cc
File src/regexp/jsregexp.cc (right):

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/jsregexp.cc#...
src/regexp/jsregexp.cc:4393: }
On 2016/01/26 16:25:06, erikcorry wrote:
> You don't need this check (and thus you don't need the surrogate_check
variable
> either).

Done. Thanks.

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
File src/regexp/regexp-macro-assembler.cc (right):

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
src/regexp/regexp-macro-assembler.cc:30: LoadCurrentCharacter(cp_offset, &ok,
true, 1);
On 2016/01/26 16:25:06, erikcorry wrote:
> True, 1 are default args and not needed.

Done.

https://codereview.chromium.org/1601653006/diff/80001/src/regexp/regexp-macro...
src/regexp/regexp-macro-assembler.cc:34:
CheckCharacterNotInRange(kLeadSurrogateStart, kLeadSurrogateEnd, &ok);
On 2016/01/26 16:25:06, erikcorry wrote:
> CheckCharacterInRange(on_failure) ?

Done.

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
File test/mjsunit/harmony/unicode-regexp-backrefs.js (right):

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
test/mjsunit/harmony/unicode-regexp-backrefs.js:10: .replace(/l/g, "\ud801")
On 2016/01/26 16:25:07, erikcorry wrote:
> ]You could replace . (non-newline) with [^] (anything) to make the test
tougher.

Done.

https://codereview.chromium.org/1601653006/diff/80001/test/mjsunit/harmony/un...
test/mjsunit/harmony/unicode-regexp-backrefs.js:12: .replace(/t/g, "\udc01");
On 2016/01/26 16:25:07, erikcorry wrote:
> You don't use the lower case 't' anywhere.

Done.

[Yang, 2016-01-27 09:06:11]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2016-01-27 09:06:12]

The patchset sent to the CQ was uploaded after l-g-t-m from erikcorry@google.com
Link to the patchset: https://codereview.chromium.org/1601653006/#ps100001
(title: "rebase and addressed comments")

[commit-bot: I haz the power, 2016-01-27 09:06:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1601653006/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1601653006/100001

[commit-bot: I haz the power, 2016-01-27 10:50:12]

Description was changed from

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N
==========

to

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N
==========

[commit-bot: I haz the power, 2016-01-27 10:50:13]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-01-27 10:51:35]

Description was changed from

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N
==========

to

==========
[regexp] back refs must not start/end in the middle of a surrogate pair

R=littledan@chromium.org
BUG=v8:2952
LOG=N

Committed: https://crrev.com/49fda47c5fa88cea9ce0a913c023da1d9204ffb1
Cr-Commit-Position: refs/heads/master@{#33540}
==========

[commit-bot: I haz the power, 2016-01-27 10:51:36]

Patchset 6 (id:??) landed as
https://crrev.com/49fda47c5fa88cea9ce0a913c023da1d9204ffb1
Cr-Commit-Position: refs/heads/master@{#33540}