Addressed Michaels comments.
- Renamed elements to elements_transition.
- Never return NULL for elements_transition and transitions(), but always
let
the client check if the wanted value is present using HasElementsTransition
or
HasTransitionArray.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/mark-compact.cc
File src/mark-compact.cc (right):
https://chromiumcodereview.appspot.com/10697015/diff/1/src/mark-compact.cc#newcode1910
src/mark-compact.cc:1910:
base_marker()->MarkObjectAndPush(reinterpret_cast<HeapObject*>(key));
On 2012/06/28 16:02:12, Michael Starzinger wrote:
We can also use HeapObject::cast() here.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/mark-compact.cc#newcode1965
src/mark-compact.cc:1965:
base_marker()->MarkObjectAndPush(reinterpret_cast<HeapObject*>(key));
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Use HeapObject::cast() here.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/mark-compact.cc#newcode1978
src/mark-compact.cc:1978: base_marker()->MarkObjectWithoutPush(value);
On 2012/06/28 16:02:12, Michael Starzinger wrote:
We only need to mark the slots if the pair itself wasn't marked. So
move the
MarkObjectWithoutPush() into the condition.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-debug.cc
File src/objects-debug.cc (right):
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-debug.cc#newcode921
src/objects-debug.cc:921: if (!IsTransitionArray()) return true;
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Can this really happen? If not, drop this line.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-debug.cc#newcode948
src/objects-debug.cc:948: if (this == NULL) return true;
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Can "this" really be NULL? If not, drop this line.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-debug.cc#newcode956
src/objects-debug.cc:956: continue;
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Drop the continue.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-debug.cc#newcode958
src/objects-debug.cc:958: if (!CheckOneBackPointer(current_map, value))
return false;
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Can we turn this into a proper "else" branch and also assert that it
actually is
a map in the else branch?
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-inl.h
File src/objects-inl.h (right):
https://chromiumcodereview.appspot.com/10697015/diff/1/src/objects-inl.h#newcode2001
src/objects-inl.h:2001: if (!this->MayContainTransitions()) return NULL;
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Somehow it feels wrong to return NULL here. Either return some empty
transitions
array (e.g. EmptyFixedArray) or make the return type of this method
"Object" and
return Smi::FromInt(0) here.
Done.
https://chromiumcodereview.appspot.com/10697015/diff/1/src/transitions.h
File src/transitions.h (right):
https://chromiumcodereview.appspot.com/10697015/diff/1/src/transitions.h#newcode49
src/transitions.h:49: inline Map* elements();
On 2012/06/28 16:02:12, Michael Starzinger wrote:
Can we call this elements_transition instead?
Done.
https://chromiumcodereview.appspot.com/10697015/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
