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

Merged
merged 2 commits into from
Jul 7, 2024
Merged

Refactor Resources #617

merged 2 commits into from
Jul 7, 2024

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 (this one is removed again)
  • Add direct access methods to get an immutable reference to the wrapped objects
  • Add corresponding converters
  • Add explicit resource registration functions
  • Add an additional optional "destructor" function to implement use cases like Lifetime issues when I want a resource to hold a NIF term #626, where a message is supposed to be sent when finalising a resource object
  • Implement monitoring support

This allows us to eventually deprecate the existing resource! macro and enables a path that fixes #606.

@filmor filmor mentioned this pull request Jun 3, 2024
3 tasks
@filmor filmor force-pushed the resource branch 3 times, most recently from ae70d92 to 5744d8f Compare June 8, 2024 11:03
@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.

- Move storage of "Type->NIF Resource Type Handle" to a OnceLock map to
  allow implementing resource types without resorting to dynamic trait
  implementations
- Add direct access methods to get an immutable reference to the wrapped
  objects
- Add corresponding converters
- Add monitor resource implementation
- Require explicit trait implementation (not when using the existing
  macro for compatibility)
- Allow implementing a "destructor" function that runs just before
  `Drop` but has access to the caller environment
@filmor filmor merged commit 6101085 into rusterlium:master Jul 7, 2024
34 checks passed
@filmor filmor deleted the resource branch July 18, 2024 11:49
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
2 participants