LGTM, with questions. I will need an explanation :)

http://codereview.chromium.org/9638/diff/1/2
File src/ast.h (right):

http://codereview.chromium.org/9638/diff/1/2#newcode1216
Line 1216: virtual RegExpNode* ToNode(RegExpCompiler* compiler,
Could this be a Visitor instead?

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

http://codereview.chromium.org/9638/diff/1/4#newcode1057
Line 1057: //
Do you test that x didn't match the empty string while t <= r < f ?
That would seem to require an extra register to store the position
before x and test for progress after it.
We also need to clear captures in x between loops.

http://codereview.chromium.org/9638/diff/1/4#newcode1061
Line 1061: int reg = needs_counter ? compiler->AllocateRegister() : -1;
The case where min = 0 and max = 1 (i.e., the '?' quantifier) doesn't
need a loop and could be generated as a simple choice node.
Also, if x is known to be simple and max is low, it might be more
efficient to unroll the loop. But that's an optimization for later.

http://codereview.chromium.org/9638/diff/1/4#newcode1121
Line 1121: RegExpNode* proceed = ActionNode::EndSubmatch(on_success);
What does the Submatch concept cover?
Why isn't a positive lookahead the same as any other expression to
match, except that the on_success goes to a node that restores the
position before continuing?
(I.e., am I missing a complexity here?)

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

http://codereview.chromium.org/9638/diff/1/5#newcode323
Line 323: };
What!?! No third out-parameter? ;)
Good call.

http://codereview.chromium.org/9638

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

Reply via email to