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 intonodejs:mainfrom
gabrielschulhof:same-object
Closed

converter: implement SameObject#19
gabrielschulhof wants to merge 1 commit intonodejs:mainfrom
gabrielschulhof:same-object

Conversation

@gabrielschulhof
Copy link
Copy Markdown
Contributor

Generate idempotent versions of property getter results for attributes
marked as SameObject.

@gabrielschulhof gabrielschulhof marked this pull request as draft November 11, 2020 06:30
Generate idempotent versions of property getter results for attributes
marked as `SameObject`.
@gabrielschulhof gabrielschulhof marked this pull request as ready for review November 11, 2020 06:48
Comment thread webidl-napi-inl.h
napi_status Wrapping<T>::Retrieve(napi_env env,
napi_value js_rcv,
T** cc_rcv,
int ref_idx,
Copy link
Copy Markdown

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

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

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.

gabrielschulhof pushed a commit that referenced this pull request Nov 16, 2020
Generate idempotent versions of property getter results for attributes
marked as `SameObject`.

Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
PR-URL: #19
Reviewed-By: Kai Ninomiya <kainino@chromium.org>
@gabrielschulhof
Copy link
Copy Markdown
Contributor Author

Landed in 88a9f9a.

@gabrielschulhof gabrielschulhof deleted the same-object branch November 16, 2020 07:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants