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

Issue 13618: * Delayed compilation of irregexps until use-time, and specialize on char type. (Closed)

Created:
12 years ago by Lasse Reichstein
Modified:
9 years, 6 months ago
Reviewers:
Erik Corry
CC:
v8-dev
Visibility:
Public.

Description

Irregexp is specialized on subject character type.

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+444 lines, -298 lines) Patch
M src/jsregexp.h View 5 chunks +13 lines, -9 lines 0 comments Download
M src/jsregexp.cc View 20 chunks +417 lines, -279 lines 4 comments Download
M src/objects.h View 1 chunk +1 line, -1 line 0 comments Download
M src/regexp-macro-assembler-ia32.cc View 3 chunks +7 lines, -4 lines 1 comment Download
M test/cctest/test-regexp.cc View 4 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Lasse Reichstein
Medium sized review, please.
12 years ago (2008-12-08 12:26:31 UTC) #1
Erik Corry
12 years ago (2008-12-08 12:47:51 UTC) #2
LGTM with comments.

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

http://codereview.chromium.org/13618/diff/1/2#newcode315
Line 315: // on JSCRE. We rejoice at the though of the day when this is
spolling.

http://codereview.chromium.org/13618/diff/1/2#newcode346
Line 346: // on JSCRE. We rejoice at the though of the day when this is
Speling

http://codereview.chromium.org/13618/diff/1/2#newcode914
Line 914: int char_size = is_ascii ? sizeof(char) : sizeof(uc16);  // NOLINT
It's part of the definition of the language that sizeof(char) == 1.  Also I'd
prefer a char_shift of 0 or 1.

http://codereview.chromium.org/13618/diff/1/2#newcode935
Line 935: SeqTwoByteString::kHeaderSize - kHeapObjectTag;
SeqAsciiString and SeqTwoByteString have methods for this which should be used.

http://codereview.chromium.org/13618/diff/1/5
File src/regexp-macro-assembler-ia32.cc (right):

http://codereview.chromium.org/13618/diff/1/5#newcode687
Line 687: // This function MUST NOT cause a garbage collection. A GC might move
Instead of shouting RFC-style I would prefer 'is not allowed to'

Powered by Google App Engine
This is Rietveld 408576698