Subj : Re: why does it not print Hello twice?
To   : netscape.public.mozilla.jseng
From : Brendan Eich <brendan@meer.net>
Date : Thu Apr 15 2004 10:49 am

Sterling Bates wrote:
> Brendan Eich wrote:
> 
>> -    fun = js_ValueToFunction(cx, &argv[1], 0);
>> -    if (!fun)
>> -        return JS_FALSE;
>> -    argv[1] = OBJECT_TO_JSVAL(fun->object);
>> +    if (JSVAL_IS_FUNCTION(cx, argv[1])) {
>> +        funobj = JSVAL_TO_OBJECT(argv[1]);
>> +    } else {
>> +        fun = js_ValueToFunction(cx, &argv[1], 0);
>> +        if (!fun)
>> +            return JS_FALSE;
>> +        funobj = fun->object;
>> +    }
>> +    argv[1] = OBJECT_TO_JSVAL(funobj);
> 
> 
> For the junior padawans among us: where did js_ValueToFunction() break 
> the code in this case?  Was it the statement at
>     http://lxr.mozilla.org/mozilla/source/js/src/jsfun.c#2028
> ?
> 
> Aside from JS_GetPrivate, the js_ValueToFunction code seems to do 
> exactly what the patch does above (as far as Jens' testcase goes).


The problem lay elsewhere (I've checked in the fix already).  Note that 
fun->object is a 1:1 mapping from a private data structure to one of the 
objects sharing it, but JS_GetPrivate may map N:1 objects to private 
JSFunction data structures.

Cloned function objects all share a reference in the original, 
compiler-created function object's JSFunction private.  This is 
necessary to give each clone a separate parent slot (ECMA-262 Edition 3 
calls this the [[Scope]] internal property).

The original engine had no cloned function objects, so old code such as 
obj_watch used fun->object to map in reverse, given an object-tagged 
jsval that it converted to a function private datum.

In the clone wars world of today ;-), that's just wrong.  In fact, 
js_ValueToFunction should be considered harmful, and we should scour the 
code for abuses of it, where the caller then uses fun->object given fun 
returned by it.

To be completely clear, in the patch hunk above, argv[1] is a jsval 
referring to a clone, not the original compiler-created function object 
(fun->object).  So if it's already of Function class, we should use it 
and not call js_ValueToFunction | fun->object.

/be

.