Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instanceof master v8 #48

Conversation

gabrielschulhof
Copy link
Collaborator

@gabrielschulhof gabrielschulhof commented Jan 16, 2017

Implementation of napi_instanceof() which uses the "prototype" property of the constructor and the "__proto__" property of the object to be examined.

It also adds a test to make sure that the result fully implements the semantics of the instanceof operator. The test is taken from V8's https://github.com/nodejs/abi-stable-node/blob/api-prototype-6.2.0/deps/v8/test/mjsunit/instanceof.js and https://github.com/nodejs/abi-stable-node/blob/api-prototype-6.2.0/deps/v8/test/mjsunit/instanceof-2.js and modified to replace the JS instanceof operator with calls to the addon that exposes napi_instanceof().

Note that napi_instanceof() is not "linearly independent", in that it can be implemented with calls to existing NAPI APIs (https://gist.github.com/gabrielschulhof/3a8d174b860db0c18a732c0b25c12cbb). Unfortunately this currently means that napi_instanceof() will be less efficient on V8 than the native V8 HasInstance() because we cannot call FunctionTemplate::HasInstance(), whereas on ChakraCore napi_instanceof() will be more efficient than an implementation using other napi APIs, because we can call JsInstanceOf().

So, adding napi_instanceof() will be a net benefit for ChakraCore and a net detriment for V8 - with the benefit that there will be a VM-neutral API for accomplishing this task.

Re #47

jasongin

This comment was marked as off-topic.

jasongin

This comment was marked as off-topic.

jasongin

This comment was marked as off-topic.

jasongin

This comment was marked as off-topic.

jasongin

This comment was marked as off-topic.

jasongin

This comment was marked as off-topic.

Gabriel Schulhof added 3 commits January 20, 2017 11:37
In napi_instanceof(), GetPrototype() can be used for retrieving the
object prototype, but not for retrieving the function prototype. For
that, we need to retrieve the "prototype" property.
@gabrielschulhof gabrielschulhof force-pushed the add-instanceof-master-v8 branch from b9e3990 to d137ac0 Compare January 20, 2017 10:15
@jasongin
Copy link
Member

LGTM

Maybe replace all your tabs with spaces though to avoid mixing and be consistent with the Node.js code style.

gabrielschulhof pushed a commit that referenced this pull request Jan 20, 2017
Since FunctionTemplate::HasInstance() is inaccessible, we must implement
the Javascript instanceof operator using the public V8 API. This does
that, and uses the V8 tests for instanceof to ensure that the semantics
of the API are identical to the semantics of the Javascript operator.

Closes #48
@gabrielschulhof gabrielschulhof deleted the add-instanceof-master-v8 branch January 20, 2017 20:20
jasongin pushed a commit to jasongin/abi-stable-node that referenced this pull request Jan 26, 2017
Since FunctionTemplate::HasInstance() is inaccessible, we must implement
the Javascript instanceof operator using the public V8 API. This does
that, and uses the V8 tests for instanceof to ensure that the semantics
of the API are identical to the semantics of the Javascript operator.

Closes nodejs#48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants