LGTM.
http://codereview.chromium.org/875001/diff/1/2 File src/runtime.cc (right): http://codereview.chromium.org/875001/diff/1/2#newcode2087 src/runtime.cc:2087: struct StringMatchState { Consider making this a class and moving BoyerMoore variants initialization here to methods like SwitchToBoyerMoore*(pattern, start). http://codereview.chromium.org/875001/diff/1/2#newcode2089 src/runtime.cc:2089: StringMatchState(int pattern_length) explicit http://codereview.chromium.org/875001/diff/1/2#newcode2199 src/runtime.cc:2199: int badness; It might be less fragile to use state->badness directly below. http://codereview.chromium.org/875001/diff/1/2#newcode4256 src/runtime.cc:4256: Too many empty lines. http://codereview.chromium.org/875001/diff/1/2#newcode4264 src/runtime.cc:4264: unsigned int max_split) { unsigned int is dangerous. Can we just use int here? http://codereview.chromium.org/875001/diff/1/2#newcode4280 src/runtime.cc:4280: void inline FindStringIndices(Vector<const schar> subject, FindCharIndices? http://codereview.chromium.org/875001/diff/1/2#newcode4283 src/runtime.cc:4283: unsigned int max_split) { See above. http://codereview.chromium.org/875001/diff/1/2#newcode4302 src/runtime.cc:4302: CONVERT_NUMBER_CHECKED(uint32_t, max_split, Uint32, args[2]); Why not use the name from spec and from string.js ("limit")? http://codereview.chromium.org/875001/diff/1/2#newcode4314 src/runtime.cc:4314: int initial_capacity = Can this be simplified to Min<uint32_t>(kMaxInitialListCapacity, max_split)? http://codereview.chromium.org/875001/diff/1/2#newcode4375 src/runtime.cc:4375: HandleScope localLoopHandle; noCamelCasePlease. http://codereview.chromium.org/875001/diff/1/2#newcode4379 src/runtime.cc:4379: elements->set(i, *substring, UPDATE_WRITE_BARRIER); UPDATE_WRITE_BARRIER is the default. http://codereview.chromium.org/875001 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
