Bug 1289749 - Backout the existing function.name implementation; r=jorendorff, a=gchang
authorMorgan Phillips <winter2718@gmail.com>
Wed, 27 Jul 2016 15:40:32 -0700
changeset 340181 fcdf4bb703567bca5a5d7065f3a8a35ce1dea9ff
parent 340180 0bef242afe57fc6bfcce47e726eae5d209ca751e
child 340182 d2f65d99fd51e5ee2f34d892115cc5d6c39a8ea9
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, gchang
bugs1289749
milestone49.0a2
Bug 1289749 - Backout the existing function.name implementation; r=jorendorff, a=gchang 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;
         }
 
@@ -1484,191 +1478,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');