From c517c41b44e2120c0ac19f473932d88565250cbf Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Fri, 6 Sep 2013 21:41:26 -0500 Subject: [PATCH] Bug 913445 - Print something less confusing than "null" for non-stringifiable values in the shell. r=luke. --- js/src/builtin/Object.cpp | 66 ++++++++++++------------ js/src/builtin/Object.h | 10 ++-- js/src/jit-test/tests/basic/bug913445.js | 17 ++++++ js/src/jsfun.cpp | 9 ++-- js/src/jsstr.cpp | 5 +- 5 files changed, 65 insertions(+), 42 deletions(-) create mode 100644 js/src/jit-test/tests/basic/bug913445.js diff --git a/js/src/builtin/Object.cpp b/js/src/builtin/Object.cpp index e449f51087b..c2970246e79 100644 --- a/js/src/builtin/Object.cpp +++ b/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(cx, "{}"); - if (!str) - return false; - args.rval().setString(str); - return true; - } + return NULL; + if (detector.foundCycle()) + return js_NewStringCopyZ(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(cx, idv); if (!s) - return false; + return NULL; Rooted 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 */ diff --git a/js/src/builtin/Object.h b/js/src/builtin/Object.h index 892dce5c219..f49dfa69799 100644 --- a/js/src/builtin/Object.h +++ b/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 */ diff --git a/js/src/jit-test/tests/basic/bug913445.js b/js/src/jit-test/tests/basic/bug913445.js new file mode 100644 index 00000000000..6b255914c6a --- /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]})"); diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index 0ef6152f4a2..fb409bb5155 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -829,13 +829,14 @@ fun_toSource(JSContext *cx, unsigned argc, Value *vp) if (!obj) return false; - if (!obj->is() && !obj->is()) - return obj_toSource(cx, argc, vp); + RootedString str(cx); + if (obj->is() || obj->is()) + 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; } diff --git a/js/src/jsstr.cpp b/js/src/jsstr.cpp index 9aabec2a79b..b0e66a93709 100644 --- a/js/src/jsstr.cpp +++ b/js/src/jsstr.cpp @@ -3907,17 +3907,18 @@ js::ValueToSource(JSContext *cx, HandleValue v) return ToString(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(cx, rval); } - return ToString(cx, rval); + return ObjectToSource(cx, obj); } JSString *