+VItalyr who did a lot of string work. First round of comments.
http://codereview.chromium.org/7477045/diff/1/src/heap.cc File src/heap.cc (right): http://codereview.chromium.org/7477045/diff/1/src/heap.cc#newcode2687 src/heap.cc:2687: WriteBarrierMode mode = sliced_string->GetWriteBarrierMode(no_gc); sliced_string should be allocated in new space, so do we need a write barrier at all? http://codereview.chromium.org/7477045/diff/1/src/heap.cc#newcode2692 src/heap.cc:2692: sliced_string->set_parent(cons->first(), mode); why first? what if I slice in the second part? http://codereview.chromium.org/7477045/diff/1/src/objects-inl.h File src/objects-inl.h (right): http://codereview.chromium.org/7477045/diff/1/src/objects-inl.h#newcode190 src/objects-inl.h:190: return (type & (kIsNotStringMask | kStringRepresentationMask)) == probably is worth refactoring. http://codereview.chromium.org/7477045/diff/1/src/objects-inl.h#newcode2269 src/objects-inl.h:2269: WRITE_FIELD(this, kParentOffset, parent); maybe add an assert that parent is never a sliced string/external string. http://codereview.chromium.org/7477045/diff/1/src/objects.h File src/objects.h (right): http://codereview.chromium.org/7477045/diff/1/src/objects.h#newcode494 src/objects.h:494: const uint32_t kIsIndirectStringTag = 0x1; maybe worth adding some documentation why it works. And would probably added a helper function right here which checks if type is indirect. http://codereview.chromium.org/7477045/diff/1/test/mjsunit/string-slices.js File test/mjsunit/string-slices.js (right): http://codereview.chromium.org/7477045/diff/1/test/mjsunit/string-slices.js#newcode155 test/mjsunit/string-slices.js:155: //Test charAt for different strings. nit: missing space before Test. http://codereview.chromium.org/7477045/diff/1/test/mjsunit/string-slices.js#newcode159 test/mjsunit/string-slices.js:159: for ( var i = 0; i < 50; i++) { nit: unnecessary space after ( (here and below) http://codereview.chromium.org/7477045/ -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev
