[Webkit-unassigned] [Bug 7831] .apply/call does not set this for eval

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=7831


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7146|review?                     |review-
               Flag|                            |




------- Comment #7 from darin at apple.com  2006-03-19 16:30 PDT -------
(From update of attachment 7146)
Thanks for taking this on!

There are many minor things wrong with this patch.

1) no tests -- patches that fix bugs require tests that demonstrate the bug is
fixed (although I suppose the fact that this fixes on of the Mozilla tests
might be an OK substitute)
2) no change log entry
3) commented-out code left in internal.cpp, should not be
4) patch that changes only whitespace in function.cpp
5) tabs in the patch (we use spaces for indenting)
6) no space between "if" and the "(" following it in the code in
function_object.cpp
7) no spaces around the "==" operator
8) "ContextImp *cpy= " should be "ContextImp* cpy = ".
9) We don't declare multiple variables with a single declaration as is done
with "cpy" and "ctx".
10) local variable named "cpy" -- "copy" would be better
11) local variable named "ctx" -- name that is either a word or just a single
letter would be better
12) no spaces after commas in some function calls

However, the main issue here is almost certainly performance. Adding an
inherits check to every function call is probably not a good idea. And as to
the reason that's needed, I don't see what information
FunctionProtoFunc::callAsFunction has that GlobalFuncImp::callAsFunction will
not have.

Won't the internal.cpp change fix the bug without any change to
function_object.cpp at all? If not, what's the high level description of what
the function_object.cpp change does?


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.