Bug 913445 - Print something less confusing than "null" for non-stringifiable values in the shell. r=luke.
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 06 Sep 2013 21:41:26 -0500
changeset 158964 1a9a72fbdc59c632184c0c9170cedb2b4c7a0145
parent 158963 b4b1369c759540b56e4ba1528a136913d61ebb31
child 158965 5c2a0f1510bcada4acb0f123b75a647616a4869a
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs913445
milestone26.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 913445 - Print something less confusing than "null" for non-stringifiable values in the shell. r=luke.
js/src/builtin/Object.cpp
js/src/builtin/Object.h
js/src/jit-test/tests/basic/bug913445.js
js/src/jsfun.cpp
js/src/jsstr.cpp
--- a/js/src/builtin/Object.cpp
+++ b/js/src/builtin/Object.cpp
@@ -82,63 +82,69 @@ obj_propertyIsEnumerable(JSContext *cx, 
     if (!JSObject::getGenericAttributes(cx, pobj, id, &attrs))
         return false;
 
     args.rval().setBoolean((attrs & JSPROP_ENUMERATE) != 0);
     return true;
 }
 
 #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);
 
     RootedObject obj(cx, ToObject(cx, args.thisv()));
     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};
 
     RootedString str0(cx), str1(cx);
     MutableHandleString gsop[2] = {&str0, &str1};
 
     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) {
         RootedId id(cx, idv[i]);
         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;
         if (shape) {
             bool doGet = true;
             if (obj2->isNative() && !IsImplicitDenseElement(shape)) {
                 unsigned attrs = shape->attributes();
                 if (attrs & JSPROP_GETTER) {
@@ -153,57 +159,57 @@ js::obj_toSource(JSContext *cx, unsigned
                     gsop[valcnt].set(cx->names().set);
                     valcnt++;
                 }
             }
             if (doGet) {
                 valcnt = 1;
                 gsop[0].set(NULL);
                 if (!JSObject::getGeneric(cx, obj, obj, id, val[0]))
-                    return false;
+                    return NULL;
             }
         }
 
         /* Convert id to a linear string. */
         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
          * integer, then it must be quoted.
          */
         if (JSID_IS_ATOM(id)
             ? !IsIdentifier(idstr)
             : (!JSID_IS_INT(id) || JSID_TO_INT(id) < 0))
         {
             s = js_QuoteString(cx, idstr, jschar('\''));
             if (!s || !(idstr = s->ensureLinear(cx)))
-                return false;
+                return NULL;
         }
 
         for (int j = 0; j < valcnt; j++) {
             /*
              * Censor an accessor descriptor getter or setter part if it's
              * undefined.
              */
             if (gsop[j] && val[j].isUndefined())
                 continue;
 
             /* 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();
 
             /*
              * Remove '(function ' from the beginning of valstr and ')' from the
              * end so that we can put "get" in front of the function definition.
              */
             if (gsop[j] && IsFunctionObject(val[j])) {
                 const jschar *start = vchars;
@@ -232,43 +238,39 @@ js::obj_toSource(JSContext *cx, unsigned
                     vlength = end - vchars - parenChomp;
                 } else {
                     gsop[j].set(NULL);
                     vchars = start;
                 }
             }
 
             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 */
 
 JSString *
 JS_BasicObjectToString(JSContext *cx, HandleObject obj)
 {
     const char *className = JSObject::className(cx, obj);
 
--- a/js/src/builtin/Object.h
+++ b/js/src/builtin/Object.h
@@ -10,18 +10,20 @@
 #include "jsobj.h"
 
 namespace js {
 
 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 */
 
 #endif /* builtin_Object_h */
new file mode 100644
--- /dev/null
+++ b/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]})");
--- a/js/src/jsfun.cpp
+++ b/js/src/jsfun.cpp
@@ -824,23 +824,24 @@ fun_toSource(JSContext *cx, unsigned arg
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     JS_ASSERT(IsFunctionObject(args.calleev()));
 
     RootedObject obj(cx, ToObject(cx, args.thisv()));
     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;
 }
 #endif
 
 bool
 js_fun_call(JSContext *cx, unsigned argc, Value *vp)
 {
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -3902,27 +3902,28 @@ js::ValueToSource(JSContext *cx, HandleV
             /* NB: _ucNstr rather than _ucstr to indicate non-terminated. */
             static const jschar js_negzero_ucNstr[] = {'-', '0'};
 
             return js_NewStringCopyN<CanGC>(cx, js_negzero_ucNstr, 2);
         }
         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 *
 js::StringToSource(JSContext *cx, JSString *str)
 {
     return js_QuoteString(cx, str, '"');
 }