Bug 913445 - Print something less confusing than "null" for non-stringifiable values in the shell. r=luke.

js/src/builtin/Object.cpp

@@ -87,8 +87,8 @@ obj_propertyIsEnumerable(JSContext *cx, unsigned argc, Value *vp)
 }
 
 #if JS_HAS_TOSOURCE
-bool
-js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
+static bool
+obj_toSource(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     JS_CHECK_RECURSION(cx, return false);
@@ -97,25 +97,31 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
     if (!obj)
         return false;
 
+    JSString *str = ObjectToSource(cx, obj);
+    if (!str)
+        return false;
+
+    args.rval().setString(str);
+    return true;
+}
+
+JSString *
+js::ObjectToSource(JSContext *cx, HandleObject obj)
+{
     /* If outermost, we need parentheses to be an expression, not a block. */
     bool outermost = (cx->cycleDetectorSet.count() == 0);
 
     AutoCycleDetector detector(cx, obj);
     if (!detector.init())
-        return false;
-    if (detector.foundCycle()) {
-        JSString *str = js_NewStringCopyZ<CanGC>(cx, "{}");
-        if (!str)
-            return false;
-        args.rval().setString(str);
-        return true;
-    }
+        return NULL;
+    if (detector.foundCycle())
+        return js_NewStringCopyZ<CanGC>(cx, "{}");
 
     StringBuffer buf(cx);
     if (outermost && !buf.append('('))
-        return false;
+        return NULL;
     if (!buf.append('{'))
-        return false;
+        return NULL;
 
     RootedValue v0(cx), v1(cx);
     MutableHandleValue val[2] = {&v0, &v1};
@@ -125,7 +131,7 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
 
     AutoIdVector idv(cx);
     if (!GetPropertyNames(cx, obj, JSITER_OWNONLY, &idv))
-        return false;
+        return NULL;
 
     bool comma = false;
     for (size_t i = 0; i < idv.length(); ++i) {
@@ -133,7 +139,7 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
         RootedObject obj2(cx);
         RootedShape shape(cx);
         if (!JSObject::lookupGeneric(cx, obj, id, &obj2, &shape))
-            return false;
+            return NULL;
 
         /*  Decide early whether we prefer get/set or old getter/setter syntax. */
         int valcnt = 0;
@@ -158,7 +164,7 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
                 valcnt = 1;
                 gsop[0].set(NULL);
                 if (!JSObject::getGeneric(cx, obj, obj, id, val[0]))
-                    return false;
+                    return NULL;
             }
         }
 
@@ -166,10 +172,10 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
         RootedValue idv(cx, IdToValue(id));
         JSString *s = ToString<CanGC>(cx, idv);
         if (!s)
-            return false;
+            return NULL;
         Rooted<JSLinearString*> idstr(cx, s->ensureLinear(cx));
         if (!idstr)
-            return false;
+            return NULL;
 
         /*
          * If id is a string that's not an identifier, or if it's a negative
@@ -181,7 +187,7 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
         {
             s = js_QuoteString(cx, idstr, jschar('\''));
             if (!s || !(idstr = s->ensureLinear(cx)))
-                return false;
+                return NULL;
         }
 
         for (int j = 0; j < valcnt; j++) {
@@ -195,10 +201,10 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
             /* Convert val[j] to its canonical source form. */
             RootedString valstr(cx, ValueToSource(cx, val[j]));
             if (!valstr)
-                return false;
+                return NULL;
             const jschar *vchars = valstr->getChars(cx);
             if (!vchars)
-                return false;
+                return NULL;
             size_t vlength = valstr->length();
 
             /*
@@ -237,33 +243,29 @@ js::obj_toSource(JSContext *cx, unsigned argc, Value *vp)
             }
 
             if (comma && !buf.append(", "))
-                return false;
+                return NULL;
             comma = true;
 
             if (gsop[j])
                 if (!buf.append(gsop[j]) || !buf.append(' '))
-                    return false;
+                    return NULL;
 
             if (!buf.append(idstr))
-                return false;
+                return NULL;
             if (!buf.append(gsop[j] ? ' ' : ':'))
-                return false;
+                return NULL;
 
             if (!buf.append(vchars, vlength))
-                return false;
+                return NULL;
         }
     }
 
     if (!buf.append('}'))
-        return false;
+        return NULL;
     if (outermost && !buf.append(')'))
-        return false;
+        return NULL;
 
-    JSString *str = buf.finishString();
-    if (!str)
-        return false;
-    args.rval().setString(str);
-    return true;
+    return buf.finishString();
 }
 #endif /* JS_HAS_TOSOURCE */
 

js/src/builtin/Object.h

@@ -15,12 +15,14 @@ extern const JSFunctionSpec object_methods[];
 extern const JSFunctionSpec object_static_methods[];
 
 // Object constructor native. Exposed only so the JIT can know its address.
-extern bool
+bool
 obj_construct(JSContext *cx, unsigned argc, js::Value *vp);
 
-// Object.prototype.toSource. Exposed so that Function.prototype.toSource can chain up.
-extern bool
-obj_toSource(JSContext *cx, unsigned argc, js::Value *vp);
+#if JS_HAS_TOSOURCE
+// Object.prototype.toSource. Function.prototype.toSource and uneval use this.
+JSString *
+ObjectToSource(JSContext *cx, HandleObject obj);
+#endif // JS_HAS_TOSOURCE
 
 } /* namespace js */
 

js/src/jit-test/tests/basic/bug913445.js

@@ -0,0 +1,17 @@
+// uneval works on objects with no callable .toSource method.
+
+var obj = Object.create(null);
+assertEq(uneval(obj), "({})");
+assertEq(Function.prototype.toSource.call(obj), "({})");
+obj.x = 1;
+obj.y = 2;
+assertEq(uneval(obj), "({x:1, y:2})");
+
+var d = new Date();
+delete Date.prototype.toSource;
+assertEq(uneval(d), "({})");
+
+delete Object.prototype.toSource;
+assertEq(uneval({p: 2+2}), "({p:4})");
+
+assertEq(uneval({toSource: [0]}), "({toSource:[0]})");

js/src/jsfun.cpp

@@ -829,13 +829,14 @@ fun_toSource(JSContext *cx, unsigned argc, Value *vp)
     if (!obj)
         return false;
 
-    if (!obj->is<JSFunction>() && !obj->is<FunctionProxyObject>())
-        return obj_toSource(cx, argc, vp);
+    RootedString str(cx);
+    if (obj->is<JSFunction>() || obj->is<FunctionProxyObject>())
+        str = fun_toStringHelper(cx, obj, JS_DONT_PRETTY_PRINT);
+    else
+        str = ObjectToSource(cx, obj);
 
-    RootedString str(cx, fun_toStringHelper(cx, obj, JS_DONT_PRETTY_PRINT));
     if (!str)
         return false;
-
     args.rval().setString(str);
     return true;
 }

js/src/jsstr.cpp

@@ -3907,17 +3907,18 @@ js::ValueToSource(JSContext *cx, HandleValue v)
         return ToString<CanGC>(cx, v);
     }
 
-    RootedValue rval(cx, NullValue());
     RootedValue fval(cx);
     RootedObject obj(cx, &v.toObject());
     if (!JSObject::getProperty(cx, obj, obj, cx->names().toSource, &fval))
         return NULL;
     if (js_IsCallable(fval)) {
+        RootedValue rval(cx);
         if (!Invoke(cx, ObjectValue(*obj), fval, 0, NULL, &rval))
             return NULL;
+        return ToString<CanGC>(cx, rval);
     }
 
-    return ToString<CanGC>(cx, rval);
+    return ObjectToSource(cx, obj);
 }
 
 JSString *