Incorporated review comments, PTAL. Note that I'm currently run tons of test to make sure that we can really use Foo::cast here, but I'm quite sure that this is the case...
http://codereview.chromium.org/9225056/diff/1/src/objects.cc File src/objects.cc (left): http://codereview.chromium.org/9225056/diff/1/src/objects.cc#oldcode7295 src/objects.cc:7295: ASSERT(target->prototype() == this || On 2012/01/30 12:57:56, Michael Starzinger wrote:
Can we keep this assertion around? If we turn RestorePrototype into a
member of
Map, this assertion can be placed in there.
Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#oldcode7312 src/objects.cc:7312: ASSERT(target->prototype() == this || On 2012/01/30 12:57:56, Michael Starzinger wrote:
Likewise.
Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#oldcode7321 src/objects.cc:7321: // If no map was found, make sure the FixedArray also gets collected. On 2012/01/30 12:57:56, Michael Starzinger wrote:
Can we keep this comment (and place it above line 7363 on the right
side)? Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc File src/objects.cc (right): http://codereview.chromium.org/9225056/diff/1/src/objects.cc#newcode7287 src/objects.cc:7287: static bool RestoreProto(Object* object, On 2012/01/30 12:57:56, Michael Starzinger wrote:
I would prefer "RestorePrototype" as a name. Also a comment of what
the
semantics of keep_entry and the return value are, is needed here I
think.
Just an idea: Since this is the diametrical opposite of Map::CreateOneBackPointer, why not make it a member if Map and call it
something
like RestoreOneBackPointer?
Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#newcode7291 src/objects.cc:7291: Map* map = static_cast<Map*>(object); On 2012/01/30 12:57:56, Michael Starzinger wrote:
If object->IsMap() is usable at this point, then Map::cast() is as
well. I would
suggest using that instead of a static_cast<> here.
Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#newcode7303 src/objects.cc:7303: // Live DescriptorArray objects will be marked, so we must use low-level On 2012/01/30 12:57:56, Michael Starzinger wrote:
The comment is no longer accurate.
Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#newcode7305 src/objects.cc:7305: DescriptorArray* d = static_cast<DescriptorArray*>( On 2012/01/30 12:57:56, Michael Starzinger wrote:
I think we can use our internal casts here. Could you please verify
that? Done. http://codereview.chromium.org/9225056/diff/1/src/objects.cc#newcode7310 src/objects.cc:7310: FixedArray* contents = static_cast<FixedArray*>( On 2012/01/30 12:57:56, Michael Starzinger wrote:
Likewise.
Done. http://codereview.chromium.org/9225056/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
