Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(189)

Issue 8732: Character classes (Closed)

Created:
12 years, 1 month ago by Christian Plesner Hansen
Modified:
9 years, 7 months ago
Reviewers:
Lasse Reichstein
CC:
v8-dev
Visibility:
Public.

Description

Added most of an implementation of compact character classes, for use in the regexp compiler.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+585 lines, -4 lines) Patch
M src/jsregexp.h View 1 chunk +111 lines, -0 lines 2 comments Download
M src/jsregexp.cc View 2 chunks +214 lines, -1 line 2 comments Download
A src/jsregexp-inl.h View 1 chunk +90 lines, -0 lines 0 comments Download
M src/utils.h View 2 chunks +13 lines, -0 lines 0 comments Download
M test/cctest/test-regexp.cc View 2 chunks +157 lines, -3 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Christian Plesner Hansen
Here's my work so far on character classes. Bit-orama!
12 years, 1 month ago (2008-10-30 08:36:19 UTC) #1
Lasse Reichstein
12 years, 1 month ago (2008-10-30 10:28:42 UTC) #2
LGTM

http://codereview.chromium.org/8732/diff/1/3
File src/jsregexp.cc (right):

http://codereview.chromium.org/8732/diff/1/3#newcode1019
Line 1019: left->InitializeFieldFrom(boundaries.SubVector(0, i));
Try to read some more first, and see if more ranges fit into the same field.

http://codereview.chromium.org/8732/diff/1/3#newcode1023
Line 1023: *left = *this;
As discussed, the segments won't necessarily be disjoint if the next range are
in the same segment as the previous one.

http://codereview.chromium.org/8732/diff/1/4
File src/jsregexp.h (right):

http://codereview.chromium.org/8732/diff/1/4#newcode193
Line 193: static const int kFieldSegmentWidth = kCharSize -
kFieldSegmentIndexWidth;
Isn't this the field width, not the segment width?
(It's used to compute the kFieldMax).

http://codereview.chromium.org/8732/diff/1/4#newcode216
Line 216: } u_union;
Slightly confusing name to have for a struct (especially inside a union). Can't
think of more appropriate one, though.

Powered by Google App Engine
This is Rietveld 408576698