Add CallAsFunction method to the Object class in the API

Add CallAsFunction method to the Object class in the API

Patch by Peter Varga.

BUG=v8:1336
TEST=cctest/test-api/CallAsFunction

Committed: http://code.google.com/p/v8/source/detail?r=7781

[Mads Ager (chromium), 2011-04-28 11:53:05]

Adding this makes sense. I haven't seen a need for it yet though. Is this change
motivated by a use case or is it just because you think it is nicer for
consistency?

Normally I would let these sort of changes be driven by an actual need.

http://codereview.chromium.org/6883045/diff/1/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/6883045/diff/1/include/v8.h#newcode1688
include/v8.h:1688: V8EXPORT Local<Value> Call(Handle<Object> recv,
Please add a comment explaining when it makes sense to call an object as a
function. 

For symmetry with the SetCallAsFunctionHandler name, maybe call the method
CallAsFunction?

http://codereview.chromium.org/6883045/diff/1/src/api.cc
File src/api.cc (right):

http://codereview.chromium.org/6883045/diff/1/src/api.cc#newcode3186
src/api.cc:3186: v8::Handle<v8::Value> argv[]) {
Indentation is off.

http://codereview.chromium.org/6883045/diff/1/src/api.cc#newcode3210
src/api.cc:3210: raw_result = *returned;
Please use return scope.Close instead. That will get rid of the local scope here
too.

http://codereview.chromium.org/6883045/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6883045/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:7010: // Check that the call-as-function handler can be
called through
This needs more testing. Please add tests for calling a random object as a
function (which should throw an exception). Please also add tests for calling an
object where the call as function handler throws an exception.

[Peter Varga, 2011-05-03 14:54:45]

We are implementing the QtScript (a JS API for C++) using v8 as the backend. One
of the features that we do support is to Call any callable object(be it a
function or non-function). We get callable non-function objects from our API by
creating objects from a QScriptClass that supports call. Our code basically
wraps Object/FunctionTemplate and SetCallAsFunctionHandler.
Besides that, the functions make sense for v8 API, so we think is a good
addition.

[Mads Ager (chromium), 2011-05-04 09:28:54]

LGTM. I will address a couple of nits in the tests and land.

Thanks for the patch.

http://codereview.chromium.org/6883045/diff/5001/src/execution.cc
File src/execution.cc (right):

http://codereview.chromium.org/6883045/diff/5001/src/execution.cc#newcode237
src/execution.cc:237: Handle<Object>
Execution::TryGetFunctionDelegate(Handle<Object> object,
It is a bit strange that this does not handle the regexp case when the
GetFunctionDelegate method above does. However, we should make regexps
non-callable soon, so I think this is the right thing to do for the API.

http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:7030: "TypeError: Property 'obj2' of object #<Object> is
not a function");
Instead of checking the actual text here I think we should just check that it is
a type error.

http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:7040: CHECK_EQ(*exception_value2, "TypeError: [object
Object] is not a function");
Same here.

[Mads Ager (chromium), 2011-05-04 10:02:44]

Actually nevermind about the nits. This is the style used in the rest of the
file too. I'll land as is.

On 2011/05/04 09:28:54, Mads Ager wrote:
> LGTM. I will address a couple of nits in the tests and land.
> 
> Thanks for the patch.
> 
> http://codereview.chromium.org/6883045/diff/5001/src/execution.cc
> File src/execution.cc (right):
> 
> http://codereview.chromium.org/6883045/diff/5001/src/execution.cc#newcode237
> src/execution.cc:237: Handle<Object>
> Execution::TryGetFunctionDelegate(Handle<Object> object,
> It is a bit strange that this does not handle the regexp case when the
> GetFunctionDelegate method above does. However, we should make regexps
> non-callable soon, so I think this is the right thing to do for the API.
> 
> http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc
> File test/cctest/test-api.cc (right):
> 
>
http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc#newc...
> test/cctest/test-api.cc:7030: "TypeError: Property 'obj2' of object #<Object>
is
> not a function");
> Instead of checking the actual text here I think we should just check that it
is
> a type error.
> 
>
http://codereview.chromium.org/6883045/diff/5001/test/cctest/test-api.cc#newc...
> test/cctest/test-api.cc:7040: CHECK_EQ(*exception_value2, "TypeError: [object
> Object] is not a function");
> Same here.

[Mads Ager (chromium), 2011-05-04 10:51:49]

Reverted this change. The tests fail in debug mode.

[Peter Varga, 2011-05-04 12:04:39]

Remove cast of the Object 'returned' to JSObject in Object::CallAsFunction
method.

[Mads Ager (chromium), 2011-05-04 12:45:06]

Thanks. I'll reland.