Currently in a few places we duplicate a code pattern to allocate reserved slots for Function and Call objects. It would be nice to have a common function for this code. Initially I planned to do this within a patch for bug 495061 but that increased patch complexity there.

The patch adds a new function, js_EnsureReservedSlots, that ensures that reserved slots are allocated. To use it with Function objects I have added js_CountScriptFunctionReserveSlots. That is not a best name but at least it reflects that the function can only be applied to interpreted functions.

Looks like this will collide with a patch Andreas is working on -- Andreas, please take a look. Maybe minor, just js_AllocSlots -> AllocSlots. /be

Comment on attachment 389988 [details] [diff] [review] v1 >+ if (!js_EnsureReservedSlots(cx, clone, >+ js_CountScriptFunctionReserveSlots(fun))) { Really want fun->countReservedSlots(). (Note the 'd' in 'Reserved'). r=me with that, modulo conflict avoidance with gal@uci.edu's pending patch. /be

The updated patch adds JSFunction::countInterpretedReserveSlots(). I have the explicit "Intepreted" in the name to indicate that the method can only be applied for interpreted functions.

To :gal - with which patch this could conflict?

It collides with the GC-ed scope patch and the inline dslots patch, but thats all easy to fix. I wouldn't worry about it. I can easily rebase.

No 'd' in countInterpretedReserveSlots, as requested :-(. The Interpreted name part still seems fruitless to me. The method could return 0 for natives and assert in DEBUG builds that the |this| function is interpreted. Or we could subclass interpreted vs. native functions and put this method only in the former class, but that's for later. /be

http://hg.mozilla.org/tracemonkey/rev/ada08e63ab62 - I have landed before I spotted that bad spelling in method's name. (In reply to comment #7) > The Interpreted name part still seems fruitless to me. The method could return > 0 for natives and assert in DEBUG builds that the |this| function is > interpreted. Returning 0 for native functions would add a useless if-check.

(In reply to comment #8) > http://hg.mozilla.org/tracemonkey/rev/ada08e63ab62 - I have landed before I > spotted that bad spelling in method's name. > > (In reply to comment #7) > > The Interpreted name part still seems fruitless to me. The method could return > > 0 for natives and assert in DEBUG builds that the |this| function is > > interpreted. > > Returning 0 for native functions would add a useless if-check. Ok, then just assert. We do similar things elsewhere. It's not as if the name does anything at compile or runtime to prevent a call on a native function. /be

Anyway, rs=me on s/Reserve/Reserved/ in the function name. /be

misspellings fix - http://hg.mozilla.org/tracemonkey/rev/36fab1f75e34