LGTM if we have good test coverage.

http://codereview.chromium.org/7832002/diff/1/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/7832002/diff/1/src/heap.cc#newcode2669
src/heap.cc:2669: // The string encoding of the string might be ascii,
while the underlying
Let's just replace this with
"WriteToFlat takes care of the case when an indirect string has a
different encoding from its underlying string. These encodings may
differ because of externalization."

http://codereview.chromium.org/7832002/diff/1/src/heap.cc#newcode2699
src/heap.cc:2699: // The string encoding tag in the newly created slice
does not really matter.
"When slicing an indirect string we use its encoding for a newly created
slice and don't check the encoding of the underlying string. This is
safe even if the encodings are different because of externalization. If
an indirect ASCII string is pointing to a two-byte string, the two-byte
char codes of the underlying string must still fit into ASCII (because
externalization must not change char codes)."

http://codereview.chromium.org/7832002/diff/1/test/cctest/test-strings.cc
File test/cctest/test-strings.cc (right):

http://codereview.chromium.org/7832002/diff/1/test/cctest/test-strings.cc#newcode517
test/cctest/test-strings.cc:517: TEST(SliceFromExternal) {
Do we have a test for the case of the underlying string changing its
encoding from ASCII to two-byte?

http://codereview.chromium.org/7832002/diff/1/test/cctest/test-strings.cc#newcode526
test/cctest/test-strings.cc:526: // After slicing, the original string
becomes a flat cons.
Update the comment.

http://codereview.chromium.org/7832002/

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

Reply via email to