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

Issue 5270006: Maintain the invariant that the pattern string in an atom regexp is always ... (Closed)

Created:
10 years ago by sandholm
Modified:
9 years, 7 months ago
Reviewers:
Lasse Reichstein
CC:
v8-dev
Visibility:
Public.

Description

Maintain the invariant that the pattern string in an atom regexp is always a flat non-cons string. Committed: http://code.google.com/p/v8/source/detail?r=5907

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 4

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+50 lines, -20 lines) Patch
M src/handles.cc View 1 1 chunk +1 line, -7 lines 0 comments Download
M src/jsregexp.cc View 1 4 chunks +46 lines, -7 lines 0 comments Download
M src/runtime.cc View 1 2 3 chunks +3 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
sandholm
10 years ago (2010-11-25 16:07:22 UTC) #1
Lasse Reichstein
LGTM http://codereview.chromium.org/5270006/diff/5001/src/jsregexp.cc File src/jsregexp.cc (right): http://codereview.chromium.org/5270006/diff/5001/src/jsregexp.cc#newcode229 src/jsregexp.cc:229: if (!subject->IsFlat()) FlattenString(subject); Could you use FlattenGetString here? ...
10 years ago (2010-11-30 11:34:38 UTC) #2
sandholm
10 years ago (2010-11-30 11:55:24 UTC) #3
http://codereview.chromium.org/5270006/diff/5001/src/jsregexp.cc
File src/jsregexp.cc (right):

http://codereview.chromium.org/5270006/diff/5001/src/jsregexp.cc#newcode229
src/jsregexp.cc:229: if (!subject->IsFlat()) FlattenString(subject);
On 2010/11/30 11:34:38, Lasse Reichstein wrote:
> Could you use FlattenGetString here?

Yes, in fact, I already tried but it is a bit slower.

http://codereview.chromium.org/5270006/diff/5001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/5270006/diff/5001/src/runtime.cc#newcode3195
src/runtime.cc:3195: String::cast(regexp->DataAt(JSRegExp::kAtomPatternIndex)));
On 2010/11/30 11:34:38, Lasse Reichstein wrote:
> Add an ASSERT(pattern->IsFlat()); somewhere, just to be pedantic.

Done.

Powered by Google App Engine
This is Rietveld 408576698