Skip to content

Add an unsizing operation to {Arc,Rc}Box #71

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

Merged
merged 3 commits into from
Apr 16, 2021
Merged

Add an unsizing operation to {Arc,Rc}Box #71

merged 3 commits into from
Apr 16, 2021

Conversation

197g
Copy link
Contributor

@197g 197g commented Apr 15, 2021

No description provided.

197g added 2 commits April 15, 2021 23:25
To avoid the unstable traits this uses a helper crate that provides the
necessary checks and mechanisms for the raw pointer conversion while
opting into them via a trait implemented for the smart pointers here.
Copy link
Owner

@CAD97 CAD97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Honestly, unsize would fit right in with the pointer-utils family. (I've been thinking of de-mono-repoing and putting these crates in an org, if you'd want to collaborate. Just a thought, not an ask. I'm still not even sure I want to set up an org.) I just have a couple nits and one thought:

thought: should the other crates support explicit unsizing? Specifically, {A}RcBorrow<T> and Thin<Ptr<T>> seem like good candidates. (The latter would need to make sure that the coerced-to pointer is also erasable.)

I'm happy to merge as-is once you've thought about those impls. Either way, they should probably be added in a followup PR anyway.

bors: d+

@bors
Copy link
Contributor

bors bot commented Apr 16, 2021

✌️ HeroicKatora can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@CAD97
Copy link
Owner

CAD97 commented Apr 16, 2021

Oh, and I fixed the new warnings in #72, don't worry about them.

@197g
Copy link
Contributor Author

197g commented Apr 16, 2021

bors r+

@197g
Copy link
Contributor Author

197g commented Apr 16, 2021

thought: should the other crates support explicit unsizing? Specifically, {A}RcBorrow and Thin<Ptr> seem like good candidates. (The latter would need to make sure that the coerced-to pointer is also erasable.)

I'll see about a follow-up PR for those two crates, this sounds good to me as well. I wasn't sure about Thin initially but from the point of view of this being a true operation, not a builtin zero-cost coercion, and after viewing its implementation it does seem more reasonable.

@bors
Copy link
Contributor

bors bot commented Apr 16, 2021

Build succeeded:

@bors bors bot merged commit 5b296c5 into CAD97:master Apr 16, 2021
@CAD97
Copy link
Owner

CAD97 commented Apr 16, 2021

I wasn't sure about Thin initially but from the point of view of this being a true operation, not a builtin zero-cost coercion, and after viewing its implementation it does seem more reasonable.

Yeah, erase/unerase is always just be a pointer cast for sized types, so coercing from Thin<Ptr<Sized>> to Thin<Ptr<Unsized>> should just be

  • cast ErasedPtr to *mut Sized per Erasable::unerase (guaranteed to just be a pointer cast)
  • cast *mut Sized to Ptr<Sized> per ErasablePtr::unerase (essentially Ptr::from_raw; no pointer offset if coercion happens)
  • coerce Ptr<Sized> to Ptr<Unsized>
  • cast Box<Unsized> to *mut Unsized per ErasablePtr::erase (essentially Ptr::into_raw; no pointer offset if coercion happens)
  • cast *mut Unsized to ErasedPtr per Erasable::erase (likely just a pointer cast)

all of which should be unlined.

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.

2 participants