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.

Sign up for GitHub

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

Jump to bottom

Add guarded transmute to mut bytes #19

Closed
wants to merge 1 commit into from
Closed

Add guarded transmute to mut bytes #19

wants to merge 1 commit into from

Conversation

frankier
Copy link

@frankier frankier commented Mar 5, 2018

Further to #17

After some quick testing, it seems that transmute does do the correct thing with mutable references. For example:

This doesn't work: https://play.rust-lang.org/?gist=1ff0495b84b12268e97ccf4defaf164f&version=stable

But this does: https://play.rust-lang.org/?gist=7c7b4cb6e87f405bb97806ef69835a8c&version=stable

So it seems like it should also be safe to allow guarded transmutes into mutable references as done here.

Does it seem like a reasonable idea? As with #9 it's possible these functions could be organised a bit better.

Enet4
Enet4 reviewed Mar 5, 2018
View reviewed changes
Copy link
Collaborator

@Enet4 Enet4 left a comment

Choose a reason for hiding this comment

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

I gave this some thought, and it looks ok to me. I would greatly appreciate some tests over the new functions, though!

nabijaczleweli
nabijaczleweli requested changes Mar 5, 2018
View reviewed changes
Copy link
Owner

@nabijaczleweli nabijaczleweli left a comment

Choose a reason for hiding this comment

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

Documentation & tests, but otherwise 👍

@nabijaczleweli
Copy link
Owner

@frankier are you gonna work on this, or?

@frankier
Copy link
Author

frankier commented Apr 5, 2018

Okay sure. Not for a little longer though. I am resting my hand due to rsi.

@nabijaczleweli
Copy link
Owner

Cool and good 👍

@Enet4 Enet4 mentioned this pull request Jun 17, 2019
Merged
@nabijaczleweli
Copy link
Owner

Closed by merge of #57

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Enet4 Enet4 Enet4 left review comments

@nabijaczleweli nabijaczleweli nabijaczleweli requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frankier @nabijaczleweli @Enet4