LGTM

http://codereview.chromium.org/3139005/diff/8001/9002
File src/objects-inl.h (right):

http://codereview.chromium.org/3139005/diff/8001/9002#newcode462
src/objects-inl.h:462: return Object::IsHeapObject()
nit: "&&" should stay on this line.

http://codereview.chromium.org/3139005/diff/8001/9002#newcode470
src/objects-inl.h:470: && HeapObject::cast(this)->map() ==
Ditto.

http://codereview.chromium.org/3139005/diff/8001/9002#newcode554
src/objects-inl.h:554: && HeapObject::cast(this)->map() ==
Ditto.

http://codereview.chromium.org/3139005/diff/8001/9003
File src/objects.cc (right):

http://codereview.chromium.org/3139005/diff/8001/9003#newcode99
src/objects.cc:99: if (IsTrue()) return
HeapObject::cast(this)->GetHeap()->true_value();
These two first lines are a bit strange now. If Is{True,False} accepted
some broad class of objects (not just {true,false}_value) e.g. 42 is
true and 0 is false, then HeapObject::cast would be dangerous. But given
that they only accept {true,false}_value we can return "this" instead.

http://codereview.chromium.org/3139005/diff/8001/9003#newcode2885
src/objects.cc:2885: || HEAP->isolate()->MayNamedAccess(this, name,
v8::ACCESS_SET));
I don't think it's any better than Isolate::Current().

http://codereview.chromium.org/3139005/diff/8001/9003#newcode8291
src/objects.cc:8291: Heap* heap = Isolate::Current()->heap();
Dictionary is a FixedArray. GetHeap() should work here.

http://codereview.chromium.org/3139005/diff/8001/9003#newcode8384
src/objects.cc:8384: Heap* heap = Isolate::Current()->heap();
Ditto.

http://codereview.chromium.org/3139005/diff/8001/9003#newcode8594
src/objects.cc:8594: Heap* heap = Isolate::Current()->heap();
Ditto.

http://codereview.chromium.org/3139005/show

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

Reply via email to