Lgtm, with comments.

The test case wasn't uploaded for some reason so I can't see it.



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

http://codereview.chromium.org/28184/diff/1/10#newcode257
Line 257: static void EnsureSize(Handle<JSArray> lastMatchInfo,
You might consider making this a method on JSArray or JSObject even --
it looks generally useful.  In fact, I know one other place that does
the exact same thing, NeanderArray::add in api.cc.

http://codereview.chromium.org/28184/diff/1/10#newcode276
Line 276: Smi* index,
Consider converting the index to a C right away -- we usually do that
eagerly.  Also, what happens when using an integer > 2^30?

http://codereview.chromium.org/28184/diff/1/10#newcode338
Line 338: SetLastCaptureCount(*array, 2);
Could this sequence of 'Set's be packed into a function?  It looks like
you're doing the exact same thing 30 lines below.

http://codereview.chromium.org/28184/diff/1/10#newcode351
Line 351: CHECK(lastMatchInfo->HasFastElements());
Don't you mean ASSERT?

http://codereview.chromium.org/28184/diff/1/10#newcode365
Line 365: Handle<FixedArray> array(lastMatchInfo->elements());
You're creating an unbounded number of handles here.  You should insert
a handle scope.

http://codereview.chromium.org/28184/diff/1/10#newcode592
Line 592: Handle<FixedArray>
matches_array(JSArray::cast(*matches)->elements());
Again with the unbounded number of handles.

http://codereview.chromium.org/28184/diff/1/4
File src/macros.py (right):

http://codereview.chromium.org/28184/diff/1/4#newcode113
Line 113: macro LAST_SUBJECT(array) = ((array)[(array)[0] + 1]);
Beware: this evaluates its argument twice.

http://codereview.chromium.org/28184/diff/1/6
File src/runtime.cc (right):

http://codereview.chromium.org/28184/diff/1/6#newcode866
Line 866: CONVERT_CHECKED(Smi, index, args[2]);
This looks suspect -- plain small integers sometimes end up in heap
numbers.  Maybe CONVERT_NUMBER_CHECKED is what you're looking for.

http://codereview.chromium.org/28184

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to