Does it lint. LGTM.
http://codereview.chromium.org/11231/diff/1/4 File src/jsregexp.cc (right): http://codereview.chromium.org/11231/diff/1/4#newcode211 Line 211: pattern->Flatten(shape); Since this shape is only used once I think you should just make it in the Flatten call. This also avoids the subtlety around the fact that flattening a string changes its shape, so the shape can't be reused again anyway. http://codereview.chromium.org/11231/diff/1/6 File src/objects.cc (right): http://codereview.chromium.org/11231/diff/1/6#newcode3504 Line 3504: FlatStringReader* FlatStringReader::top_ = NULL; Some people don't like static initializers. Perhaps this should be in a function called from v8::Initialize? http://codereview.chromium.org/11231/diff/1/7 File src/objects.h (right): http://codereview.chromium.org/11231/diff/1/7#newcode3595 Line 3595: explicit FlatStringReader(Handle<String> str); FlatStringReaders can only be stack allocated, right? So it should be marked as such. Also if it is used with some kinds of handles there could be issues around the lifetime of the handle vs lifetime of the FlatStringReader. That deserves a comment. http://codereview.chromium.org/11231 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
