Bug 645416, part 14 - Update ToString for symbols. r=sfink.

The change in jit-test/tests/symbol/toString.js is that we now check that an
exception is actually thrown. Until this patch, stringifying a symbol did not
throw. (The test was mainly checking that we did not assert in Ion.)

No changes in Ion. If a symbol is being stringified, it's ok to be in a slow
path because that is going to throw anyway.

--HG--
extra : rebase_source : 9cf314dafa7392a20fee9d3b5acc4ad7fc1c5229

js/src/builtin/SymbolObject.cpp

@@ -6,10 +6,13 @@
 
 #include "builtin/SymbolObject.h"
 
+#include "vm/StringBuffer.h"
+
 #include "jsobjinlines.h"
 
 #include "vm/Symbol-inl.h"
 
+using JS::Symbol;
 using namespace js;
 
 const Class SymbolObject::class_ = {
@@ -40,6 +43,7 @@ const JSPropertySpec SymbolObject::properties[] = {
 };
 
 const JSFunctionSpec SymbolObject::methods[] = {
+    JS_FN(js_toString_str, toString, 0, 0),
     JS_FS_END
 };
 
@@ -135,6 +139,50 @@ SymbolObject::for_(JSContext *cx, unsigned argc, Value *vp)
     return true;
 }
 
+MOZ_ALWAYS_INLINE bool
+IsSymbol(HandleValue v)
+{
+    return v.isSymbol() || (v.isObject() && v.toObject().is<SymbolObject>());
+}
+
+//ES6 rev 24 (2014 Apr 27) 19.4.3.2
+bool
+SymbolObject::toString_impl(JSContext *cx, CallArgs args)
+{
+    // steps 1-3
+    HandleValue thisv = args.thisv();
+    JS_ASSERT(IsSymbol(thisv));
+    Rooted<Symbol*> sym(cx, thisv.isSymbol()
+                            ? thisv.toSymbol()
+                            : thisv.toObject().as<SymbolObject>().unbox());
+
+    // steps 4-7
+    StringBuffer sb(cx);
+    if (!sb.append("Symbol("))
+        return false;
+    RootedString str(cx, sym->description());
+    if (str) {
+        if (!sb.append(str))
+            return false;
+    }
+    if (!sb.append(')'))
+        return false;
+
+    // step 8
+    str = sb.finishString();
+    if (!str)
+        return false;
+    args.rval().setString(str);
+    return true;
+}
+
+bool
+SymbolObject::toString(JSContext *cx, unsigned argc, Value *vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+    return CallNonGenericMethod<IsSymbol, toString_impl>(cx, args);
+}
+
 JSObject *
 js_InitSymbolClass(JSContext *cx, HandleObject obj)
 {

js/src/builtin/SymbolObject.h

@@ -45,6 +45,10 @@ class SymbolObject : public JSObject
     // Static methods.
     static bool for_(JSContext *cx, unsigned argc, Value *vp);
 
+    // Methods defined on Symbol.prototype.
+    static bool toString_impl(JSContext *cx, CallArgs args);
+    static bool toString(JSContext *cx, unsigned argc, Value *vp);
+
     static const JSPropertySpec properties[];
     static const JSFunctionSpec methods[];
     static const JSFunctionSpec staticMethods[];

js/src/jit-test/tests/debug/Frame-evalWithBindings-02.js

@@ -1,4 +1,5 @@
-// evalWithBindings to call a method of a debuggee object
+// evalWithBindings to call a method of a debuggee value
+
 var g = newGlobal();
 var dbg = new Debugger;
 var global = dbg.addDebuggee(g);
@@ -14,5 +15,7 @@ g.eval("function f(obj, expected) { debugger; }");
 
 g.eval("f(new Number(-0), '0');");
 g.eval("f(new String('ok'), 'ok');");
+g.eval("f(Symbol('still ok'), 'Symbol(still ok)');");
+g.eval("f(Object(Symbol('still ok')), 'Symbol(still ok)');");
 g.eval("f({toString: function () { return f; }}, f);");
-assertEq(hits, 3);
+assertEq(hits, 5);

js/src/jit-test/tests/symbol/toString.js

@@ -1,8 +1,11 @@
 // ToString(symbol) throws a TypeError.
 
-var obj;
-for (var i = 0; i < 10; i++) {
+var N = 10, obj, hits = 0;
+for (var i = 0; i < N; i++) {
     try {
         obj = new String(Symbol());
-    } catch (exc) {}
+    } catch (exc) {
+        hits++;
+    }
 }
+assertEq(hits, N);

js/src/js.msg

@@ -236,7 +236,7 @@ MSG_DEF(JSMSG_BAD_GENERATOR_SEND,     182, 1, JSEXN_TYPEERR, "attempt to send {0
 MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE,    183, 0, JSEXN_TYPEERR, "invalid transferable array for structured clone")
 MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE,    184, 0, JSEXN_TYPEERR, "duplicate transferable for structured clone")
 MSG_DEF(JSMSG_CANT_REPORT_AS_NON_EXTENSIBLE, 185, 0, JSEXN_TYPEERR, "proxy can't report an extensible object as non-extensible")
-MSG_DEF(JSMSG_UNUSED186,              186, 0, JSEXN_NONE, "")
+MSG_DEF(JSMSG_SYMBOL_TO_STRING,       186, 0, JSEXN_TYPEERR, "can't convert symbol to string")
 MSG_DEF(JSMSG_UNUSED187,              187, 0, JSEXN_NONE, "")
 MSG_DEF(JSMSG_INCOMPATIBLE_METHOD,    188, 3, JSEXN_TYPEERR, "{0} {1} called on incompatible {2}")
 MSG_DEF(JSMSG_UNUSED189,              189, 0, JSEXN_NONE, "")

js/src/jsarray.cpp

@@ -989,11 +989,14 @@ ArrayJoinKernel(JSContext *cx, SeparatorOp sepOp, HandleObject obj, uint32_t len
             } else if (elem.isBoolean()) {
                 if (!BooleanToStringBuffer(elem.toBoolean(), sb))
                     return false;
-            } else if (elem.isObject()) {
+            } else if (elem.isObject() || elem.isSymbol()) {
                 /*
                  * Object stringifying could modify the initialized length or make
                  * the array sparse. Delegate it to a separate loop to keep this
                  * one tight.
+                 *
+                 * Symbol stringifying is a TypeError, so into the slow path
+                 * with those as well.
                  */
                 break;
             } else {

js/src/jsstr.cpp

@@ -4494,7 +4494,14 @@ js::ToStringSlow(ExclusiveContext *cx, typename MaybeRooted<Value, allowGC>::Han
         str = js_BooleanToString(cx, v.toBoolean());
     } else if (v.isNull()) {
         str = cx->names().null;
+    } else if (v.isSymbol()) {
+        if (cx->shouldBeJSContext() && allowGC) {
+            JS_ReportErrorNumber(cx->asJSContext(), js_GetErrorMessage, nullptr,
+                                 JSMSG_SYMBOL_TO_STRING);
+        }
+        return nullptr;
     } else {
+        MOZ_ASSERT(v.isUndefined());
         str = cx->names().undefined;
     }
     return str;

js/src/tests/ecma_6/Symbol/constructor.js

@@ -1,7 +1,28 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/licenses/publicdomain/ */
 
-// More tests will be added here when Symbol.prototype.toString is added.
+// Symbol(symbol) throws a TypeError.
+var sym = Symbol();
+assertThrowsInstanceOf(() => Symbol(sym), TypeError);
+
+// Symbol(undefined) is equivalent to Symbol().
+assertEq(Symbol(undefined).toString(), "Symbol()");
+
+// Otherwise, Symbol(v) means Symbol(ToString(v)).
+assertEq(Symbol(7).toString(), "Symbol(7)");
+assertEq(Symbol(true).toString(), "Symbol(true)");
+assertEq(Symbol(null).toString(), "Symbol(null)");
+assertEq(Symbol([1, 2]).toString(), "Symbol(1,2)");
+
+var hits = 0;
+var obj = {
+    toString: function () {
+        hits++;
+        return "ponies";
+    }
+};
+assertEq(Symbol(obj).toString(), "Symbol(ponies)");
+assertEq(hits, 1);
 
 assertEq(Object.getPrototypeOf(Symbol.prototype), Object.prototype);
 

js/src/tests/ecma_6/Symbol/conversions.js

@@ -17,6 +17,13 @@ for (var sym of symbols) {
     // 7.1.3 ToNumber
     assertEq(+sym, NaN);
     assertEq(sym | 0, 0);
+
+    // 7.1.12 ToString
+    assertThrowsInstanceOf(() => String(sym), TypeError);
+    assertThrowsInstanceOf(() => "" + sym, TypeError);
+    assertThrowsInstanceOf(() => sym + "", TypeError);
+    assertThrowsInstanceOf(() => "" + [1, 2, Symbol()], TypeError);
+    assertThrowsInstanceOf(() => ["simple", "thimble", Symbol()].join(), TypeError);
 }
 
 if (typeof reportCompare === "function")

js/src/tests/ecma_6/Symbol/realms.js

@@ -21,6 +21,12 @@ if (typeof newGlobal === "function") {
     g.smith = smith;  // put smith into the realm
     assertEq(g.smith, smith);  // pull it back out
 
+    // Spot-check that non-generic methods can be applied to symbols and Symbol
+    // objects from other realms.
+    assertEq(Symbol.prototype.toString.call(gj), "Symbol(jones)");
+    assertEq(Symbol.prototype.toString.call(g.eval("Object(Symbol('brown'))")),
+             "Symbol(brown)");
+
     // Symbol.for functions share a symbol registry across all realms.
     assertEq(g.Symbol.for("ponies"), Symbol.for("ponies"));
     assertEq(g.eval("Symbol.for('rainbows')"), Symbol.for("rainbows"));

js/src/tests/ecma_6/Symbol/surfaces.js

@@ -22,6 +22,7 @@ assertEq(desc.enumerable, false);
 assertEq(desc.writable, true);
 
 assertEq(Symbol.for.length, 1);
+assertEq(Symbol.prototype.toString.length, 0);
 
 if (typeof reportCompare === "function")
     reportCompare(0, 0);

js/src/tests/ecma_6/Symbol/toString.js

@@ -0,0 +1,27 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/ */
+
+var cases = [
+    {sym: Symbol(), str: "Symbol()"},
+    {sym: Symbol("ok"), str: "Symbol(ok)"},
+    {sym: Symbol("\0"), str: "Symbol(\0)"},
+    {sym: Symbol.iterator, str: "Symbol(Symbol.iterator)"},
+    {sym: Symbol.for("dummies"), str: "Symbol(dummies)"}
+];
+
+// Symbol.prototype.toString works on both primitive symbols and Symbol
+// objects.
+for (var test of cases) {
+    assertEq(test.sym.toString(), test.str);
+    assertEq(Object(test.sym).toString(), test.str);
+}
+
+// Any other value throws.
+var nonsymbols = [
+    undefined, null, "not-ok", new String("still-not-ok"), {}, []
+];
+for (var nonsym of nonsymbols)
+    assertThrowsInstanceOf(() => Symbol.prototype.toString.call(nonsym), TypeError);
+
+if (typeof reportCompare === "function")
+    reportCompare(0, 0);

js/src/vm/StringBuffer.cpp

@@ -144,6 +144,10 @@ js::ValueToStringBufferSlow(JSContext *cx, const Value &arg, StringBuffer &sb)
         return BooleanToStringBuffer(v.toBoolean(), sb);
     if (v.isNull())
         return sb.append(cx->names().null);
+    if (v.isSymbol()) {
+        JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_SYMBOL_TO_STRING);
+        return false;
+    }
     JS_ASSERT(v.isUndefined());
     return sb.append(cx->names().undefined);
 }