I had to revert this change. probably a push-pop imbalance. Caused crashes.
On Fri, Jan 23, 2009 at 2:34 PM, <[email protected]> wrote: > Addressed review comments > > > http://codereview.chromium.org/18708/diff/1/3 > File src/jsregexp.cc (right): > > http://codereview.chromium.org/18708/diff/1/3#newcode1385 > Line 1385: const int push_limit = (assembler->stack_limit_slack() + 1) / > 2; > Ack, it lost its comment. > The +1 is to avoid doing modulo with zero if the slack is 1 (one). > > http://codereview.chromium.org/18708/diff/1/3#newcode1397 > Line 1397: enum DeferredActionUndoType { IGNORE, RESTORE, CLEAR } > undo_action = IGNORE; > On 2009/01/23 13:09:21, Erik Corry wrote: > >> I think it's cleaner to create the enum and use it in two different >> > statements. > > Done. > > http://codereview.chromium.org/18708/diff/1/3#newcode1414 > Line 1414: value = psr->value(); > On 2009/01/23 13:09:21, Erik Corry wrote: > >> Here you are overwriting a value with an older one! You should just >> > ignore the > >> value, but use the action for the undo action. >> > > Done. > > http://codereview.chromium.org/18708/diff/1/3#newcode1420 > Line 1420: // counters. They have no significant previous value. > Correct. We will have to propagate the information on whether we are > inside a loop to this point. Until then, we'll have to restore the value > to be safe. > > http://codereview.chromium.org/18708/diff/1/3#newcode1440 > Line 1440: undo_action = CLEAR; > This is also a problem if the STORE_POSITION is not a capture, but for > checking if a loop matches emptily. > If the loop is inside another loop, we need to restore. > > http://codereview.chromium.org/18708/diff/1/3#newcode1467 > Line 1467: (pushes % push_limit == 0) ? > RegExpMacroAssembler::kCheckStackLimit > On 2009/01/23 13:09:21, Erik Corry wrote: > >> I'd prefer just zeroing the pushlimit here. That way you avoid the >> > expensive % > >> operation and I think it would be clearer. >> > > Done. > > > http://codereview.chromium.org/18708 > -- Erik Corry, Software Engineer Google Denmark ApS. CVR nr. 28 86 69 84 c/o Philip & Partners, 7 Vognmagergade, P.O. Box 2227, DK-1018 Copenhagen K, Denmark. --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
