Thanks for the feedback.

http://codereview.chromium.org/9038/diff/1/8
File src/objects.cc (right):

http://codereview.chromium.org/9038/diff/1/8#newcode2915
Line 2915: return true;
On 2008/11/03 08:45:49, Mads Ager wrote:
> Doesn't it make sense to check that the representation tag makes
sense?

It makes no sense because all bit combinations are valid.

http://codereview.chromium.org/9038/diff/1/13
File src/objects.h (right):

http://codereview.chromium.org/9038/diff/1/13#newcode3026
Line 3026: // string can potentially alter its shape.
On 2008/11/03 08:45:49, Mads Ager wrote:
> Would it make sense to put in some debug code that adds a valid_
member to the
> StringShape class and sets it to false when flattened?  Then each of
the
> operations can assert that the shape is valid which could potentially
catch
> nasty bugs.

At the moment the flatten operation doesn't actually change the shape,
though this is in the works :-)  I put in the scaffolding for the check,
but at the moment it's not used.

http://codereview.chromium.org/9038/diff/1/12
File src/runtime.cc (right):

http://codereview.chromium.org/9038/diff/1/12#newcode2187
Line 2187: StringShape dshape(destination);
On 2008/11/03 08:45:49, Mads Ager wrote:
> dest_shape or even destination_shape?

I changed the other one, but I left this in, because the 80 character
limit means that this code gets very much less readable with dest_shape.

http://codereview.chromium.org/9038

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

Reply via email to