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

Use TryInto to avoid unsafe. #507

Merged
merged 2 commits into from Jan 17, 2021
Merged

Use TryInto to avoid unsafe. #507

merged 2 commits into from Jan 17, 2021

Conversation

Gaelan
Copy link
Contributor

@Gaelan Gaelan commented Jan 4, 2021

I'm submitting a(n) refactor

Description

Previously, to_fields and to_fields_le used unsafe to convert a &[u8] into a &[u8; 8]. Now that we're only supporting Rust versions where TryInto is stable, we can use try_into().unwrap() instead, making uuid entirely safe Rust.

In release mode, the compiler detects that the slice will always be the correct size, so try_into can never fail. Thus, the unwrap
is optimized out and we end up with the exact same assembly as the unsafe block.

Godbolt output showing the resulting assembly: https://godbolt.org/z/nWxT6W

Motivation

Makes UUID entirely safe Rust.

With this PR,

Tests

All existing tests pass. Doesn't add any new functionality, so that should be sufficient. Assuming the Godbolt test is consistent with what happens in the context of the larger crate, this shouldn't change the resulting binary at all.

Related Issue(s)

Closes #488.

Previously, to_fields and to_fields_le used unsafe to convert a
&[u8] into a &[u8; 8]. Now that we're only supporting Rust
versions where TryInto is stable, we can use try_into().unwrap()
instead, making uuid entirely safe Rust.

In release mode, the compiler detects that the slice will always
be the correct size, so try_into can never fail. Thus, the unwrap
is optimized out and we end up with the exact same assembly
as the unsafe block.

Godbolt output showing the resulting assembly:
https://godbolt.org/z/nWxT6W

Closes uuid-rs#488.
src/lib.rs Outdated Show resolved Hide resolved
@kinggoesgaming
Copy link
Member

@kinggoesgaming kinggoesgaming commented Jan 17, 2021

bors r+

@bors
Copy link
Contributor

@bors bors bot commented Jan 17, 2021

Build succeeded:

@bors bors bot merged commit 805f4ed into uuid-rs:master Jan 17, 2021
21 checks passed
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.

3 participants