560126 - funobj is uselessly set in WrapFunction

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[timeless]

412 WrapFunction(JSContext *cx, JSObject *scope, JSObject *funobj, jsval *rval)

419   if (native == XPC_COW_FunctionWrapper) {

426     funobj = JSVAL_TO_OBJECT(funobjVal);
427   }
428 
429   JSFunction *funWrapper =
430     JS_NewFunction(cx, XPC_COW_FunctionWrapper,
431                    JS_GetFunctionArity(wrappedFun), 0,
432                    scope,
433                    JS_GetFunctionName(wrappedFun));
434   if (!funWrapper) {
435     return JS_FALSE;
436   }
437 
438   JSObject *funWrapperObj = JS_GetFunctionObject(funWrapper);
439   *rval = OBJECT_TO_JSVAL(funWrapperObj);
440 
441   return JS_SetReservedSlot(cx, funWrapperObj,
442                             XPCWrapper::eWrappedFunctionSlot,
443                             funobjVal);
444 }

Comment 1

[Boris Zbarsky [:bzbarsky]]

Should we be using that (but fail to) or is this just a set that shouldn't be there?

Requesting blocking in case it's the former...

Comment 2

[timeless]

Attachment: proposal

so, jsvals are aliases to jsobjects. i'm hoping it's just someone trying to keep the function argument in sync with the local jsobject. I'd rather we have a DO_NOT_USE(function_argument) statement at the point where it's stale and rely on static analysis to complain if it's used. The current approach risks compilers not optimizing away the useless bookkeeping we have. -- Assuming I'm right that it's useless (instead of errantly unused).

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 439863 [details] [diff] [review]
proposal

I think that I might have wanted to re-load wrappedFun out of funobj (as well as just keep it updated), but since we only use wrappedFun for its name and arity, this is fine.

Comment 4

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/996419229468