[intl] Clean up function name handling in AddBoundMethod

[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
Committed: https://crrev.com/54ce1935b5160045822fa5f17ab7cd01d54cf884
Cr-Commit-Position: refs/heads/master@{#37459}

[Dan Ehrenberg, 2016-06-29 21:10:12]

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js#newcode124
src/js/i18n.js:124: %object_define_property(getter, "name", {value: "get " +
methodName});
The name should be configurable, but seems to be defined as non-configurable
here.

Style nit: It seems a little cleaner to switch this with line 118, setting up
getter first and then attatching it to obj.prototype.

[adamk, 2016-06-29 22:53:10]

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js#newcode124
src/js/i18n.js:124: %object_define_property(getter, "name", {value: "get " +
methodName});
On 2016/06/29 21:10:12, Dan Ehrenberg wrote:
> The name should be configurable, but seems to be defined as non-configurable
> here.

For pre-existing properties (as "name" is here), define property won't modify
the existing attributes. So this still leaves us with a configurable property.
But I can future-proof if you'd like by adding "configurable: true" to the
descriptor.

> Style nit: It seems a little cleaner to switch this with line 118, setting up
> getter first and then attatching it to obj.prototype.

Strangely, swapping the order causes the name to become "N". Will investigate...

[adamk, 2016-06-29 23:18:06]

Description was changed from

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch amends the InstallGetter interface to allow skipping the
%FunctionSetName step (replacing the unused "prefix" parameter), and
then uses %object_define_property to set the name on the JSFunctions
themselves.

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
==========

to

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
==========

[adamk, 2016-06-29 23:19:16]

See CL description for what the heck this is doing.

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2109223002/diff/1/src/js/i18n.js#newcode124
src/js/i18n.js:124: %object_define_property(getter, "name", {value: "get " +
methodName});
On 2016/06/29 22:53:10, adamk wrote:
> On 2016/06/29 21:10:12, Dan Ehrenberg wrote:
> > Style nit: It seems a little cleaner to switch this with line 118, setting
up
> > getter first and then attatching it to obj.prototype.
> 
> Strangely, swapping the order causes the name to become "N". Will
investigate...

I now take a different approach. Let me know if you think it's acceptable (also
happy to put this on the backlog if we decide it's not worth being hacky to get
these tests passing).

[Dan Ehrenberg, 2016-06-30 19:33:05]

lgtm modulo fixing that Intl is apparently very broken by this change

[adamk, 2016-06-30 19:36:10]

Changed getter back to being a regular function, since it uses "this" (had to
add a %FunctionRemovePrototype call to retain existing behavior).

[adamk, 2016-06-30 19:36:13]

The CQ bit was checked by adamk@chromium.org

[adamk, 2016-06-30 19:36:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/2109223002/#ps60001
(title: "Getter cannot be an arrow function, of course")

[commit-bot: I haz the power, 2016-06-30 19:36:19]

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-06-30 19:39:32]

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

[commit-bot: I haz the power, 2016-06-30 19:39:34]

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_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_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/8340)

[Dan Ehrenberg, 2016-06-30 19:41:57]

lgtm

[adamk, 2016-06-30 19:44:34]

The CQ bit was checked by adamk@chromium.org

[adamk, 2016-06-30 19:44:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/2109223002/#ps80001
(title: "Fix build, I think")

[commit-bot: I haz the power, 2016-06-30 19:44:39]

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-06-30 20:08:38]

Description was changed from

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
==========

to

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
==========

[commit-bot: I haz the power, 2016-06-30 20:08:39]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-30 20:10:56]

Description was changed from

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778
==========

to

==========
[intl] Clean up function name handling in AddBoundMethod

AddBoundMethod, in i18n.js, returns functions all of which share the
same backing SharedFunctionInfo, which means that its calls to
InstallGetter were causing all such functions to have a single name
(that of the last caller, "get breakType").

This patch skips calling InstallGetter and instead directly calls
%DefineGetterPropertyUnchecked, which itself sets the name property
on the JSFunction instance (it knows how to do this in order to handle
getters that have computed property names).

Also takes care of a TODO having to do with the inner boundMethod:
its name is now made empty, by using a new macro that gets around
ES2015's function name inference.

Finally, removes a redundant %FunctionRemovePrototype: arrow functions
have no prototypes to begin with.

R=littledan@chromium.org
BUG=v8:4778

Committed: https://crrev.com/54ce1935b5160045822fa5f17ab7cd01d54cf884
Cr-Commit-Position: refs/heads/master@{#37459}
==========

[commit-bot: I haz the power, 2016-06-30 20:10:57]

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