[intl] Clean up function name handling in AddBoundMethod

src/js/i18n.js

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // ECMAScript 402 API implementation.
 
 /**
  * Intl object is a single object that has some named properties,
  * all of which are constructors.
  */
@@ skipping 76 matching lines @@
 function AddBoundMethod(obj, methodName, implementation, length, type) {
   %CheckIsBootstrapping();
   var internalName = %CreatePrivateSymbol(methodName);
   var getter = function() {
     if (!%IsInitializedIntlObjectOfType(this, type)) {
       throw MakeTypeError(kMethodCalledOnWrongObject, methodName);
     }
     if (IS_UNDEFINED(this[internalName])) {
       var boundMethod;
       if (IS_UNDEFINED(length) || length === 2) {
-        boundMethod = (x, y) => implementation(this, x, y);
+        boundMethod = ANONYMOUS_FUNCTION((x, y) => implementation(this, x, y));
       } else if (length === 1) {
-        boundMethod = x => implementation(this, x);
+        boundMethod = ANONYMOUS_FUNCTION(x => implementation(this, x));
       } else {
-        boundMethod = (...args) => {
-          // DateTimeFormat.format needs to be 0 arg method, but can stil
-          // receive optional dateValue param. If one was provided, pass it
+        boundMethod = ANONYMOUS_FUNCTION((...args) => {
+          // DateTimeFormat.format needs to be 0 arg method, but can still
+          // receive an optional dateValue param. If one was provided, pass it
           // along.
           if (args.length > 0) {
             return implementation(this, args[0]);
           } else {
             return implementation(this);
           }
-        }
+        });
       }
-      // TODO(littledan): Once function name reform is shipped, remove the
-      // following line and wrap the boundMethod definition in an anonymous
-      // function macro.
-      %FunctionSetName(boundMethod, '__bound' + methodName + '__');
-      %FunctionRemovePrototype(boundMethod);
       %SetNativeFlag(boundMethod);
       this[internalName] = boundMethod;
     }
     return this[internalName];
   };
 
-  InstallGetter(obj.prototype, methodName, getter, DONT_ENUM);
+  InstallGetter(obj.prototype, methodName, getter, DONT_ENUM,
+                true /* skipSetFunctionName */);
+
+  // All functions returned from AddBoundMethod share a single
+  // SharedFunctionInfo, so we have to set the "name" property
+  // on the JSFunction itself.
+  %object_define_property(getter, "name", {value: "get " + methodName});

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

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]

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.
Strangely, swapping the order causes the name to become "N". Will investigate...

[adamk, 2016/06/29 23:19:15]

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).

 }
 
 // -------------------------------------------------------------------
 
 var Intl = {};
 
 %AddNamedProperty(global, "Intl", Intl, DONT_ENUM);
 
 /**
  * Caches available locales for each service.
@@ skipping 2115 matching lines @@
   }
 );
 
 utils.Export(function(to) {
   to.AddBoundMethod = AddBoundMethod;
   to.IntlParseDate = IntlParseDate;
   to.IntlParseNumber = IntlParseNumber;
 });
 
 })