https://codereview.chromium.org/1303033012/diff/1/src/string-search.h
File src/string-search.h (left):
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h#oldcode196
src/string-search.h:196:
nit: keep this line
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h
File src/string-search.h (right):
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h#newcode191
src/string-search.h:191:
nit: two empty lines between top-level things
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h#newcode197
src/string-search.h:197:
nit: two empty lines between top-level things
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h#newcode199
src/string-search.h:199: inline int FindFirstByte(Vector<const
PatternChar> pattern,
Since this works with two-byte strings too, I'd call it
FindFirstCharacter (especially if you fix it to actually find the first
character and not just a nonbinding guess, see below).
https://codereview.chromium.org/1303033012/diff/1/src/string-search.h#newcode214
src/string-search.h:214: pos = AlignDown(pos, sizeof(SubjectChar));
I don't think this is correct. If memchr() found the search_low_byte as
the high byte of one of the characters in the subject, it can't just
AlignDown() and return success. SingleCharSearch() below doesn't care
because it performs the necessary check itself, but the other two
callers would compute bogus results. You have two options:
(1) Keep the behavior of FindFirstByte, and robustify the callers. Since
this behavior is a rather surprising contract, it must be properly
documented in the method name and/or a comment
("MaybeFindFirstCharacter" or somesuch).
(2) Fix the behavior. You'll need to check if the found byte is the low
byte of a character, and if the high byte matches too, and loop (call
memchr() again) if either of those checks fails. I'd prefer this option.
You can then drop line 245 below.
https://codereview.chromium.org/1303033012/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
