Bug 1084019 - Make the "name" property of function objects configurable. r=jorendorff
authorTill Schneidereit <till@tillschneidereit.net>
Sat, 17 Jan 2015 15:38:58 +0100
changeset 224403 be17c68f88cbe33caa1f6599bf135939d1704526
parent 224402 9124cbf3e75fee0f1e1e048fa0e5a2affee285dd
child 224404 6396e2e43cdde77a135ab48123c7927e252bc9a3
push id54211
push usertschneidereit@gmail.com
push dateSat, 17 Jan 2015 14:40:49 +0000
treeherdermozilla-inbound@be17c68f88cb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1084019
milestone38.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1084019 - Make the "name" property of function objects configurable. r=jorendorff
js/src/jsfun.cpp
js/src/jsfun.h
js/src/tests/ecma_5/strict/function-name.js
js/src/tests/ecma_6/Function/function-name.js
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -481,46 +481,54 @@ js::fun_resolve(JSContext *cx, HandleObj
         return true;
     }
 
     bool isLength = JSID_IS_ATOM(id, cx->names().length);
     if (isLength || JSID_IS_ATOM(id, cx->names().name)) {
         MOZ_ASSERT(!IsInternalFunctionObject(obj));
 
         RootedValue v(cx);
-        uint32_t attrs;
+
+        // Since f.length and f.name are configurable, they could be resolved
+        // and then deleted:
+        //     function f(x) {}
+        //     assertEq(f.length, 1);
+        //     delete f.length;
+        //     assertEq(f.name, "f");
+        //     delete f.name;
+        // Afterwards, asking for f.length or f.name again will cause this
+        // resolve hook to run again. Defining the property again the second
+        // time through would be a bug.
+        //     assertEq(f.length, 0);  // gets Function.prototype.length!
+        //     assertEq(f.name, "");  // gets Function.prototype.name!
+        // We use the RESOLVED_LENGTH and RESOLVED_NAME flags as a hack to prevent this
+        // bug.
         if (isLength) {
-            // Since f.length is configurable, it could be resolved and then deleted:
-            //     function f(x) {}
-            //     assertEq(f.length, 1);
-            //     delete f.length;
-            // Afterwards, asking for f.length again will cause this resolve
-            // hook to run again. Defining the property again the second
-            // time through would be a bug.
-            //     assertEq(f.length, 0);  // gets Function.prototype.length!
-            // We use the RESOLVED_LENGTH flag as a hack to prevent this bug.
             if (fun->hasResolvedLength())
                 return true;
 
             if (fun->isInterpretedLazy() && !fun->getOrCreateScript(cx))
                 return false;
             uint16_t length = fun->hasScript() ? fun->nonLazyScript()->funLength() :
                 fun->nargs() - fun->hasRest();
             v.setInt32(length);
-            attrs = JSPROP_READONLY;
         } else {
+            if (fun->hasResolvedName())
+                return true;
+
             v.setString(fun->atom() == nullptr ? cx->runtime()->emptyString : fun->atom());
-            attrs = JSPROP_READONLY | JSPROP_PERMANENT;
         }
 
-        if (!NativeDefineProperty(cx, fun, id, v, nullptr, nullptr, attrs))
+        if (!NativeDefineProperty(cx, fun, id, v, nullptr, nullptr, JSPROP_READONLY))
             return false;
 
         if (isLength)
             fun->setResolvedLength();
+        else
+            fun->setResolvedName();
 
         *resolvedp = true;
         return true;
     }
 
     return true;
 }
 
--- a/js/src/jsfun.h
+++ b/js/src/jsfun.h
@@ -43,16 +43,17 @@ class JSFunction : public js::NativeObje
                                        decompilable nor constructible. */
         SELF_HOSTED_CTOR = 0x0200,  /* function is self-hosted builtin constructor and
                                        must be constructible but not decompilable. */
         HAS_REST         = 0x0400,  /* function has a rest (...) parameter */
         ASMJS            = 0x0800,  /* function is an asm.js module or exported function */
         INTERPRETED_LAZY = 0x1000,  /* function is interpreted but doesn't have a script yet */
         ARROW            = 0x2000,  /* ES6 '(args) => body' syntax */
         RESOLVED_LENGTH  = 0x4000,  /* f.length has been resolved (see js::fun_resolve). */
+        RESOLVED_NAME    = 0x8000,  /* f.name has been resolved (see js::fun_resolve). */
 
         /* Derived Flags values for convenience: */
         NATIVE_FUN = 0,
         ASMJS_CTOR = ASMJS | NATIVE_CTOR,
         ASMJS_LAMBDA_CTOR = ASMJS | NATIVE_CTOR | LAMBDA,
         INTERPRETED_LAMBDA = INTERPRETED | LAMBDA,
         INTERPRETED_LAMBDA_ARROW = INTERPRETED | LAMBDA | ARROW,
         STABLE_ACROSS_CLONES = NATIVE_CTOR | IS_FUN_PROTO | EXPR_CLOSURE | HAS_GUESSED_ATOM |
@@ -129,16 +130,17 @@ class JSFunction : public js::NativeObje
     bool hasRest()                  const { return flags() & HAS_REST; }
     bool isInterpretedLazy()        const { return flags() & INTERPRETED_LAZY; }
     bool hasScript()                const { return flags() & INTERPRETED; }
 
     // Arrow functions store their lexical |this| in the first extended slot.
     bool isArrow()                  const { return flags() & ARROW; }
 
     bool hasResolvedLength()        const { return flags() & RESOLVED_LENGTH; }
+    bool hasResolvedName()          const { return flags() & RESOLVED_NAME; }
 
     bool hasJITCode() const {
         if (!hasScript())
             return false;
 
         return nonLazyScript()->hasBaselineScript() || nonLazyScript()->hasIonScript();
     }
 
@@ -201,16 +203,20 @@ class JSFunction : public js::NativeObje
     void setArrow() {
         flags_ |= ARROW;
     }
 
     void setResolvedLength() {
         flags_ |= RESOLVED_LENGTH;
     }
 
+    void setResolvedName() {
+        flags_ |= RESOLVED_NAME;
+    }
+
     JSAtom *atom() const { return hasGuessedAtom() ? nullptr : atom_.get(); }
 
     js::PropertyName *name() const {
         return hasGuessedAtom() || !atom_ ? nullptr : atom_->asPropertyName();
     }
 
     void initAtom(JSAtom *atom) { atom_.init(atom); }
 
rename from js/src/tests/ecma_5/strict/function-name.js
rename to js/src/tests/ecma_6/Function/function-name.js
--- a/js/src/tests/ecma_5/strict/function-name.js
+++ b/js/src/tests/ecma_6/Function/function-name.js
@@ -1,19 +1,64 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-
-/*
- * Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/licenses/publicdomain/
- */
-
-function fn() {
-  return function f(a, b, c) { };
+function testFunctionName(f) {
+    var name = f.name;
+    f.name = 'g';
+    assertEq(f.name, name);
+    assertEq(delete f.name, true);
+    assertEq(f.name, '');
+    assertEq(f.hasOwnProperty('name'), false);
+    f.name = 'g';
+    assertEq(f.name, '');
+    Object.defineProperty(f, 'name', {value: 'g'});
+    assertEq(f.name, 'g');
+}
+function testFunctionNameStrict(f) {
+    "use strict";
+    var name = f.name;
+    var error;
+    try {
+        f.name = 'g';
+    } catch (e) {
+        error = e;
+    }
+    assertEq(f.name, name);
+    assertEq(error instanceof TypeError, true);
+    assertEq(delete f.name, true);
+    assertEq(f.name, '');
+    assertEq(f.hasOwnProperty('name'), false);
+    error = null;
+    try {
+        f.name = 'g';
+    } catch (e) {
+        error = e;
+    }
+    assertEq(f.name, '');
+    assertEq(error instanceof TypeError, true);
+    Object.defineProperty(f, 'name', {value: 'g'});
+    assertEq(f.name, 'g');
 }
 
-assertEq(testLenientAndStrict('var f = fn(); f.name = "g"; f.name',
-                              returns("f"), raisesException(TypeError)),
-         true);
-assertEq(testLenientAndStrict('var f = fn(); delete f.name',
-                              returns(false), raisesException(TypeError)),
-         true);
+assertEq(Object.getOwnPropertyDescriptor(Object, "name").writable, false);
+assertEq(Object.getOwnPropertyDescriptor(Object, "name").enumerable, false);
+assertEq(Object.getOwnPropertyDescriptor(Object, "name").configurable, true);
+assertEq(Object.getOwnPropertyDescriptor(Object, "name").value, 'Object');
+assertEq(Object.getOwnPropertyDescriptor(function f(){}, "name").writable, false);
+assertEq(Object.getOwnPropertyDescriptor(function f(){}, "name").enumerable, false);
+assertEq(Object.getOwnPropertyDescriptor(function f(){}, "name").configurable, true);
+assertEq(Object.getOwnPropertyDescriptor(function f(){}, "name").value, 'f');
 
-reportCompare(true, true);
+// Basic test ensuring that Object.defineProperty works on pristine function.
+function f() {};
+Object.defineProperty(f, 'name', {value: 'g'});
+assertEq(f.name, 'g');
+
+// .name behaves as expected on scripted function.
+testFunctionName(function f(){});
+testFunctionNameStrict(function f(){});
+// .name behaves as expected on builtin function.
+testFunctionName(Function.prototype.apply);
+testFunctionNameStrict(Function.prototype.call);
+// .name behaves as expected on self-hosted builtin function.
+testFunctionName(Array.prototype.forEach);
+testFunctionNameStrict(Array.prototype.some);
+
+if (typeof reportCompare === "function")
+    reportCompare(0, 0);