Some comments
- A lot of the changes from kMaxAsciiCode to kMaxOneByteCode are not gated
by
ENABLE_LATIN_1. Are we sure this doesn't break things?
- Many comments and variable names still refer to ASCII, I assume CL(s) are
coming to fix this?
- I'm not sure moving to Latin1 is a good idea for FilterASCII, I put an
inline
comment for this issue. I suggest keeping the ASCII semantics, based on
kAsciiDataHintTag.
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc
File src/jsregexp.cc (right):
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc#newcode2010
src/jsregexp.cc:2010: if (*border - 1 > String::kMaxOneByteCharCode &&
// ASCII case.
Change comment.
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc#newcode2201
src/jsregexp.cc:2201: bool ascii,
Changing the parameter name would make sense.
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc#newcode2512
src/jsregexp.cc:2512: bool QuickCheckDetails::Rationalize(bool asc) {
Ditto (parameter name).
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc#newcode2567
src/jsregexp.cc:2567: if (compiler->ascii()) {
Maybe onebyte() instead of ascii()?
https://codereview.chromium.org/11759008/diff/5001/src/jsregexp.cc#newcode2868
src/jsregexp.cc:2868: if (quarks[j] > String::kMaxOneByteCharCode) {
Does the comment still hold true for the Latin1 char set? Apparently ÿ
(\u00ff) is part of Latin1, but its uppercase Ÿ is is \u0178...
Maybe we want to keep FilterASCII the way it is, and use it in
combination with kAsciiDataHintTag.
https://codereview.chromium.org/11759008/diff/5001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/11759008/diff/5001/src/objects.h#newcode7069
src/objects.h:7069: bool IsAscii() { return state_ == ASCII; }
I assume this is planned to be renamed to IsOneByte?
https://codereview.chromium.org/11759008/diff/5001/src/runtime.cc
File src/runtime.cc (right):
https://codereview.chromium.org/11759008/diff/5001/src/runtime.cc#newcode5937
src/runtime.cc:5937: #ifndef ENABLE_LATIN_1
I assume that this part will be used for strings with kAsciiDataHintTag,
in a later CL?
https://codereview.chromium.org/11759008/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
