[esnext] implement Object.getOwnPropertyDescriptors() proposal

[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org

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

[caitp (gmail), 2016-02-01 20:40:31]

Description was changed from

==========
[es7] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

to

==========
[es7] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

[caitp (gmail), 2016-02-01 20:41:08]

here's another one

[andrea.giammarchi, 2016-02-02 08:17:28]

On 2016/02/01 20:41:08, caitp wrote:
> here's another one

very nice! I wasn't aware of this one so I've made a PR to the github repo (
which I believe nobody checks :-) )

For record sake: https://github.com/v8/v8/pull/9

Hope this will land soon.

[Camillo Bruni, 2016-02-02 13:40:58]

will look into it tomorrow. just back from being sick so working at 10% brain
capacity ;)

[Jakob Kummerow, 2016-02-02 13:55:49]

DBC: Don't use "auto".

There's one exception: if it avoids a *lot* of boilerplate, e.g. when you can
s/std::map<Foo*, Bar*>::iterator/auto/, then "auto" is OK to use. That's rare in
V8, though. When functions return MaybeHandle<FixedArray> or Maybe<bool> or
similar, the details matter and should be spelled out explicitly.

[caitp (gmail), 2016-02-02 14:21:20]

On 2016/02/02 13:55:49, Jakob wrote:
> DBC: Don't use "auto".
> 
> There's one exception: if it avoids a *lot* of boilerplate, e.g. when you can
> s/std::map<Foo*, Bar*>::iterator/auto/, then "auto" is OK to use. That's rare
in
> V8, though. When functions return MaybeHandle<FixedArray> or Maybe<bool> or
> similar, the details matter and should be spelled out explicitly.

the 80column limit makes `auto` very appealing :p Alright, will make explicit

[rossberg, 2016-02-02 15:12:06]

Meta nit: This function will not be in ES7. Feature freeze was last week. You
might want to adjust the CL title.

[caitp (gmail), 2016-02-02 16:34:38]

Description was changed from

==========
[es7] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

to

==========
[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

[Camillo Bruni, 2016-02-03 10:52:34]

lgtm

[caitp (gmail), 2016-02-04 19:51:10]

The CQ bit was checked by caitpotter88@gmail.com

[caitp (gmail), 2016-02-04 19:51:11]

The patchset sent to the CQ was uploaded after l-g-t-m from cbruni@chromium.org
Link to the patchset: https://codereview.chromium.org/1658773003/#ps70001
(title: "fix rebase + re-enable test")

[commit-bot: I haz the power, 2016-02-04 19:51:16]

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

[commit-bot: I haz the power, 2016-02-04 19:54:24]

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

[commit-bot: I haz the power, 2016-02-04 19:54:25]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/10560)

[adamk, 2016-02-04 20:13:48]

https://codereview.chromium.org/1658773003/diff/70001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1658773003/diff/70001/src/builtins.cc#newcode...
src/builtins.cc:1827: Handle<Object> from_descriptor =
did_get_descriptor.FromJust()
Isn't this guaranteed to return true since we MAYBE_RETURN above?

[caitp (gmail), 2016-02-04 20:14:49]

https://codereview.chromium.org/1658773003/diff/70001/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1658773003/diff/70001/src/builtins.cc#newcode...
src/builtins.cc:1827: Handle<Object> from_descriptor =
did_get_descriptor.FromJust()
On 2016/02/04 20:13:48, adamk wrote:
> Isn't this guaranteed to return true since we MAYBE_RETURN above?

nope, it can return false if the property doesn't exist

[adamk, 2016-02-04 21:32:47]

ah, right, lgtm

[caitp (gmail), 2016-02-04 21:54:32]

On 2016/02/04 21:32:47, adamk wrote:
> ah, right, lgtm

Thanks

[caitp (gmail), 2016-02-04 21:54:37]

The CQ bit was checked by caitpotter88@gmail.com

[commit-bot: I haz the power, 2016-02-04 21:54:42]

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

[commit-bot: I haz the power, 2016-02-04 21:56:12]

Description was changed from

==========
[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

to

==========
[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

[commit-bot: I haz the power, 2016-02-04 21:56:13]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2016-02-04 21:56:32]

Description was changed from

==========
[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org
==========

to

==========
[esnext] implement Object.getOwnPropertyDescriptors() proposal

BUG=v8:4725
LOG=N
R=adamk@chromium.org, cbruni@chromium.org, rossberg@chromium.org

Committed: https://crrev.com/ee10b595f902d04ad1d66ae4231ef20873eb6852
Cr-Commit-Position: refs/heads/master@{#33752}
==========

[commit-bot: I haz the power, 2016-02-04 21:56:32]

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