LGTM
http://codereview.chromium.org/13343/diff/201/15 File src/jsregexp.cc (right): http://codereview.chromium.org/13343/diff/201/15#newcode3677 Line 3677: bool has_been_expanded_; Isn't this really two different visitors, rolled into one with a boolean tag? How about making ValidateInfo pure virtual, and have two different subclasses that implement ValidateInfo differently (as each branch of the conditional in the current NodeValidator::ValidateInfo). http://codereview.chromium.org/13343/diff/201/15#newcode3778 Line 3778: node = new TextNode(new RegExpCharacterClass('*'), node); What happens if the look-behind is at the start of the string (there is no character before the start position)? The current implementation preloads the previous character, or "\n" if at the start of the string, so the code-generation can start out with a current-character-test without reading. Will the code-generation be able to see that this text node is the one before the real start? http://codereview.chromium.org/13343/diff/201/19 File test/cctest/test-regexp.cc (right): http://codereview.chromium.org/13343/diff/201/19#newcode1295 Line 1295: Execute("\\bboy\\b", false, true, true); Could you (plural, everybody using this) not move this execution into another file, so we don't get changes to this file that aren't really test related, or meaningfull. I'll write you a Perl script to compile and execute it if necessary. http://codereview.chromium.org/13343 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
