Issue 6171001: Change interpretation of malformed \c? escapes in RegExp to match JSC.

Issue 6171001: Change interpretation of malformed \c? escapes in RegExp to match JSC. (Closed)

Created:
9 years, 11 months ago by Lasse Reichstein

Modified:
9 years, 6 months ago

Reviewers:
Erik Corry

CC:
v8-dev

Visibility:
Public.

Description

Change interpretation of malformed \c? escapes in RegExp to match JSC.

Patch Set 2 : Fix lint problem #

Total comments: 8

Patch Set 3 : Added one more test. #

Patch Set 4 : Addressed review comments. #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+136 lines, -37 lines)			Patch
M	src/parser.h	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	src/parser.cc	View	1 2 3	3 chunks	+32 lines, -23 lines	0 comments	Download
M	src/scanner-base.cc	View	1 2 3	1 chunk	+9 lines, -2 lines	0 comments	Download
M	test/cctest/test-parsing.cc	View	1 2 3	1 chunk	+55 lines, -0 lines	0 comments	Download
M	test/cctest/test-regexp.cc	View	1 2 3	2 chunks	+16 lines, -4 lines	0 comments	Download
M	test/mjsunit/regexp.js	View	1 2 3	2 chunks	+24 lines, -7 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Erik Corry

LGTM Florian should be careful when pushing to Chromium. http://codereview.chromium.org/6171001/diff/2001/src/parser.cc File src/parser.cc (right): http://codereview.chromium.org/6171001/diff/2001/src/parser.cc#newcode4037 src/parser.cc:4037: ...

9 years, 11 months ago (2011-01-07 10:38:15 UTC) #2

Lasse Reichstein

9 years, 11 months ago (2011-01-07 12:34:09 UTC) #3

http://codereview.chromium.org/6171001/diff/2001/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/6171001/diff/2001/src/parser.cc#newcode4037
src/parser.cc:4037: }
On 2011/01/07 10:38:16, Erik Corry wrote:
> I think it's more readable with an else and a single break.

Done.

http://codereview.chromium.org/6171001/diff/2001/src/parser.cc#newcode4382
src/parser.cc:4382: // For compatability with JSC, inside a character class
On 2011/01/07 10:38:16, Erik Corry wrote:
> ata -> ati

Done.

http://codereview.chromium.org/6171001/diff/2001/test/cctest/test-regexp.cc
File test/cctest/test-regexp.cc (right):

http://codereview.chromium.org/6171001/diff/2001/test/cctest/test-regexp.cc#n...
test/cctest/test-regexp.cc:237: CHECK_PARSE_EQ("\\c", "'\\c'");
Added to cctest/test-parsing.cc

http://codereview.chromium.org/6171001/diff/2001/test/mjsunit/third_party/reg...
File test/mjsunit/third_party/regexp-pcre.js (right):

http://codereview.chromium.org/6171001/diff/2001/test/mjsunit/third_party/reg...
test/mjsunit/third_party/regexp-pcre.js:4232: // assertEquals(null,
res[835].exec("a+ Z0+\x08\n\x1d\x12", 2585));
Whaddayaknow, it didn't need to be disabled at all.

Expand Messages | Collapse Messages