Added unsafe SendSEXP to ownership-module

[CGMossa]

Currently, the Ownership module converts all SEXP to usize (?), such that they become Send, so that the ownership module can be used from multiple threads.

• Wrap SEXP in an explicit Send type. Accessing the pointer would require safety comments, as it can only be accessed through inner and set_inner.
• No pointer conversion to usize and back.
• Prototype for an eventual support for an unsafe SendRobj.

Background

This came up when looking at why tests started to fail sporadically. This isn't meant to fix anything in particular,
but it adds an explicit unsafe layer that is involved with what the ownership-module currently does. If we get rid of multithreaded support, we'd still provide users with something similar to this, or they would write something themselves, see #659 for discussion.

[sorhawell]

I'm not familiar with the ownership.rs module.

As I read this PR, it might be compiled down to the very same binary before and after.
I think the aim of this PR makes sense to not hide pointers in plain usize, whenever possible.

[CGMossa]

I'm not familiar with the ownership.rs module.

As I read this PR, it might be compiled down to the very same binary before and after. I think the aim of this PR makes sense to not hide pointers in plain usize, whenever possible.

Thanks for your comment. It is indeed my intention not to convert pointers to usize and back again. I can't find a reason why this is definitively bad, but it is unnecessary and the intent is unclear.

[CGMossa]

I've rebased and removed commented code.

[Ilia-Kosenkov]

This looks scary

Error: Error: SET_STRING_ELT() can only be applied to a 'character vector', not a 'integer'
*** longjmp causes uninitialized stack frame ***: terminated

[CGMossa]

This looks scary

Error: Error: SET_STRING_ELT() can only be applied to a 'character vector', not a 'integer'
*** longjmp causes uninitialized stack frame ***: terminated

Indeed. It is caused by the multithreaded issues we've been having. Restarting CI, will give us a success, by the luck of the draw. We can discuss this particular issue in #674, or other places.

[JosiahParry]

I tested this PR on {rsgeo} it builds just fine and works as anticipated.

R CMD check output:

Cargo.lock illustrating it is using this branch.

I feel confident in merging this. But since this is my first time getting familiar with the ownership module I'd like to ensure another set of eyes approve it. Would either @Ilia-Kosenkov or @sorhawell mind giving this another approval?

[JosiahParry]

@Ilia-Kosenkov do you have some time this week to review this PR?

[Ilia-Kosenkov]

I saw it, I will check it this weekend.

[CGMossa]

@Ilia-Kosenkov where is your comment on .entry? I still don't know what to do there :(

[Ilia-Kosenkov]

Disregard my comment on.entry, I was too meticulous :)

[JosiahParry]

All comments have been resolved, green checks abound. I'm going to merge! Thanks for your patience @CGMossa