Skip to content
This repository was archived by the owner on Mar 25, 2023. It is now read-only.

converter: implement SameObject #19

Closed
gabrielschulhof wants to merge 1 commit into from
Closed

converter: implement SameObject #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 57 additions & 25 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ function generateParamRetrieval(sigs, maxArgs, isForConstructor) {
].join('\n');
}

function generateCall(ifname, sig, indent) {
function generateCall(ifname, sig, indent, sameObjAttrCount) {
function argToNativeCall(idlType, index, indent) {
return [
`NAPI_CALL(`,
Expand Down Expand Up @@ -412,10 +412,9 @@ function generateCall(ifname, sig, indent) {
// If this is not a static method or a constructor, declare and retrieve the
// native instance `cc_rcv` corresponding to the JS instance in `js_rcv`.
...((sig.special !== 'static' && sig.type != 'constructor') ? [
`void* cc_rcv_raw;`,
`${ifname}* cc_rcv;`,
`NAPI_CALL(env, napi_unwrap(env, js_rcv, &cc_rcv_raw));`,
`cc_rcv = static_cast<${ifname}*>(cc_rcv_raw);`,
`NAPI_CALL(env,`,
` WebIdlNapi::Wrapping<${ifname}>::Retrieve(env, js_rcv, &cc_rcv));`,
``
] : [])
] : []),
Expand Down Expand Up @@ -448,13 +447,11 @@ function generateCall(ifname, sig, indent) {
// it into the JS object we're constructing.
...(sig.type === 'constructor' ? [
`NAPI_CALL(env,`,
` napi_wrap(`,
` WebIdlNapi::Wrapping<${ifname}>::Create(`,
` env,`,
` js_rcv,`,
` static_cast<void*>(ret),`,
` &WebIdlNapi::ObjectWrapDestructor<${ifname}>,`,
` nullptr,`,
` nullptr));`
` ret,`,
` ${sameObjAttrCount}));`
] : []),
// Special handling for promises. We need to call `Conclude()` before
// returning to JS to at least create the `napi_deferred` and even resolve
Expand All @@ -468,7 +465,7 @@ function generateCall(ifname, sig, indent) {
.join('\n');
}

function generateIfaceOperation(ifname, opname, sigs) {
function generateIfaceOperation(ifname, opname, sigs, sameObjAttrCount) {
if (sigs.length === 0) {
// If we have no signatures, generate a trivial one.
sigs = [ {
Expand Down Expand Up @@ -508,7 +505,10 @@ function generateIfaceOperation(ifname, opname, sigs) {
// Handle the case in a constructor where the `argv[0]` was an external.
...(opname === 'constructor' ? [
` if (external != nullptr) {`,
generateCall(ifname, { type: 'constructor', external: true }, ' '),
generateCall(ifname,
{ type: 'constructor', external: true },
' ',
sameObjAttrCount),
` return nullptr;`,
` }`,
] : []),
Expand All @@ -520,10 +520,10 @@ function generateIfaceOperation(ifname, opname, sigs) {
...(sigs.length > 1
? [ sigs.map((sig, index) => [
` if (sig_idx == ${index}) {`,
generateCall(ifname, sig, ' '),
generateCall(ifname, sig, ' ', sameObjAttrCount),
' }'
].join('\n')).join('\n else\n') ]
: [ generateCall(ifname, sigs[0], ' ') ]),
: [ generateCall(ifname, sigs[0], ' ', sameObjAttrCount) ]),
// If the op has a return type, compute it and store the resulting
// `napi_value` in `js_ret`.
...(hasReturn ? [
Expand All @@ -539,7 +539,7 @@ function generateIfaceOperation(ifname, opname, sigs) {
].join('\n');
}

function generateIfaceAttribute(ifname, attribute) {
function generateIfaceAttribute(ifname, attribute, sameObjIdx) {
const nativeAttributeType = generateNativeType(attribute.idlType);
function generateAccessor(slug) {
return [
Expand All @@ -566,9 +566,25 @@ function generateIfaceAttribute(ifname, attribute) {
` &js_rcv,`,
` nullptr));`,
``,
` void* raw_data;`,
` NAPI_CALL(env, napi_unwrap(env, js_rcv, &raw_data));`,
` ${ifname}* cc_rcv = static_cast<${ifname}*>(raw_data);`,
` ${ifname}* cc_rcv;`,
...((sameObjIdx >= 0 && slug === 'get') ? [
` WebIdlNapi::Wrapping<${ifname}>* wrapping;`,
` NAPI_CALL(env,`,
` WebIdlNapi::Wrapping<${ifname}>::Retrieve(`,
` env,`,
` js_rcv,`,
` &cc_rcv,`,
` ${sameObjIdx},`,
` &result,`,
` &wrapping));`,
` if (result != nullptr) return result;`
] : [
` NAPI_CALL(env,`,
` WebIdlNapi::Wrapping<${ifname}>::Retrieve(`,
` env,`,
` js_rcv,`,
` &cc_rcv));`,
]),
``,
...(slug === 'set' ? [
` NAPI_CALL(`,
Expand All @@ -584,6 +600,9 @@ function generateIfaceAttribute(ifname, attribute) {
` env,`,
` cc_rcv->${attribute.name},`,
` &result));`,
...((sameObjIdx >= 0 && slug === 'get') ? [
` NAPI_CALL(env, wrapping->SetRef(env, ${sameObjIdx}, result));`,
] : [])
]),
` return result;`,
`}`,
Expand Down Expand Up @@ -708,11 +727,12 @@ function generateIfaceConverters(ifaceName) {
` napi_env env,`,
` napi_value val,`,
` ${ifaceName}* result) {`,
` void* data;`,
` napi_status status = napi_unwrap(env, val, &data);`,
` ${ifaceName}* data;`,
` napi_status status =`,
` WebIdlNapi::Wrapping<${ifaceName}>::Retrieve(env, val, &data);`,
` if (status != napi_ok) return status;`,
``,
` *result = *static_cast<${ifaceName}*>(data);`,
` *result = *data;`,
` return napi_ok;`,
`}`
].join('\n');
Expand All @@ -731,8 +751,17 @@ function generateIface(iface) {
const collapsedCtors =
iface.members.filter((item) => (item.type === 'constructor'))

const attributes = iface.members
.filter((item) => (item.type === 'attribute'));
const { attrs, sameObjAttrs } =
iface.members.reduce((soFar, item) => {
if (item.type === 'attribute') {
const list = ((item.extAttrs.filter(({ name }) =>
(name === 'SameObject'))).length > 0)
? 'sameObjAttrs'
: 'attrs';
soFar[list].push(item);
}
return soFar;
}, { attrs: [], sameObjAttrs: [] });

return [
[
Expand All @@ -747,11 +776,14 @@ function generateIface(iface) {
// `generateIfaceOperation`. That way, only one binding is generated for all
// signatures of an operation.
generateIfaceConverters(iface.name),
generateIfaceOperation(iface.name, 'constructor', collapsedCtors),
generateIfaceOperation(iface.name, 'constructor', collapsedCtors,
sameObjAttrs.length),
...Object.entries(collapsedOps).map(([opname, sigs]) =>
generateIfaceOperation(iface.name, opname, sigs)),
...attributes.map((item) => generateIfaceAttribute(iface.name, item)),
generateIfaceInit(iface.name, collapsedOps, attributes)
...attrs.map((item) => generateIfaceAttribute(iface.name, item)),
...sameObjAttrs.map((item, idx) =>
generateIfaceAttribute(iface.name, item, idx)),
generateIfaceInit(iface.name, collapsedOps, [...attrs, ...sameObjAttrs])
].join('\n\n');
}

Expand Down
2 changes: 1 addition & 1 deletion test/class/class-impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Incrementor::Incrementor(): Incrementor(0) {}
Incrementor::Incrementor(DOMString initial_value)
: Incrementor(std::stoul(initial_value)) {}

Incrementor::Incrementor(unsigned long initial_value) {
Incrementor::Incrementor(unsigned long initial_value): props{"blah", 42} {
val = new value__(initial_value);
val->val = initial_value;
}
Expand Down
9 changes: 9 additions & 0 deletions test/class/class-impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
typedef struct value__* Value;
class Incrementor;

struct Properties {
DOMString name;
unsigned long count;
};

class Decrementor {
public:
void operator=(const Decrementor& other);
Expand All @@ -23,6 +28,10 @@ class Incrementor {
Incrementor(unsigned long initial);
Incrementor(DOMString initial);
unsigned long increment();

Properties props;
Properties settableProps;

Decrementor getDecrementor();
friend class Decrementor;
~Incrementor();
Expand Down
7 changes: 7 additions & 0 deletions test/class/class.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@ interface Decrementor {
unsigned long decrement();
};

dictionary Properties {
DOMString name;
unsigned long count;
};

interface Incrementor {
constructor();
constructor(unsigned long initial);
constructor(DOMString initial);
unsigned long increment();
Decrementor getDecrementor();
[SameObject] readonly attribute Properties props;
attribute Properties settableProps;
};
14 changes: 14 additions & 0 deletions test/class/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@ const assert = require('assert');
test(require('bindings')({ bindings: 'class', module_root: __dirname }));

function test(binding) {
{
const inc = new binding.Incrementor(49);
const props1 = inc.props;
const props2 = inc.props;
assert.strictEqual(props1, props2);

const settableProps1 = inc.settableProps;
const settableProps2 = inc.settableProps;
assert.deepStrictEqual(settableProps1, settableProps2);

const expectedNewValue = { name: "new name", count: 99 };
inc.settableProps = expectedNewValue;
assert.deepStrictEqual(inc.settableProps, expectedNewValue);
}
{
assert.strictEqual((new binding.Incrementor(12)).increment(), 13);
assert.strictEqual((new binding.Incrementor()).increment(), 1);
Expand Down
6 changes: 6 additions & 0 deletions test/webgpu/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,10 @@ async function test(binding) {

await testAdapter({ powerPreference: 'high-performance' });
await testAdapter();

// Make sure the `gpu` property is `SameObject`.
const nav = new binding.Navigator();
const gpu1 = nav.gpu;
const gpu2 = nav.gpu;
assert.strictEqual(gpu1, gpu2);
}
76 changes: 69 additions & 7 deletions webidl-napi-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,6 @@ inline napi_status IsConstructCall(napi_env env,
return status;
}

template <typename T>
static void ObjectWrapDestructor(napi_env env, void* data, void* hint) {
(void) env;
(void) hint;
delete reinterpret_cast<T*>(data);
}

inline napi_status PickSignature(napi_env env,
size_t argc,
napi_value* argv,
Expand Down Expand Up @@ -435,5 +428,74 @@ inline napi_ref InstanceData::GetConstructor(const char* name) {
return ctors[name];
}

// static
template <typename T>
napi_status Wrapping<T>::Create(napi_env env,
napi_value js_rcv,
T* cc_rcv,
size_t same_obj_count) {
Wrapping<T>* wrapping = new Wrapping<T>;
wrapping->native = cc_rcv;
if (same_obj_count > 0)
wrapping->refs.resize(same_obj_count, nullptr);
return napi_wrap(env, js_rcv, wrapping, Destroy, nullptr, nullptr);
}

// static
template <typename T>
napi_status Wrapping<T>::Retrieve(napi_env env,
napi_value js_rcv,
T** cc_rcv,
int ref_idx,
Copy link
Copy Markdown

@kainino0x kainino0x Nov 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional suggestion: I think I eventually figured out how it worked, but this system is pretty hard to follow. Seems like you assign a ref_idx to each member that has [SameObject], and then store the ref for that value in refs[ref_idx] for that member. Could it be possible to have the refs members instead be actual named napi_ref members in a generated struct?

Current code looks reasonably correct though.

Copy link
Copy Markdown
Contributor Author

@gabrielschulhof gabrielschulhof Nov 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could go with a std::map instead.

Copy link
Copy Markdown

@kainino0x kainino0x Nov 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the map would be keyed on a string, I think the current code is better.

napi_value* ref,
Wrapping<T>** get_wrapping) {
void* data = nullptr;

napi_status status = napi_unwrap(env, js_rcv, &data);
if (status != napi_ok) return status;

Wrapping<T>*wrapping = static_cast<Wrapping<T>*>(data);

if (ref_idx >= 0 &&
ref_idx < wrapping->refs.size() &&
wrapping->refs[ref_idx] != nullptr) {
napi_value ref_value = nullptr;

status = napi_get_reference_value(env, wrapping->refs[ref_idx], &ref_value);
if (status != napi_ok) return status;

if (ref != nullptr) *ref = ref_value;
}

if (get_wrapping != nullptr) *get_wrapping = wrapping;
*cc_rcv = wrapping->native;
return napi_ok;
}

template <typename T>
inline napi_status
Wrapping<T>::SetRef(napi_env env, int idx, napi_value same_obj) {
napi_ref ref;

napi_status status = napi_create_reference(env, same_obj, 1, &ref);
if (status != napi_ok) return status;

refs[idx] = ref;
return napi_ok;
}

// static
template <typename T>
void Wrapping<T>::Destroy(napi_env env, void* data, void* hint) {
(void) hint;
Wrapping<T>* wrapping = static_cast<Wrapping<T>*>(data);

for (napi_ref ref: wrapping->refs)
if (ref != nullptr)
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, ref));
delete wrapping->native;
delete wrapping;
}

} // end of namespace WebIdlNapi
#endif // WEBIDL_NAPI_INL_H
23 changes: 20 additions & 3 deletions webidl-napi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ static napi_status IsConstructCall(napi_env env,
const char* ifname,
bool* result);

template <typename T>
static void ObjectWrapDestructor(napi_env env, void* data, void* hint);

template <typename T>
class Converter {
public:
Expand Down Expand Up @@ -151,6 +148,26 @@ class InstanceData {
napi_finalize cb = nullptr;
};

template <typename T>
class Wrapping {
public:
static napi_status Create(napi_env env,
napi_value js_rcv,
T* cc_rcv,
size_t same_obj_count = 0);
static napi_status Retrieve(napi_env env,
napi_value js_rcv,
T** cc_rcv,
int ref_idx = -1,
napi_value* ref = nullptr,
Wrapping<T>** wrapping = nullptr);
napi_status SetRef(napi_env env, int idx, napi_value same_obj);
private:
static void Destroy(napi_env env, void* data, void* hint);
T* native = nullptr;
std::vector<napi_ref> refs;
};

} // end of namespace WebIdlNapi

#include "webidl-napi-inl.h"
Expand Down