Introduce a PrototypeIterator class and use it for prototype access

Introduce a PrototypeIterator class and use it for prototype access

The new pattern is that we first get the map of the root of the
prototype chain using Object::GetMapRoot() and then walk up the
prototype chain using Map::prototype().

BUG=???
R=verwaest@chromium.org
LOG=n

Committed: https://code.google.com/p/v8/source/detail?r=22365

[Toon Verwaest, 2014-07-10 20:30:03]

LGTM

[Toon Verwaest, 2014-07-10 20:37:49]

Oh wait.

If START_AT_RECEIVER, Advance will fail if receiver is non-JSReceiver, right?

What about having separate "receiver" and "current", where receiver can be
Object*, but current can only be JSReceiver*? That way you can figure out
whether you needs to get the root by doing current == NULL; Or something of the
sort.

Also, if END_AT_HIDDEN and START_AT_RECEIVER, the first object will not be
hidden. So it'll immediately stop rather than walk the hidden prototype chain.
And again, object isn't guaranteed to not be a smi, so HeapObject::cast isn't
valid.

[jochen (gone - plz use gerrit), 2014-07-11 09:06:01]

On 2014/07/10 at 20:37:49, verwaest wrote:
> Oh wait.
> 
> If START_AT_RECEIVER, Advance will fail if receiver is non-JSReceiver, right?

right. There's now a bool that does the RootMap() thing. Also, it should really
be HeapObject, not JSReceiver.


updated the iterator and changed all GetPrototype(isolate) callsites to use it

PTAL

[Toon Verwaest, 2014-07-11 09:41:28]

Some more comments.

https://codereview.chromium.org/376233002/diff/20001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/376233002/diff/20001/src/builtins.cc#newcode1098
src/builtins.cc:1098: ASSERT(!iter.GetCurrent()->IsJSProxy());
What about returning heap->null_value() if iter.GetCurrent()->IsJSProxy()?

https://codereview.chromium.org/376233002/diff/20001/src/ic.cc
File src/ic.cc (right):

https://codereview.chromium.org/376233002/diff/20001/src/ic.cc#newcode253
src/ic.cc:253: if (iter.GetCurrent()->IsNull()) return false;
IsAtEnd?

https://codereview.chromium.org/376233002/diff/20001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/376233002/diff/20001/src/objects.cc#newcode805
src/objects.cc:805: if (object->IsUndefined()) {
I think this condition should never occur. Reading an element from undefined is
supposed to be a type error. Can you turn this into an ASSERT? If it fails, for
now you can keep it like this, but we should investigate where that undefined is
coming from.

https://codereview.chromium.org/376233002/diff/20001/src/objects.cc#newcode821
src/objects.cc:821: ASSERT(PrototypeIterator::GetCurrent(iter)->IsJSObject());
This condition should be ensured by the iterator I think. The iterator should
ensure that GetCurrent after the receiver is always a JSReceiver (JSProxy +
JSObject). And it should ensure that you never try to look beyond a JSProxy.

https://codereview.chromium.org/376233002/diff/20001/src/objects.cc#newcode6383
src/objects.cc:6383: JSObject::cast(*PrototypeIterator::GetCurrent(iter)),
isolate);
Handle<JSObject> current =
Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter)); ?

https://codereview.chromium.org/376233002/diff/20001/src/prototype.h
File src/prototype.h (right):

https://codereview.chromium.org/376233002/diff/20001/src/prototype.h#newcode73
src/prototype.h:73: } else {
What do we do in case of null_value / JSProxy? It seems like getting the
prototype of null via the map will result in null as well, but what about
proxies?

We should make those 2 cornercases explicit, and either return null_value as a
result, or ASSERT that we don't call Advance on them.

https://codereview.chromium.org/376233002/diff/20001/src/prototype.h#newcode89
src/prototype.h:89: return handle_->IsNull() ||
Should either be true for JSProxy, or we should ASSERT that we don't call this
for JSProxy.

https://codereview.chromium.org/376233002/diff/20001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/376233002/diff/20001/src/runtime.cc#newcode12869
src/runtime.cc:12869: if (result->IsJSGlobalProxy()) {
We should revisit walking beyond the JSGlobalProxy at some point. What if we
detached the global proxy? DetachGlobal does:
SetObjectPrototype(global_proxy, factory->null_value());

[Toon Verwaest, 2014-07-11 14:07:35]

lgtm

[jochen (gone - plz use gerrit), 2014-07-14 07:20:03]

Committed patchset #4 manually as r22365 (presubmit successful).