LGTM.

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

http://codereview.chromium.org/21450/diff/1/5#newcode2273
Line 2273: pos->determines_perfectly = false;
Should this be set to false? If the remaining cases of a choice node are
actually determined perfectly, we should match them.
I.e., treat a cannot-match result as if it wasn't there at all.

http://codereview.chromium.org/21450/diff/1/5#newcode2329
Line 2329: // succeed.
This comment should be changed to just "not yet handled".
It would be fairly easy to normalize the character class representation
to be non-negated and treat it like a positive class.

http://codereview.chromium.org/21450/diff/1/5#newcode2677
Line 2677: return quick_check->positions(offset)->determines_perfectly;
Just checking: If we find that a position determines the branch
perfectly, will the position always be tested? Or is it possible that,
e.g., both the first and second character perfectly determines the
branch, and only the first is actually checked?

http://codereview.chromium.org/21450/diff/1/5#newcode2722
Line 2722: int* checked_up_to) {
This function is one big glob of special cases.
If the different passes has something in common, please refactor it to,
e.g., a template function or pass objects with virtual methods or
something else that avoids putting all the cases in one monolithic
function.

http://codereview.chromium.org/21450

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

Reply via email to