I was wandering around defaultValue-land today and came across a couple of places where defaultValue is called with hint = JSTYPE_OBJECT or JSTYPE_FUNCTION. Apparently, without liveconnect around our necks, this is no longer needed and they can/should be removed. Uses include (but may not limited to): js_ValueToObject, js_ValueToFunction, fun_toStringHelper, js_fun_call, js_fun_apply

This is in the way of my iterator work.

Comment on attachment 438390 [details] [diff] [review] patch > JS_FRIEND_DATA(JSClass) js_CallClass = { > "Call", > JSCLASS_HAS_PRIVATE | > JSCLASS_HAS_RESERVED_SLOTS(CALL_CLASS_FIXED_RESERVED_SLOTS) | > JSCLASS_NEW_RESOLVE | JSCLASS_IS_ANONYMOUS | JSCLASS_MARK_IS_TRACE, > JS_PropertyStub, JS_PropertyStub, > JS_PropertyStub, JS_PropertyStub, > call_enumerate, (JSResolveOp)call_resolve, >- call_convert, NULL, >+ JS_ConvertStub, NULL, Use NULL not JS_ConvertStub -- we should crash if anyone gets its hands on a Call object and tries to convert it to anything. > js_ValueToFunction(JSContext *cx, jsval *vp, uintN flags) > { > jsval v; > JSObject *obj; > > v = *vp; >+ if (!VALUE_IS_FUNCTION(cx, v)) { > js_ReportIsNotFunction(cx, vp, flags); > return NULL; > } >+ return GET_FUNCTION_PRIVATE(cx, JSVAL_TO_OBJECT(v)); Great -- just wondering whether proxies will change this again. >@@ -358,18 +358,18 @@ js_ValueToIterator(JSContext *cx, uintN > > JS_ASSERT(!(flags & ~(JSITER_ENUMERATE | JSITER_FOREACH | JSITER_KEYVALUE))); > > /* JSITER_KEYVALUE must always come with JSITER_FOREACH */ > JS_ASSERT(!(flags & JSITER_KEYVALUE) || (flags & JSITER_FOREACH)); > > AutoValueRooter tvr(cx); > >- /* XXX work around old valueOf call hidden beneath js_ValueToObject */ > if (!JSVAL_IS_PRIMITIVE(*vp)) { >+ /* Common case. */ > obj = JSVAL_TO_OBJECT(*vp); > } else { > /* > * Enumerating over null and undefined gives an empty enumerator. > * This is contrary to ECMA-262 9.9 ToObject, invoked from step 3 of > * the first production in 12.6.4 and step 4 of the second production, > * but it's "web JS" compatible. Add "ES5 fixed for-in to match this de-facto standard." or words to that effect. ECMA-262 Fifth Edition 12.6.4 step 3 (ignore the "experValue" typos). [js_DefaultValue:] > default: > if (!obj->getClass()->convert(cx, obj, hint, &v)) > return JS_FALSE; > if (!JSVAL_IS_PRIMITIVE(v)) { > JSType type = JS_TypeOfValue(cx, v); >- if (type == hint || >- (type == JSTYPE_FUNCTION && hint == JSTYPE_OBJECT)) { >- goto out; >- } >- if (!js_TryMethod(cx, obj, cx->runtime->atomState.toStringAtom, 0, >- NULL, &v)) { >+ if (type != hint && >+ !js_TryMethod(cx, obj, cx->runtime->atomState.toStringAtom, 0, NULL, &v)) { If type != hint but !JSVAL_IS_PRIMITIVE(v) and type = JS_TypeOfValue(cx, v), then type must be JSTYPE_OBJECT or JSTYPE_FUNCTION. But by the assertion at the top of js_DefaultValue that this patch adds, hint cannot be JSTYPE_OBJECT or JSTYPE_FUNCTION, therefore type != hint is always true. What's really going on here is that we want to fall back on js_TryMethod(...toString...) if convert (AKA tryValueOf) succeeded (didn't throw/report/return-false) but did not give a primitive result (of any primitive type). So maybe just assert that type != hint and try toString. /be

Comment on attachment 438392 [details] [diff] [review] patch r=me, looks great -- but what is up with that test failure you mentioned on IRC? /be

Cite it! :-P http://hg.mozilla.org/tracemonkey/rev/3de0a7da3a8e /be

(In reply to comment #6) > (From update of attachment 438392 [details] [diff] [review]) > r=me, looks great -- but what is up with that test failure you mentioned on > IRC? Anyone? Bueller? /be