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

Refactor Resources #617

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Refactor Resources #617

wants to merge 2 commits into from

Conversation

filmor
Copy link
Member

@filmor filmor commented Jun 3, 2024

  • Move storage of "Type->NIF Resource Type Handle" to a OnceLock map to allow implementing resource types without resorting to dynamic trait implementations
  • Implement resource types as a derive macro
  • Add direct access methods to get an immutable reference to the wrapped objects
  • Add corresponding converters

This allows us to eventually deprecate the existing resource! macro to fix #606.

TODO (or follow-up):

@filmor filmor mentioned this pull request Jun 3, 2024
3 tasks
@filmor filmor force-pushed the resource branch 2 times, most recently from 0941bde to ae70d92 Compare June 7, 2024 09:37
- Move storage of "Type->NIF Resource Type Handle" to a OnceLock map to
  allow implementing resource types without resorting to dynamic trait
  implementations
- Implement resource types as a `derive` macro
- Add direct access methods to get an immutable reference to the wrapped
  objects
- Add corresponding converters
@filmor filmor marked this pull request as ready for review June 8, 2024 11:04
@filmor filmor requested a review from a team June 8, 2024 11:04
@filmor filmor marked this pull request as draft June 8, 2024 12:41
@filmor
Copy link
Member Author

filmor commented Jun 8, 2024

Another option, instead of a derive macro (or additional to one) would be to allow explicitly implementing a Resource trait and registering it using a rustler::resource!() outside of load, which would use the inventory::submit! under the hood. This would allow us to make the down callback for #376 (and dyncall and other potential callbacks) a trait function with an empty default implementation.

If we like the derive style, another alternative would be, to have a derive(Resource) and a derive(MonitorResource), where MonitorResource requires an implementation of a MonitorResource trait with a down_callback function and these would be different code paths with two sets of registrations. I think I'm in favour of that.

/edit: This is how it is implemented now: #[derive(MonitorResource)] + implementing the MonitorResource trait.

@filmor
Copy link
Member Author

filmor commented Jun 11, 2024

Note to self: #84 is also a very interesting approach (completely doing away with the necessary pre-registration for resource types!), I'm just not quite sure how to incorporate monitors into it.

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.

rustler::resource! produces compiler warning
1 participant