Bug 1289749 - Backout the existing function.name implementation; r=jorendorff
authorMorgan Phillips <winter2718@gmail.com>
Wed, 27 Jul 2016 15:40:32 -0700
changeset 349601 b6bc93be1406a0bf83f5f6c49df44214a42817e5
parent 349600 5b08ae92f655c5ba6f2c356ab43e323a3ab0cf8c
child 349602 34f662c8a932551e07a6c66853b9f696b5906f42
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1289749
milestone50.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 1289749 - Backout the existing function.name implementation; r=jorendorff The existing implementation can not meet the spec's requirement that function.name is only set during assignment with the left-hand side is an identifier reference.
js/src/frontend/NameFunctions.cpp
js/src/jit-test/tests/self-test/assertDeepEq.js
js/src/jsfun.cpp
js/src/tests/ecma_5/extensions/error-tostring-function.js
js/src/tests/ecma_6/Function/name.js
--- a/js/src/frontend/NameFunctions.cpp
+++ b/js/src/frontend/NameFunctions.cpp
@@ -178,19 +178,17 @@ class NameResolver
         }
 
         return nullptr;
     }
 
     /*
      * Resolve the name of a function. If the function already has a name
      * listed, then it is skipped. Otherwise an intelligent name is guessed to
-     * assign to the function's displayAtom field. Note: the format used to
-     * describe guessed names is relied upon for generation of es6 compatible
-     * [function] names (see: FunctionNameFromDisplayName in jsfun.cpp).
+     * assign to the function's displayAtom field.
      */
     bool resolveFun(ParseNode* pn, HandleAtom prefix, MutableHandleAtom retAtom) {
         MOZ_ASSERT(pn != nullptr);
         MOZ_ASSERT(pn->isKind(PNK_FUNCTION));
         MOZ_ASSERT(pn->isArity(PN_CODE));
         RootedFunction fun(cx, pn->pn_funbox->function());
 
         StringBuffer buf(cx);
--- a/js/src/jit-test/tests/self-test/assertDeepEq.js
+++ b/js/src/jit-test/tests/self-test/assertDeepEq.js
@@ -72,16 +72,23 @@ assertNotDeepEq(p, q);
 assertNotDeepEq(q, p);
 q.prop = 1;
 assertDeepEq(q, p);
 
 // functions
 assertNotDeepEq(() => 1, () => 2);
 assertNotDeepEq((...x) => 1, x => 1);
 assertNotDeepEq(function f(){}, function g(){});
+var f1 = function () {}, f2 = function () {};
+assertDeepEq(f1, f1);
+assertDeepEq(f1, f2);  // same text, close enough
+f1.prop = 1;
+assertNotDeepEq(f1, f2);
+f2.prop = 1;
+assertDeepEq(f1, f2);
 
 // recursion
 var a = [], b = [];
 a[0] = a;
 b[0] = b;
 assertDeepEq(a, b);
 a[0] = b;
 assertNotDeepEq(a, b);  // [#1=[#1#]] is not structurally equivalent to #1=[[#1#]]
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -10,17 +10,16 @@
 
 #include "jsfuninlines.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/PodOperations.h"
 #include "mozilla/Range.h"
 
-#include <ctype.h>
 #include <string.h>
 
 #include "jsapi.h"
 #include "jsarray.h"
 #include "jsatom.h"
 #include "jscntxt.h"
 #include "jsobj.h"
 #include "jsscript.h"
@@ -516,22 +515,17 @@ fun_resolve(JSContext* cx, HandleObject 
                 MOZ_ASSERT(fun->name() != cx->names().empty);
 
                 // Unnamed class expressions should not get a .name property
                 // at all.
                 if (fun->name() == nullptr)
                     return true;
             }
 
-            JSAtom* functionName = fun->functionName(cx);
-
-            if (!functionName)
-                ReportOutOfMemory(cx);
-
-            v.setString(functionName);
+            v.setString(fun->name() == nullptr ? cx->runtime()->emptyString : fun->name());
         }
 
         if (!NativeDefineProperty(cx, fun, id, v, nullptr, nullptr,
                                   JSPROP_READONLY | JSPROP_RESOLVING))
         {
             return false;
         }
 
@@ -1550,191 +1544,16 @@ JSFunction::createScriptForLazilyInterpr
     MOZ_ASSERT(fun->isSelfHostedBuiltin());
     RootedAtom funAtom(cx, &fun->getExtendedSlot(LAZY_FUNCTION_NAME_SLOT).toString()->asAtom());
     if (!funAtom)
         return false;
     Rooted<PropertyName*> funName(cx, funAtom->asPropertyName());
     return cx->runtime()->cloneSelfHostedFunctionScript(cx, funName, fun);
 }
 
-// Copy a substring into a buffer while simultaneously unescaping any escaped
-// character sequences.
-template <typename TextChar>
-static bool
-UnescapeSubstr(TextChar* text, size_t start, size_t length, StringBuffer& buf)
-{
-    size_t end = start + length;
-    for (size_t i = start; i < end; i++) {
-
-        if (text[i] == '\\' && i + 1 <= end) {
-            bool status = false;
-            switch (text[++i]) {
-                case (TextChar)'b' : status = buf.append('\b'); break;
-                case (TextChar)'f' : status = buf.append('\f'); break;
-                case (TextChar)'n' : status = buf.append('\n'); break;
-                case (TextChar)'r' : status = buf.append('\r'); break;
-                case (TextChar)'t' : status = buf.append('\t'); break;
-                case (TextChar)'v' : status = buf.append('\v'); break;
-                case (TextChar)'"' : status = buf.append('"');  break;
-                case (TextChar)'\?': status = buf.append('\?'); break;
-                case (TextChar)'\'': status = buf.append('\''); break;
-                case (TextChar)'\\': status = buf.append('\\'); break;
-                case (TextChar)'x' : {
-                    if (i + 2 >= end ||
-                        !JS7_ISHEX(text[i + 1]) ||
-                        !JS7_ISHEX(text[i + 2]))
-                    {
-                        return false;
-                    }
-                    char c = JS7_UNHEX((char)text[i + 1]) << 4;
-                    c += JS7_UNHEX((char)text[i + 2]);
-                    i += 2;
-                    status = buf.append(c);
-                    break;
-                }
-                case (TextChar)'u' : {
-                    uint32_t code = 0;
-                    if (!(i + 4 < end &&
-                        JS7_ISHEX(text[i + 1]) &&
-                        JS7_ISHEX(text[i + 2]) &&
-                        JS7_ISHEX(text[i + 3]) &&
-                        JS7_ISHEX(text[i + 4])))
-                    {
-                            return false;
-                    }
-
-                    code = JS7_UNHEX(text[i + 1]);
-                    code = (code << 4) + JS7_UNHEX(text[i + 2]);
-                    code = (code << 4) + JS7_UNHEX(text[i + 3]);
-                    code = (code << 4) + JS7_UNHEX(text[i + 4]);
-                    i += 4;
-                    if (code < 0x10000) {
-                        status = buf.append((char16_t)code);
-                    } else {
-                        status = buf.append((char16_t)((code - 0x10000) / 1024 + 0xD800)) &&
-                            buf.append((char16_t)(((code - 0x10000) % 1024) + 0xDC00));
-                    }
-                    break;
-                }
-            }
-            if (!status)
-                return false;
-        } else {
-            if (!buf.append(text[i]))
-                return false;
-        }
-    }
-    return true;
-}
-
-// Locate a function name matching the format described in ECMA-262 (2016-02-27) 9.2.11
-// SetFunctionName and copy it into a string buffer.
-template <typename TextChar>
-static bool
-FunctionNameFromDisplayName(JSContext* cx, TextChar* text, size_t textLen, StringBuffer& buf)
-{
-    size_t start = 0, end = textLen;
-
-    for (size_t i = 0; i < textLen; i++) {
-        // The string is parsed in reverse order.
-        size_t index = (textLen - i) - 1;
-
-        // Because we're only interested in the name of the property where
-        // some function is bound we can stop parsing as soon as we see
-        // one of these cases: foo.methodname, prefix/foo.methodname,
-        // and methodname<
-        if (text[index] == (TextChar)'.' ||
-            text[index] == (TextChar)'<' ||
-            text[index] == (TextChar)'/')
-        {
-            start = index + 1;
-            break;
-        }
-
-        // Property names which aren't valid identifiers are represented
-        // as a quoted string between square brackets: "[\"prop@name\"]"
-        if (text[index] == (TextChar)']' && index > 1
-            && text[index - 1] == (TextChar)'"') {
-
-            // search for '["' which signals the end of a quoted
-            // property name in square brackets.
-            for (size_t j = 0; j <= index - 2; j++) {
-                if (text[(index - j) - 1] == (TextChar)'"' &&
-                    text[(index - j) - 2] == (TextChar)'[') {
-                    start = (index - j);
-                    end = index - 1;
-
-                    // The value is represented as a quoted string within a
-                    // string (e.g. "\"foo\""), so we'll need to unquote it.
-                    return UnescapeSubstr(text, start, end - start, buf);
-                }
-            }
-
-            // We should never fail to find the beginning of a square bracket.
-            MOZ_ASSERT(0);
-            break;
-        } else if (text[index] == (TextChar)']') {
-            // Here we expect an unquoted numeric value. If that's the case
-            // we can just skip to the closing bracket to save some work.
-            for (size_t j = 0; j < index; j++) {
-                TextChar numeral = text[(index - j) - 1];
-                if (numeral == (TextChar)'[') {
-                    start = index - j;
-                    end = index;
-                    break;
-                } else if (numeral > (TextChar)'9' || numeral < (TextChar)'0') {
-                    // Fail on anything that isn't a numeral (Bug 1282332).
-                    return false;
-                }
-            }
-            break;
-        }
-    }
-
-    for (size_t i = start; i < end; i++) {
-        if (!buf.append(text[i]))
-            return false;
-    }
-    return true;
-}
-
-JSAtom*
-JSFunction::functionName(JSContext* cx) const
-{
-    if (!atom_)
-        return cx->runtime()->emptyString;
-
-    // Only guessed atoms are worth parsing.
-    if (!hasGuessedAtom())
-        return atom_;
-
-    size_t textLen = atom_->length();
-    if (textLen < 1)
-        return atom_;
-
-    StringBuffer buf(cx);
-
-    // At this point we know that we're dealing with a display name,
-    // which we may need to parse in order to produce an es6 compatible
-    // function name.
-    if (atom_->hasLatin1Chars()) {
-        JS::AutoCheckCannotGC nogc;
-        const Latin1Char* textChars = atom_->latin1Chars(nogc);
-        if (!FunctionNameFromDisplayName(cx, textChars, textLen, buf))
-            return cx->runtime()->emptyString;
-    } else {
-        JS::AutoCheckCannotGC nogc;
-        const char16_t* textChars = atom_->twoByteChars(nogc);
-        if (!FunctionNameFromDisplayName(cx, textChars, textLen, buf))
-            return cx->runtime()->emptyString;
-    }
-
-    return buf.finishAtom();
-}
-
 void
 JSFunction::maybeRelazify(JSRuntime* rt)
 {
     // Try to relazify functions with a non-lazy script. Note: functions can be
     // marked as interpreted despite having no script yet at some points when
     // parsing.
     if (!hasScript() || !u.i.s.script_)
         return;
--- a/js/src/tests/ecma_5/extensions/error-tostring-function.js
+++ b/js/src/tests/ecma_5/extensions/error-tostring-function.js
@@ -22,24 +22,24 @@ function ErrorToString(v)
 // extension-land test.
 
 assertEq(ErrorToString(function f(){}), "f");
 assertEq(ErrorToString(function g(){}), "g");
 assertEq(ErrorToString(function(){}), "");
 
 var fn1 = function() {};
 fn1.message = "ohai";
-assertEq(ErrorToString(fn1), "fn1: ohai");
+assertEq(ErrorToString(fn1), "ohai");
 
 var fn2 = function blerch() {};
 fn2.message = "fnord";
 assertEq(ErrorToString(fn2), "blerch: fnord");
 
 var fn3 = function() {};
 fn3.message = "";
-assertEq(ErrorToString(fn3), "fn3");
+assertEq(ErrorToString(fn3), "");
 
 /******************************************************************************/
 
 if (typeof reportCompare === "function")
   reportCompare(true, true);
 
 print("Tests complete!");
deleted file mode 100644
--- a/js/src/tests/ecma_6/Function/name.js
+++ /dev/null
@@ -1,97 +0,0 @@
-// Anonymous functions should take on the name of
-// their arguments.
-let foo = () => 1;
-let bar = foo;
-
-assertEq(foo.name, "foo");
-assertEq(bar.name, "foo");
-
-// Anonymous generator functions should behave the same.
-let gen = function *() {};
-assertEq(gen.name, "gen");
-
-// Extra parenthesis should have no effects.
-let abc = ((((function () {}))));
-assertEq(abc.name, "abc");
-
-// In multiple assignments, the inner most should become the name.
-let a, b, c;
-a = b = c = function () {};
-assertEq(a.name, "c");
-assertEq(b.name, "c");
-assertEq(c.name, "c");
-
-// In syntactic contexts outside of direct assignment no name is
-// assigned.
-let q = (0, function(){});
-assertEq(q.name, "");
-
-let yabba, dabba;
-[yabba, dabba] = [() => 1, function(){}];
-assertEq(yabba.name, "");
-assertEq(dabba.name, "");
-
-// Methods should also have a name property
-let obj = {
-    wubba: function() {},                   // "wubba"
-    1: ()=>"lubba",                         // "1"
-    dubdub: function*(){},                  // "dubdub"
-    noOverride: function named (){},        // "named"
-    obj2: {obj3: {funky: () => 1}},         // "funky"
-    "1 + 2": () => 1,                       // "1 + 2"
-    '" \\ \n \t \v \r \b \f \0': () => 1,   // "\" \n \t \v \r \x00"
-    "\u9999": () => 1,                      // "\u9999"
-    "\u9999 ": () => 1,                     // "\u9999 "
-    "\u{999} ": () => 1,                    // "\u0999 "
-    "\u{999}": () => 1,                     // "\u0999"
-    "\x111234": () => 1,                    // "\x111234"
-    inner: {'["\\bob2 - 6"]': () => 1},     // "[\"\\bob2 - 6\"]"
-    "99 []": {"20 - 16": {rrr: () => 1}},   // "rrr"
-    "'": () => 1,                           // "'"
-}
-
-assertEq(obj.wubba.name, "wubba");
-assertEq(obj[1].name, "1");
-assertEq(obj.dubdub.name, "dubdub");
-assertEq(obj.noOverride.name, "named");
-assertEq(obj.obj2.obj3.funky.name, "funky");
-assertEq(obj["1 + 2"].name, "1 + 2");
-assertEq(obj['" \\ \n \t \v \r \b \f \0'].name, "\" \\ \n \t \v \r \b \f \0");
-assertEq(obj["\u9999"].name, "\u9999");
-assertEq(obj["\u9999 "].name, "\u9999 ");
-assertEq(obj["\u{999}"].name, "\u{999}");
-assertEq(obj["\u{999} "].name, "\u{999} ");
-assertEq(obj["\x111234"].name, "\x111234");
-assertEq(obj.inner['["\\bob2 - 6"]'].name, '["\\bob2 - 6"]');
-assertEq(obj["99 []"]["20 - 16"].rrr.name, "rrr");
-assertEq(obj["'"].name, "'");
-
-// Anonymous objects with methods.
-assertEq(({a: () => 1}).a.name, "a");
-assertEq(({"[abba]": {3: () => 1 }})["[abba]"][3].name, "3");
-assertEq(({"[abba]": () => 1})["[abba]"].name, "[abba]");
-
-// The method retains its name when assigned.
-let zip = obj.wubba;
-assertEq(zip.name, "wubba");
-
-// (Bug 1282332) Accessed as a property based on a function name
-// This creates a tricky display name of the form: x[y[0]].
-let idaho = {0: () => 1};
-
-let planetz = {};
-planetz[idaho[0]] = () => 1;
-assertEq(planetz[idaho[0]].name, "");
-
-let moya = {};
-moya[planetz[idaho[0]]] =  () => 1;
-assertEq(moya[planetz[idaho[0]]].name, "");
-
-
-// Bound function names
-function bound() {};
-assertEq(bound.name, "bound");
-assertEq(bound.bind(Object).name, "bound bound");
-assertEq((function(){}).bind(function(){}).name, "bound ");
-
-reportCompare(0, 0, 'ok');