Adding template parameter to PrototypeIterator GetCurrent for casting

Adding template parameter to PrototypeIterator GetCurrent

BUG=

Committed: https://crrev.com/aacaafd00b313d53ba6efb8caede5d0c188b7afd
Cr-Commit-Position: refs/heads/master@{#30676}

[Camillo Bruni, 2015-09-09 13:56:01]

cbruni@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Camillo Bruni, 2015-09-09 13:56:01]

PTAL

[Jakob Kummerow, 2015-09-09 14:29:55]

lgtm

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc#newcode7354
src/hydrogen.cc:7354: holder = iter.GetCurrent<JSObject>();
I think you need to keep the '*' -- when a PrototypeIterator was constructed for
a Handle, it'll return Handles.

https://codereview.chromium.org/1330153003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1330153003/diff/1/src/objects.cc#newcode5078
src/objects.cc:5078: DCHECK(iter.GetCurrent()->IsJSGlobalObject());
Forgot to update this (and a bunch more DCHECKs below)?

https://codereview.chromium.org/1330153003/diff/1/src/prototype.h
File src/prototype.h (right):

https://codereview.chromium.org/1330153003/diff/1/src/prototype.h#newcode72
src/prototype.h:72: static Handle<Object> GetCurrent(const PrototypeIterator&
iterator) {
Why do we still need this?

[Yang, 2015-09-09 15:15:19]

yangguo@chromium.org changed reviewers:
+ yangguo@chromium.org

[Yang, 2015-09-09 15:15:19]

debug part lgtm

[Camillo Bruni, 2015-09-09 17:24:07]

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc#newcode7354
src/hydrogen.cc:7354: holder = iter.GetCurrent<JSObject>();
On 2015/09/09 at 14:29:55, Jakob wrote:
> I think you need to keep the '*' -- when a PrototypeIterator was constructed
for a Handle, it'll return Handles.

PrototypeIterator::GetCurrent and ->/.GetCurrent are not the same thing
(contrary to what the name suggests), the first one returns a Handle while the
second one returns the raw Object* (quite confusing... should probably change
the name to GetRaw)

https://codereview.chromium.org/1330153003/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1330153003/diff/1/src/objects.cc#newcode5078
src/objects.cc:5078: DCHECK(iter.GetCurrent()->IsJSGlobalObject());
On 2015/09/09 at 14:29:55, Jakob wrote:
> Forgot to update this (and a bunch more DCHECKs below)?

I added a default to the template param, so it will work just as before
(returning Handle<Object> or Object*)

[Camillo Bruni, 2015-09-10 08:43:37]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-10 08:43:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330153003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330153003/1

[commit-bot: I haz the power, 2015-09-10 08:44:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-10 08:44:33]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/9494)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)

[Camillo Bruni, 2015-09-10 09:00:25]

The CQ bit was checked by cbruni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-10 09:00:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330153003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330153003/20001

[commit-bot: I haz the power, 2015-09-10 09:27:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-10 09:27:10]

Dry run: This issue passed the CQ dry run.

[Camillo Bruni, 2015-09-10 10:52:04]

The CQ bit was checked by cbruni@chromium.org

[Camillo Bruni, 2015-09-10 10:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
yangguo@chromium.org, jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1330153003/#ps20001
(title: "simplifications")

[commit-bot: I haz the power, 2015-09-10 10:52:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330153003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330153003/20001

[Camillo Bruni, 2015-09-10 10:52:49]

The CQ bit was unchecked by cbruni@chromium.org

[Camillo Bruni, 2015-09-10 10:53:59]

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/1330153003/diff/1/src/hydrogen.cc#newcode7354
src/hydrogen.cc:7354: holder = iter.GetCurrent<JSObject>();
On 2015/09/09 at 17:24:07, cbruni wrote:
> On 2015/09/09 at 14:29:55, Jakob wrote:
> > I think you need to keep the '*' -- when a PrototypeIterator was constructed
for a Handle, it'll return Handles.
> 
> PrototypeIterator::GetCurrent and ->/.GetCurrent are not the same thing
(contrary to what the name suggests), the first one returns a Handle while the
second one returns the raw Object* (quite confusing... should probably change
the name to GetRaw)

reverted

https://codereview.chromium.org/1330153003/diff/1/src/prototype.h
File src/prototype.h (right):

https://codereview.chromium.org/1330153003/diff/1/src/prototype.h#newcode72
src/prototype.h:72: static Handle<Object> GetCurrent(const PrototypeIterator&
iterator) {
On 2015/09/09 at 14:29:55, Jakob wrote:
> Why do we still need this?

removed

[Camillo Bruni, 2015-09-10 10:54:42]

The CQ bit was checked by cbruni@chromium.org

[commit-bot: I haz the power, 2015-09-10 10:54:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1330153003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1330153003/20001

[commit-bot: I haz the power, 2015-09-10 10:56:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-10 10:56:29]

Patchset 2 (id:??) landed as
https://crrev.com/aacaafd00b313d53ba6efb8caede5d0c188b7afd
Cr-Commit-Position: refs/heads/master@{#30676}