LGTM

http://codereview.chromium.org/7114/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/7114/diff/1/2#newcode959
Line 959: static const int kMaxAsciiChar = 0x7f;
Please use String::kMaxAsciiChar

http://codereview.chromium.org/7114/diff/1/2#newcode962
Line 962: // shift. Only allow the last kBMMaxShift characters of the
needle
allow -> allows

http://codereview.chromium.org/7114/diff/1/2#newcode964
Line 964: class BMGoodSuffixBuffers {
Should inherit from AllStatic

http://codereview.chromium.org/7114/diff/1/2#newcode965
Line 965: private:
Style guide dictates public before private.

http://codereview.chromium.org/7114/diff/1/2#newcode972
Line 972: void init(int needle_length) {
should be inline

http://codereview.chromium.org/7114/diff/1/2#newcode1028
Line 1028: // End of Bad Char computation
Terminate comments with '.'

http://codereview.chromium.org/7114/diff/1/2#newcode1084
Line 1084: // we have matched more than our tables allow us to be smart
about.
It seems like here we could still use the bad char table?

http://codereview.chromium.org/7114/diff/1/2#newcode1121
Line 1121: while (pattern[j] == subject[j+i]) {
This reads one too far in pattern and subject, no?

http://codereview.chromium.org/7114

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

Reply via email to