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

Shared/Unique FFI unwrap #42

Closed
wants to merge 11 commits into from

Conversation

iduartgomez
Copy link
Contributor

@iduartgomez iduartgomez commented Mar 30, 2017

Split 'unwrap' method of FFI trait into shared (returns *const _) / owned (returns *mut _) methods, instead of just one method returning *mut _.

This allows applying ownership & mutability rules consistently across all the FFI bindings as long as the C definitions are correct (*const/*mut pointers where appropriate).

Will allow for implementation of unsafe sync/senc traits for certain types (like matrix) or use of smart pointers like Rc/RefCell etc.

@iduartgomez iduartgomez changed the title Shared wrapper Shared/Unique FFI unwrap Mar 30, 2017
@GuillaumeGomez
Copy link
Owner

Can you remove the merge commit please?

@GuillaumeGomez
Copy link
Owner

Why is the fix readme back? You should try to use the rebase strategy instead of the merge one. Allow to have way better git logs.

force mutability for rng argument types

split FFI unwrap into owned/shared

fix compile error

fix readme

fix compile error
@iduartgomez
Copy link
Contributor Author

iduartgomez commented Mar 30, 2017

yeah i was trying to rebase to remove the 'merge' commit but screwed up, i can make a clean commit and delete this mess, sorry about that!

@GuillaumeGomez
Copy link
Owner

No problem, thanks a lot for all your work. It's really awesome!

@GuillaumeGomez
Copy link
Owner

If you don't know how to do it, you can take a look at this blog post.

@iduartgomez
Copy link
Contributor Author

Yes that's what I was doing but I had to review changes in a lot of files due to the recent pull (the readme one) so it was a bit of a mess.

i made a new clean pull request based on the current HEAD so it's fixed anyway, no worries. i'll close this one down.

@iduartgomez iduartgomez deleted the shared-wrapper branch March 30, 2017 18:30
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.

None yet

2 participants