[es6] Partially implement Reflect.defineProperty.

[es6] Partially implement Reflect.defineProperty.

Proxies are not properly supported yet.

R=rossberg,jkummerow@chromium.org
BUG=v8:3931
LOG=n

Committed: https://crrev.com/8e35d8ebeca2bc832ca06874f7a1dce78868010b
Cr-Commit-Position: refs/heads/master@{#31661}

[neis, 2015-10-23 08:32:02]

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

[commit-bot: I haz the power, 2015-10-23 08:32:16]

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

[commit-bot: I haz the power, 2015-10-23 08:56:30]

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

[commit-bot: I haz the power, 2015-10-23 08:56:31]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2015-10-23 12:03:43]

LGTM with comments.

https://codereview.chromium.org/1421033002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1421033002/diff/1/src/builtins.cc#newcode1455
src/builtins.cc:1455: 
Suggestion: add verbatim spec steps as comments, e.g.:

// 1. If Type(target) is not Object, throw a TypeError exception.

to make it easy to see why the implementation does what it does.

https://codereview.chromium.org/1421033002/diff/1/src/builtins.cc#newcode1465
src/builtins.cc:1465: Object::ToName(isolate, key));
Using ToPropertyKey (currently a static function in objects.cc, but we can move
it) would be more efficient for index keys as it avoids needless
int->string->int conversions.

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

https://codereview.chromium.org/1421033002/diff/1/src/objects.cc#newcode6007
src/objects.cc:6007: if (!object->IsJSObject()) return true;
nit: wouldn't "return false" be more appropriate? (I don't care much, it's
temporary code and behind a flag anyway.)

https://codereview.chromium.org/1421033002/diff/1/test/mjsunit/harmony/reflec...
File test/mjsunit/harmony/reflect-define-property.js (right):

https://codereview.chromium.org/1421033002/diff/1/test/mjsunit/harmony/reflec...
test/mjsunit/harmony/reflect-define-property.js:118: TypeError);
nit: indentation (4 spaces for broken lines, or even better column-align with
previous arguments); again several times below.

[neis, 2015-10-29 15:17:25]

https://codereview.chromium.org/1421033002/diff/1/src/builtins.cc
File src/builtins.cc (right):

https://codereview.chromium.org/1421033002/diff/1/src/builtins.cc#newcode1465
src/builtins.cc:1465: Object::ToName(isolate, key));
On 2015/10/23 12:03:42, Jakob wrote:
> Using ToPropertyKey (currently a static function in objects.cc, but we can
move
> it) would be more efficient for index keys as it avoids needless
> int->string->int conversions.

Thanks, good point.  I'll do that in a separate CL since it affects many Reflect
functions.

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

https://codereview.chromium.org/1421033002/diff/1/src/objects.cc#newcode6007
src/objects.cc:6007: if (!object->IsJSObject()) return true;
On 2015/10/23 12:03:42, Jakob wrote:
> nit: wouldn't "return false" be more appropriate? (I don't care much, it's
> temporary code and behind a flag anyway.)

I don't want to return false without also checking ShouldThrow and possibly
throwing, so just returning true here was simpler.

https://codereview.chromium.org/1421033002/diff/1/test/mjsunit/harmony/reflec...
File test/mjsunit/harmony/reflect-define-property.js (right):

https://codereview.chromium.org/1421033002/diff/1/test/mjsunit/harmony/reflec...
test/mjsunit/harmony/reflect-define-property.js:118: TypeError);
On 2015/10/23 12:03:43, Jakob wrote:
> nit: indentation (4 spaces for broken lines, or even better column-align with
> previous arguments); again several times below.

Done.

[neis, 2015-10-29 15:17:29]

The CQ bit was checked by neis@chromium.org

[neis, 2015-10-29 15:17:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1421033002/#ps20001
(title: "Address a comment and rebase.")

[commit-bot: I haz the power, 2015-10-29 15:17:41]

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

[commit-bot: I haz the power, 2015-10-29 15:39:41]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-29 15:39:57]

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