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

extendr-impl: Return the right reference #726

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

CGMossa
Copy link
Member

@CGMossa CGMossa commented Apr 6, 2024

Following the polishing of ExternalPtr is this PR on returning the right reference.

First, we remove the ability to take &T and make ExternalPtr<T>. This is a flawed construct: Making an ExternalPtr is not just wrapping a pointer, it is also signing a pointer up for destruction. Converting a &T to ExternalPtr<T> may cause spurious double-free events.

The Rules of ExternalPtr are simple:

  • From an owned type, one may create an Owned Pointer, and from an owned pointer, one can instantiate a true ExternalPtr.
  • From &ExternalPtr<T> you can create &T, and similarly for &mut ExternalPtr<T> and &mut T.

So, should there be no references used or returned? A reference that comes from a true ExternalPtr<T> may be used. We embed this logic on the wrapping function, rather than do any C-crimes.

If a reference corresponds to any of the ExternalPtr<T> in the argument list, then that is returned.

Fixes #725

@CGMossa
Copy link
Member Author

CGMossa commented Apr 6, 2024

At this stage, I now check the address of the returned-object, and see if it matches the ExternalPtr from Self.

Error (test-test-submodules.R:32:3): Return the right externalptr
Error in `x$max_ref(y)`: only able to return reference to self
Backtrace:1. └─x$max_ref(y) at test-test-submodules.R:32:3

Next step is to go through all arguments, that are externalptr, and return the one that maches the output, or error otherwise.

Thankfully, it is impossible to create a local variable and return a reference from it in Rust.

@CGMossa
Copy link
Member Author

CGMossa commented Apr 6, 2024

This is ready for feedback, not necessarily review.

@CGMossa CGMossa marked this pull request as ready for review April 6, 2024 22:44
@CGMossa CGMossa requested a review from JosiahParry May 9, 2024 11:13
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.

extendr-impl: returning &Self from impl always returns the calling item
1 participant