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

Remove uses of transmute #60

Closed
nox opened this Issue Jul 21, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@nox
Copy link
Member

nox commented Jul 21, 2017

Most uses of transmute are useless and security hazards, they should be replaced by something less magical. For examples the various transmute calls in from_u8 methods can be replaced entirely by safe code.

@nwin

This comment has been minimized.

Copy link
Contributor

nwin commented Jul 21, 2017

Do we have a good replacement of FromPrimitive by now? That is the sole reason that code exists in the first place.

Must uses of transmute are unfortunately not unnessesary here but caused by limitations of rustc. Replacing them would make the code much slower and harder to read.

@nox

This comment has been minimized.

Copy link
Member

nox commented Jul 21, 2017

The good replacement is to make match arms and let the optimiser get rid of it. It also help readability, contrary to what you just stated.

@nwin

This comment has been minimized.

Copy link
Contributor

nwin commented Jul 21, 2017

I was referring to most uses of transmute which is not in from_u8.

@nox

This comment has been minimized.

Copy link
Member

nox commented Jul 21, 2017

They all hurt readability.

@nwin

This comment has been minimized.

Copy link
Contributor

nwin commented Jul 21, 2017

Which is simply not true. If you find a replacement which doesn't involve wild boxing I'm happy to make that change. As you can read from the comments I tried hard to get rid of them.

@est31

This comment has been minimized.

Copy link

est31 commented Jul 21, 2017

Do we have a good replacement of FromPrimitive by now

There are a couple of crates for that:

@Shnatsel

This comment has been minimized.

Copy link
Contributor

Shnatsel commented Jul 12, 2018

Transmutes for from_u8 have been removed. Only two instances of the following lifetime-related transmutes remain:

// This transmute just casts the lifetime away. Since Rust only
// has SESE regions, this early return cannot be worked out and
// such that the borrow region of self includes the whole block.
// The explixit lifetimes in the function signature ensure that
// this is safe.
// ### NOTE
// To check that everything is sound, return the result without
// the match (e.g. `return Ok(try!(self.next_state(buf)))`). If
// it compiles the returned lifetime is correct.
unsafe {
    mem::transmute::<Decoded, Decoded>(result)
}

I wonder if the upcoming non-lexical lifetimes would help with that?

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