LGTM, with nitpicks.
http://codereview.chromium.org/18497/diff/1/2 File src/ast.cc (right): http://codereview.chromium.org/18497/diff/1/2#newcode264 Line 264: } It's probably useless in practice, but a capture or a positive look-ahead with an anchored body is also anchored. http://codereview.chromium.org/18497/diff/1/2#newcode439 Line 439: : alternatives_(alternatives) { Could you add an "ASSERT(alternatives->length() > 1);" here too. http://codereview.chromium.org/18497/diff/1/2#newcode453 Line 453: ASSERT(nodes->length() > 0); Asserting that it's > 1 would be even better. The parser avoids creating an alternative if there is only one node in it (ditto for disjunctions). http://codereview.chromium.org/18497/diff/1/5 File test/mjsunit/ascii-regexp-subject.js (right): http://codereview.chromium.org/18497/diff/1/5#newcode31 Line 31: Is this "test" falsifiable? Should it timeout if we aren't optimized? If we just test that "^" works, we should have some asserts, and no need for a loop. It looks more like a benchmark than a test. I suggest dropping it or creating a benchmark. http://codereview.chromium.org/18497 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
