[ESnext] Implement Object spread

[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

Object rest will be implemented in a follow on patch.

BUG=v8:5549
Review-Url: https://codereview.chromium.org/2606833002
Cr-Commit-Position: refs/heads/master@{#42102}
Committed: https://chromium.googlesource.com/v8/v8/+/a40b7172fe7d255b4ce3055d4ca7cdbbb3f09fb6

[gsathya, 2016-12-27 23:35:21]

Description was changed from

==========
[WIP] Implement Object.spread

BUG=5549
==========

to

==========
[WIP] Implement Object spread

BUG=5549
==========

[gsathya, 2016-12-28 05:28:04]

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

[commit-bot: I haz the power, 2016-12-28 05:28:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 05:31:58]

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

[commit-bot: I haz the power, 2016-12-28 05:31:59]

Dry run: Try jobs failed on following builders:
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...)

[gsathya, 2016-12-28 18:03:50]

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

[commit-bot: I haz the power, 2016-12-28 18:04:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-28 18:07:41]

Description was changed from

==========
[WIP] Implement Object spread

BUG=5549
==========

to

==========
[WIP] Implement Object spread

BUG=v8:5549
==========

[commit-bot: I haz the power, 2016-12-28 18:31:52]

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

[commit-bot: I haz the power, 2016-12-28 18:31:52]

Dry run: This issue passed the CQ dry run.

[gsathya, 2016-12-28 21:14:50]

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

[commit-bot: I haz the power, 2016-12-28 21:14:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 21:42:05]

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

[commit-bot: I haz the power, 2016-12-28 21:42:06]

Dry run: This issue passed the CQ dry run.

[gsathya, 2016-12-28 21:58:13]

Description was changed from

==========
[WIP] Implement Object spread

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

[gsathya, 2016-12-28 21:58:13]

gsathya@chromium.org changed reviewers:
+ adamk@chromium.org, littledan@chromium.org

[gsathya, 2016-12-28 23:22:09]

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

[commit-bot: I haz the power, 2016-12-28 23:22:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-28 23:23:55]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

[gsathya, 2016-12-28 23:42:57]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Renames Literal::is_computed_name_ to is_dynamic_name and uses this 
for spread properties as well, not just computed properties.

BUG=v8:5549
==========

[gsathya, 2016-12-28 23:43:06]

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

[commit-bot: I haz the power, 2016-12-28 23:43:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-28 23:45:32]

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

[commit-bot: I haz the power, 2016-12-28 23:45:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-28 23:46:17]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Renames Literal::is_computed_name_ to is_dynamic_name and uses this 
for spread properties as well, not just computed properties.

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Renames LiteralProperty::is_computed_name_ to is_dynamic_name and 
uses this for spread properties as well, not just computed properties.

BUG=v8:5549
==========

[gsathya, 2016-12-28 23:47:27]

Working on mjsunit tests, but would like feedback on implementation +
inspiration for negative parsing tests

[gsathya, 2016-12-29 00:01:51]

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

[commit-bot: I haz the power, 2016-12-29 00:01:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-29 00:02:06]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Renames LiteralProperty::is_computed_name_ to is_dynamic_name and 
uses this for spread properties as well, not just computed properties.

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

[commit-bot: I haz the power, 2016-12-29 00:26:08]

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

[commit-bot: I haz the power, 2016-12-29 00:26:09]

Dry run: This issue passed the CQ dry run.

[adamk, 2016-12-29 00:48:45]

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2118: BindingPatternUnexpectedToken();
I think you'll need to disallow this in assignment patterns too. A negative test
for this would demonstrate that.

var {...x} = {};

will fail, but

({...x} = {})

will succeed

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2123: expression = ParseAssignmentExpression(false,
CHECK_OK);
Don't you want to accept "in"? Please add a test, like

{...1 in {}}

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2355: if (allow_harmony_object_spread()) {
This can be a DCHECK, since ParsePropertyName shouldn't set kSpreadProperty
unless the flag is on anyway.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2356: DCHECK(!is_get && !is_set && !is_generator &&
!is_async);
you should add !*is_computed_name to this list.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2360: *ok = true;
No need to set *ok to true, that's how it starts. Only need to set to false on
failure.

https://codereview.chromium.org/2606833002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:767: Handle<JSReceiver> from =
Object::ToObject(isolate, source).ToHandleChecked();
I think everything from here down should be shared with the ObjectAssign
builtin; it looks like everything except the Set vs Create is the same.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6572: {"'use strict'; let y = { a: 1}; let x = ",
""},
Generally the context data is about different sorts of things than this, i.e.,
function vs top-level, strict vs non-strict.

For this I'd just start with something like the context data for the previous
test: strict vs non-strict, and then an expression context. Since these are just
parsing tests, there's no need to set up the "environment" like you've done here
(i.e., don't need to define variables you "reference").

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6607: {"'use strict'; let y = { a: 1}; let x = ",
""},
Same comment on context data here.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6619: "{ ...var z = y}",
I agree that negative tests are kinda tricky for this. Keywords is all that
really comes to mind at the moment (other than destructuring, which you handle
below).

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6636: {"try {} catch(", ") {}"},
Need to add an assignment context.

[gsathya, 2016-12-29 06:29:16]

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

[commit-bot: I haz the power, 2016-12-29 06:29:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-29 06:30:24]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.

BUG=v8:5549
==========

to

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

[commit-bot: I haz the power, 2016-12-29 06:31:57]

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

[commit-bot: I haz the power, 2016-12-29 06:32:00]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/18419)
  v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...)
  v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/14835)

[gsathya, 2016-12-29 06:36:26]

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

[commit-bot: I haz the power, 2016-12-29 06:36:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2016-12-29 06:37:14]

Description was changed from

==========
[WIP] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

to

==========
Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

[gsathya, 2016-12-29 06:37:42]

Description was changed from

==========
Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

to

==========
[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

[gsathya, 2016-12-29 06:40:57]

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2118: BindingPatternUnexpectedToken();
On 2016/12/29 00:48:45, adamk wrote:
> I think you'll need to disallow this in assignment patterns too. A negative
test
> for this would demonstrate that.
> 
> var {...x} = {};
> 
> will fail, but
> 
> ({...x} = {})
> 
> will succeed

Done.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2123: expression = ParseAssignmentExpression(false,
CHECK_OK);
On 2016/12/29 00:48:45, adamk wrote:
> Don't you want to accept "in"? Please add a test, like
> 
> {...1 in {}}

Done.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2355: if (allow_harmony_object_spread()) {
On 2016/12/29 00:48:45, adamk wrote:
> This can be a DCHECK, since ParsePropertyName shouldn't set kSpreadProperty
> unless the flag is on anyway.

Done.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2356: DCHECK(!is_get && !is_set && !is_generator &&
!is_async);
On 2016/12/29 00:48:45, adamk wrote:
> you should add !*is_computed_name to this list.

Done.

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2360: *ok = true;
On 2016/12/29 00:48:45, adamk wrote:
> No need to set *ok to true, that's how it starts. Only need to set to false on
> failure.

Done.

https://codereview.chromium.org/2606833002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:767: Handle<JSReceiver> from =
Object::ToObject(isolate, source).ToHandleChecked();
On 2016/12/29 00:48:45, adamk wrote:
> I think everything from here down should be shared with the ObjectAssign
> builtin; it looks like everything except the Set vs Create is the same.

Done.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6572: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 00:48:45, adamk wrote:
> Generally the context data is about different sorts of things than this, i.e.,
> function vs top-level, strict vs non-strict.
> 
> For this I'd just start with something like the context data for the previous
> test: strict vs non-strict, and then an expression context. Since these are
just
> parsing tests, there's no need to set up the "environment" like you've done
here
> (i.e., don't need to define variables you "reference").

Done.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6607: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 00:48:45, adamk wrote:
> Same comment on context data here.

Done.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6619: "{ ...var z = y}",
On 2016/12/29 00:48:45, adamk wrote:
> I agree that negative tests are kinda tricky for this. Keywords is all that
> really comes to mind at the moment (other than destructuring, which you handle
> below).

Done.

[gsathya, 2016-12-29 06:41:57]

gsathya@chromium.org changed reviewers:
+ bmeurer@chromium.org

[gsathya, 2016-12-29 06:42:33]

+benedikt for backends

[commit-bot: I haz the power, 2016-12-29 07:04:32]

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

[commit-bot: I haz the power, 2016-12-29 07:04:33]

Dry run: This issue passed the CQ dry run.

[Benedikt Meurer, 2016-12-29 08:39:31]

Haven't looked at parsing, but the rest is LGTM. We should probably think about
an IC soon that has a monomorphic state which gathers maps of both objects, then
TF can generate very decent code; React/Redux folks will love it.

Can you add a paragraph to the ES2015 perf plan document?

[Dan Ehrenberg, 2016-12-29 17:31:04]

The code looks good here. A couple small points: Could you call out in the
commit message that destructuring is left for a follow-on patch? Let's wait to
land this until we have the mjsunit tests in the same patch.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2098: bool use_set) {
I'm slightly worried that checking use_set parameter could cause a little bit of
runtime overhead. In other places, like case conversion, we use a template to
generate multiple versions to avoid a runtime check. However, here, there's a
lot more going on, so that's less likely. I don't think a change is needed, but
something to maybe keep in mind if we do see perf regressions following this
patch.

[adamk, 2016-12-29 18:32:46]

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2118: BindingPatternUnexpectedToken();
On 2016/12/29 06:40:56, gsathya wrote:
> On 2016/12/29 00:48:45, adamk wrote:
> > I think you'll need to disallow this in assignment patterns too. A negative
> test
> > for this would demonstrate that.
> > 
> > var {...x} = {};
> > 
> > will fail, but
> > 
> > ({...x} = {})
> > 
> > will succeed
> 
> Done.

You can get both at once with

classifier()->RecordPatternError(...)

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6572: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 06:40:56, gsathya wrote:
> On 2016/12/29 00:48:45, adamk wrote:
> > Generally the context data is about different sorts of things than this,
i.e.,
> > function vs top-level, strict vs non-strict.
> > 
> > For this I'd just start with something like the context data for the
previous
> > test: strict vs non-strict, and then an expression context. Since these are
> just
> > parsing tests, there's no need to set up the "environment" like you've done
> here
> > (i.e., don't need to define variables you "reference").
> 
> Done.

Sorry if I wasn't clear, but I don't think you need this much context. Just the
following would give you the same amount of coverage:

{"x = ", ""},
{"'use strict'; x = ", ""}

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6607: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 06:40:56, gsathya wrote:
> On 2016/12/29 00:48:45, adamk wrote:
> > Same comment on context data here.
> 
> Done.

Same here

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6619: "{ ...var z = y}",
On 2016/12/29 06:40:56, gsathya wrote:
> On 2016/12/29 00:48:45, adamk wrote:
> > I agree that negative tests are kinda tricky for this. Keywords is all that
> > really comes to mind at the moment (other than destructuring, which you
handle
> > below).
> 
> Done.

? I don't see any additional tests here.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2113: for (int j = 0; j < keys->length(); ++j) {
s/j/i/ now that this isn't a nested loop.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2140: if (!success) return Nothing<bool>();
How could this fail?

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2141: MAYBE_RETURN(JSReceiver::CreateDataProperty(&it,
prop_value,
Or this?

I think it's quite likely you could use DefineOwnPropertyIgnoreAttributes() in
this branch, since we're guaranteed that there are no non-configurable
properties on the object.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2606833002/diff/200001/src/objects.h#newcode2002
src/objects.h:2002: MUST_USE_RESULT static Maybe<bool> SetOrCopyDataProperties(
Please give this a comment.

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-bas...
src/parsing/parser-base.h:2356: case PropertyKind::kSpreadProperty: {
Nit: no need for the block.

https://codereview.chromium.org/2606833002/diff/200001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:764: MAYBE_RETURN(
Can this fail?

https://codereview.chromium.org/2606833002/diff/200001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6642: "{ ...y } = {} ",
Rather than doing this here, you could cover everything by adding an assignment
context above, like:

{"(", " = {})"}

[gsathya, 2017-01-05 21:35:20]

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

[commit-bot: I haz the power, 2017-01-05 21:35:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2017-01-05 21:37:35]

Description was changed from

==========
[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

BUG=v8:5549
==========

to

==========
[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

Object rest will be implemented in a follow on patch.

BUG=v8:5549
==========

[commit-bot: I haz the power, 2017-01-05 22:04:20]

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

[commit-bot: I haz the power, 2017-01-05 22:04:21]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-05 22:18:36]

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/140001/src/parsing/parser-bas...
src/parsing/parser-base.h:2118: BindingPatternUnexpectedToken();
On 2016/12/29 18:32:45, adamk wrote:
> On 2016/12/29 06:40:56, gsathya wrote:
> > On 2016/12/29 00:48:45, adamk wrote:
> > > I think you'll need to disallow this in assignment patterns too. A
negative
> > test
> > > for this would demonstrate that.
> > > 
> > > var {...x} = {};
> > > 
> > > will fail, but
> > > 
> > > ({...x} = {})
> > > 
> > > will succeed
> > 
> > Done.
> 
> You can get both at once with
> 
> classifier()->RecordPatternError(...)

Done.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6572: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 18:32:45, adamk wrote:
> On 2016/12/29 06:40:56, gsathya wrote:
> > On 2016/12/29 00:48:45, adamk wrote:
> > > Generally the context data is about different sorts of things than this,
> i.e.,
> > > function vs top-level, strict vs non-strict.
> > > 
> > > For this I'd just start with something like the context data for the
> previous
> > > test: strict vs non-strict, and then an expression context. Since these
are
> > just
> > > parsing tests, there's no need to set up the "environment" like you've
done
> > here
> > > (i.e., don't need to define variables you "reference").
> > 
> > Done.
> 
> Sorry if I wasn't clear, but I don't think you need this much context. Just
the
> following would give you the same amount of coverage:
> 
> {"x = ", ""},
> {"'use strict'; x = ", ""}

Done.

https://codereview.chromium.org/2606833002/diff/140001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6607: {"'use strict'; let y = { a: 1}; let x = ",
""},
On 2016/12/29 18:32:45, adamk wrote:
> On 2016/12/29 06:40:56, gsathya wrote:
> > On 2016/12/29 00:48:45, adamk wrote:
> > > Same comment on context data here.
> > 
> > Done.
> 
> Same here

Done.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2098: bool use_set) {
On 2016/12/29 17:31:03, Dan Ehrenberg wrote:
> I'm slightly worried that checking use_set parameter could cause a little bit
of
> runtime overhead. In other places, like case conversion, we use a template to
> generate multiple versions to avoid a runtime check. However, here, there's a
> lot more going on, so that's less likely. I don't think a change is needed,
but
> something to maybe keep in mind if we do see perf regressions following this
> patch.

Acknowledged.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2113: for (int j = 0; j < keys->length(); ++j) {
On 2016/12/29 18:32:46, adamk wrote:
> s/j/i/ now that this isn't a nested loop.

Done.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2140: if (!success) return Nothing<bool>();
On 2016/12/29 18:32:46, adamk wrote:
> How could this fail?

Changed to CHECK.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.cc#newcode...
src/objects.cc:2141: MAYBE_RETURN(JSReceiver::CreateDataProperty(&it,
prop_value,
On 2016/12/29 18:32:46, adamk wrote:
> Or this?
> 
> I think it's quite likely you could use DefineOwnPropertyIgnoreAttributes() in
> this branch, since we're guaranteed that there are no non-configurable
> properties on the object.

Changed to CHECK.

https://codereview.chromium.org/2606833002/diff/200001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2606833002/diff/200001/src/objects.h#newcode2002
src/objects.h:2002: MUST_USE_RESULT static Maybe<bool> SetOrCopyDataProperties(
On 2016/12/29 18:32:46, adamk wrote:
> Please give this a comment.

Done.

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-bas...
src/parsing/parser-base.h:2356: case PropertyKind::kSpreadProperty: {
On 2016/12/29 18:32:46, adamk wrote:
> Nit: no need for the block.

Rest of the cases seem to be blocks. Changed anyway

https://codereview.chromium.org/2606833002/diff/200001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:764: MAYBE_RETURN(
On 2016/12/29 18:32:46, adamk wrote:
> Can this fail?

Yes, the error needs to propagate up from SetOrCopyDataProperties

https://codereview.chromium.org/2606833002/diff/200001/test/cctest/test-parsi...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2606833002/diff/200001/test/cctest/test-parsi...
test/cctest/test-parsing.cc:6642: "{ ...y } = {} ",
On 2016/12/29 18:32:46, adamk wrote:
> Rather than doing this here, you could cover everything by adding an
assignment
> context above, like:
> 
> {"(", " = {})"}

Done.

[adamk, 2017-01-05 22:39:14]

Looking good, just a few more comments.

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/200001/src/parsing/parser-bas...
src/parsing/parser-base.h:2356: case PropertyKind::kSpreadProperty: {
On 2017/01/05 22:18:36, gsathya wrote:
> On 2016/12/29 18:32:46, adamk wrote:
> > Nit: no need for the block.
> 
> Rest of the cases seem to be blocks. Changed anyway

The rest of the cases that are blocks declare variables; that's the only reason
to use a block in a case.

https://codereview.chromium.org/2606833002/diff/240001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2606833002/diff/240001/src/objects.cc#newcode...
src/objects.cc:2136: // 4a ii 2. Perform ? CreateDataProperty(target, nextKey,
propValue).
s/?/!/ as you said.

https://codereview.chromium.org/2606833002/diff/240001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2606833002/diff/240001/src/objects.h#newcode2005
src/objects.h:2005: // This uses Set or CreateDataProperty depending on the
use_set
This could use more explanation, e.g.:

"Reads all enumerable own properties of source and adds them to target, using
either Set or CreateDataProperty depending on the use_set argument".

https://codereview.chromium.org/2606833002/diff/240001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/240001/src/parsing/parser-bas...
src/parsing/parser-base.h:2116: case Token::ELLIPSIS: {
No need for this block

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
File test/mjsunit/harmony/object-spread-basic.js (right):

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:5: // Flags: --harmony-object-spread
--allow-natives-syntax
No need for --allow-natives-syntax, that's only required if you call a
%RuntimeFunction.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:8: // assertThrows(function() { "use
strict"; proxy[symbol] = 42 }, TypeError);
What's up with these two lines?

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:13: assertEquals(x, y = { ...x});
What is this testing that's different from the above?

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:65: assertEquals({2: 2, a: 1}, y = {
...x });
Don't think you need this test, it's just testing computed property names,
nothing related to spread.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:103: assertThrows(() => { y = { ...p
} });
Can you also add a non-error, non-empty Proxy case?

[gsathya, 2017-01-05 22:55:25]

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

[commit-bot: I haz the power, 2017-01-05 22:55:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2017-01-05 22:56:29]

https://codereview.chromium.org/2606833002/diff/240001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2606833002/diff/240001/src/objects.cc#newcode...
src/objects.cc:2136: // 4a ii 2. Perform ? CreateDataProperty(target, nextKey,
propValue).
On 2017/01/05 22:39:14, adamk wrote:
> s/?/!/ as you said.

But it's not updated in the spec .. yet. Changed

https://codereview.chromium.org/2606833002/diff/240001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/2606833002/diff/240001/src/objects.h#newcode2005
src/objects.h:2005: // This uses Set or CreateDataProperty depending on the
use_set
On 2017/01/05 22:39:14, adamk wrote:
> This could use more explanation, e.g.:
> 
> "Reads all enumerable own properties of source and adds them to target, using
> either Set or CreateDataProperty depending on the use_set argument".

Done.

https://codereview.chromium.org/2606833002/diff/240001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2606833002/diff/240001/src/parsing/parser-bas...
src/parsing/parser-base.h:2116: case Token::ELLIPSIS: {
On 2017/01/05 22:39:14, adamk wrote:
> No need for this block

Done.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
File test/mjsunit/harmony/object-spread-basic.js (right):

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:5: // Flags: --harmony-object-spread
--allow-natives-syntax
On 2017/01/05 22:39:14, adamk wrote:
> No need for --allow-natives-syntax, that's only required if you call a
> %RuntimeFunction.

Done.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:8: // assertThrows(function() { "use
strict"; proxy[symbol] = 42 }, TypeError);
On 2017/01/05 22:39:14, adamk wrote:
> What's up with these two lines?

copy paste fail

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:13: assertEquals(x, y = { ...x});
On 2017/01/05 22:39:14, adamk wrote:
> What is this testing that's different from the above?

testing AssignmentExpression actually, since i use this in the rest of the
asserts. removed.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:65: assertEquals({2: 2, a: 1}, y = {
...x });
On 2017/01/05 22:39:14, adamk wrote:
> Don't think you need this test, it's just testing computed property names,
> nothing related to spread.

Done.

https://codereview.chromium.org/2606833002/diff/240001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:103: assertThrows(() => { y = { ...p
} });
On 2017/01/05 22:39:14, adamk wrote:
> Can you also add a non-error, non-empty Proxy case?

Done.

[adamk, 2017-01-05 23:09:27]

lgtm % test

https://codereview.chromium.org/2606833002/diff/280001/test/mjsunit/harmony/o...
File test/mjsunit/harmony/object-spread-basic.js (right):

https://codereview.chromium.org/2606833002/diff/280001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:105: assertEquals({}, y = { ...p });
Sorry, when I said "non-empty" I meant "generates a non-empty object".

[gsathya, 2017-01-05 23:16:33]

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

[commit-bot: I haz the power, 2017-01-05 23:16:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gsathya, 2017-01-05 23:17:28]

https://codereview.chromium.org/2606833002/diff/280001/test/mjsunit/harmony/o...
File test/mjsunit/harmony/object-spread-basic.js (right):

https://codereview.chromium.org/2606833002/diff/280001/test/mjsunit/harmony/o...
test/mjsunit/harmony/object-spread-basic.js:105: assertEquals({}, y = { ...p });
On 2017/01/05 23:09:27, adamk wrote:
> Sorry, when I said "non-empty" I meant "generates a non-empty object".

that's actually what i tried /o\

[commit-bot: I haz the power, 2017-01-05 23:40:25]

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

[commit-bot: I haz the power, 2017-01-05 23:40:26]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-05 23:42:11]

The CQ bit was checked by gsathya@chromium.org

[gsathya, 2017-01-05 23:42:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, adamk@chromium.org
Link to the patchset: https://codereview.chromium.org/2606833002/#ps300001
(title: "fix test")

[commit-bot: I haz the power, 2017-01-05 23:42:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 23:44:27]

CQ is committing da patch.

Bot data: {"patchset_id": 300001, "attempt_start_ts": 1483659731985180,
"parent_rev": "a5a376c53071067bc5b88c161819b24b9f03949a", "commit_rev":
"a40b7172fe7d255b4ce3055d4ca7cdbbb3f09fb6"}

[commit-bot: I haz the power, 2017-01-05 23:44:33]

Description was changed from

==========
[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

Object rest will be implemented in a follow on patch.

BUG=v8:5549
==========

to

==========
[ESnext] Implement Object spread

This patch adds parsing of spread object property. 

-- Changes ParsePropertyName to parse Token::ELLIPSIS. 
-- Throws if rest is encountered by setting a pattern error.
-- Adds a new PropertyKind enum (SPREAD)
-- Adds a new ObjectLiteralProperty::kind (SPREAD)
-- Adds a new harmony-object-spread flag and protects the parser code 
with it.
-- Adds a new runtime function called CopyDataProperties
-- Does not add any support for this feature in fullcodegen.
-- Ignition calls out to a runtime function CopyDataProperties to 
perform spread operation.
-- Move FastAssign from builtins-objects.cc to objects.cc
-- Refactor Builtin_ObjectAssign to use SetOrCopyDataProperties

Object rest will be implemented in a follow on patch.

BUG=v8:5549

Review-Url: https://codereview.chromium.org/2606833002
Cr-Commit-Position: refs/heads/master@{#42102}
Committed:
https://chromium.googlesource.com/v8/v8/+/a40b7172fe7d255b4ce3055d4ca7cdbbb3f...
==========

[commit-bot: I haz the power, 2017-01-05 23:44:34]

Committed patchset #16 (id:300001) as
https://chromium.googlesource.com/v8/v8/+/a40b7172fe7d255b4ce3055d4ca7cdbbb3f...