Just a preliminary glance. What you're doing is okay, but all other
callsites
of Utf8::Encode would have to change and have the separated logic for
rewriting
the surrogates, which is probably not what you want. You may want to keep
the
rewriting in Encode with a new flag for the new type of rewriting.
Additionally, the length computations must be adjust for flag
Also, you should always run test-api/Utf16. Since it's handling a lot of
edge
cases you want to consider.
https://codereview.chromium.org/121173009/diff/1/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/121173009/diff/1/src/api.cc#newcode4519
src/api.cc:4519: // @TODO use uint16_t for previous?
previous is an int because of some special values used in the encoder,
like kNoPreviousCharacter
https://codereview.chromium.org/121173009/diff/1/src/api.cc#newcode4529
src/api.cc:4529: return written + Utf8::Encode(buffer, code_point,
false);
having the length calculation here is too late. String::WriteUtf8 must
compute the identical length, which it can't do if you're potentially
changing it here.
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h
File src/unicode-inl.h (right):
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h#newcode114
src/unicode-inl.h:114: unsigned Utf8::Encode(char* str, uchar c, bool
allow_invalid) {
the name of the third argument is not the same as the declaration.
https://codereview.chromium.org/121173009/diff/1/src/unicode-inl.h#newcode129
src/unicode-inl.h:129: } else if (c <= kMaxThreeByteChar) {
can't chop this section out - need backwards compatibility for the other
call sites of Encode
https://codereview.chromium.org/121173009/diff/1/src/unicode.h
File src/unicode.h (right):
https://codereview.chromium.org/121173009/diff/1/src/unicode.h#newcode53
src/unicode.h:53: const int kReplacementCharacter = 0xFFFD;
should probably be in the Utf8 class
https://codereview.chromium.org/121173009/diff/1/src/unicode.h#newcode158
src/unicode.h:158: static inline unsigned Encode(char* str, uchar c,
bool replace_surrogates);
this is not backwards compatible. All existing call sites which you did
not cut over are casting int to bool for the third argument
https://codereview.chromium.org/121173009/
--
--
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/groups/opt_out.
