Implement implicit instance checks for API accessors.

Implement implicit instance checks for API accessors.

This allows to specify a constructor against which an implicit instance
check is performed for API accessors. If the receiver is incompatible,
an implicit TypeError is thrown and no callback is invoked.

R=svenpanne@chromium.org
BUG=v8:2075
TEST=cctest/test-api/InstanceCheckOn[*]

Committed: https://code.google.com/p/v8/source/detail?r=11734

[Michael Starzinger, 2012-06-01 13:19:42]

@Danno: Could you PTAL at the API changes.
@Sven: Could you PTAL at the implementation.

https://chromiumcodereview.appspot.com/10442129/diff/1/include/v8.h
File include/v8.h (right):

https://chromiumcodereview.appspot.com/10442129/diff/1/include/v8.h#newcode2362
include/v8.h:2362: Handle<FunctionTemplate> constructor =
As discussed offline, we might want to wrap this FunctionTemplate into some
AccessorSignature, because we already have a similar concept for invocation
callbacks.

[danno, 2012-06-01 13:55:56]

I think you should add the AccessorSignature as discussed, but otherwise it's
looking good.

[Sven Panne, 2012-06-04 07:50:32]

LGTM with nits

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

http://codereview.chromium.org/10442129/diff/1/src/api.cc#newcode1073
src/api.cc:1073: if (!constructor.IsEmpty()) {
Not exactly related to this CL, but we should really clean up this naming chaos
(here: IsEmpty vs. is_null)... :-P

http://codereview.chromium.org/10442129/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/10442129/diff/1/src/objects.cc#newcode2002
src/objects.cc:2002: HandleVector(args, 2));
2 => ARRAY_SIZE(args)

http://codereview.chromium.org/10442129/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/10442129/diff/1/src/objects.h#newcode8089
src/objects.h:8089: DECL_ACCESSORS(compatible, Object)
The name "compatible" is a bit unclear, something like "expected_receiver_type"
might be better (although our 80-column rule might hurt then >:-).

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

http://codereview.chromium.org/10442129/diff/1/test/cctest/test-api.cc#newcod...
test/cctest/test-api.cc:16601: printf("InstanceCheckedGetter!\n");
Hmmm, do we really need printf in unit tests?

[Michael Starzinger, 2012-06-04 09:11:15]

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

http://codereview.chromium.org/10442129/diff/1/src/api.cc#newcode1073
src/api.cc:1073: if (!constructor.IsEmpty()) {
On 2012/06/04 07:50:32, Sven Panne wrote:
> Not exactly related to this CL, but we should really clean up this naming
chaos
> (here: IsEmpty vs. is_null)... :-P

Yes. However the IsEmpty method (on the external handle) is part of the external
API. So that is more or less set in stone. We could think about syncing the
internal name with that.

http://codereview.chromium.org/10442129/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/10442129/diff/1/src/objects.cc#newcode2002
src/objects.cc:2002: HandleVector(args, 2));
On 2012/06/04 07:50:32, Sven Panne wrote:
> 2 => ARRAY_SIZE(args)

Done.

http://codereview.chromium.org/10442129/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/10442129/diff/1/src/objects.h#newcode8089
src/objects.h:8089: DECL_ACCESSORS(compatible, Object)
On 2012/06/04 07:50:32, Sven Panne wrote:
> The name "compatible" is a bit unclear, something like
"expected_receiver_type"
> might be better (although our 80-column rule might hurt then >:-).

Done. I agree, the name bothered me as well, "expected_receiver_type" is much
better.

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

http://codereview.chromium.org/10442129/diff/1/test/cctest/test-api.cc#newcod...
test/cctest/test-api.cc:16601: printf("InstanceCheckedGetter!\n");
On 2012/06/04 07:50:32, Sven Panne wrote:
> Hmmm, do we really need printf in unit tests?

Done. Ooops, forgot to remove that again.

[Michael Starzinger, 2012-06-04 11:34:33]

@Danno: Could you PTAL at the API changes (v8.h and api.cc). Thanks!

https://chromiumcodereview.appspot.com/10442129/diff/1/include/v8.h
File include/v8.h (right):

https://chromiumcodereview.appspot.com/10442129/diff/1/include/v8.h#newcode2362
include/v8.h:2362: Handle<FunctionTemplate> constructor =
On 2012/06/01 13:19:43, Michael Starzinger wrote:
> As discussed offline, we might want to wrap this FunctionTemplate into some
> AccessorSignature, because we already have a similar concept for invocation
> callbacks.

Done.

[danno, 2012-06-06 09:32:31]

API lgtm with nits

http://codereview.chromium.org/10442129/diff/15001/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/10442129/diff/15001/include/v8.h#newcode2353
include/v8.h:2353: *   and is used to perform implicit instance checks against.
If the receiver
nit: against them.

http://codereview.chromium.org/10442129/diff/15001/include/v8.h#newcode2485
include/v8.h:2485: * can legally be called with.
how about: "An AccessorSignature specifies which receivers are valid arguments
to an accessor callback."

[Michael Starzinger, 2012-06-08 07:40:44]

http://codereview.chromium.org/10442129/diff/15001/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/10442129/diff/15001/include/v8.h#newcode2353
include/v8.h:2353: *   and is used to perform implicit instance checks against.
If the receiver
On 2012/06/06 09:32:31, danno wrote:
> nit: against them.

Done.

http://codereview.chromium.org/10442129/diff/15001/include/v8.h#newcode2485
include/v8.h:2485: * can legally be called with.
On 2012/06/06 09:32:31, danno wrote:
> how about: "An AccessorSignature specifies which receivers are valid arguments
> to an accessor callback."  

Done. Also applied to "Signature" a few lines above.