Remove all unsafe, #![forbid(unsafe_code)]

[joshlf]

No description provided.

[BurntSushi]

I love zerocopy and what it's trying to do, and if this stuff were in std, I'd absolutely have no hesitation doing this (assuming we waited long enough to keep the MSRV somewhat conservative). But I think my view is generally unchanged from rust-lang/regex#1189. This isn't bringing in the proc macro, but it still makes cargo clean && cargo b -r be around 4x slower on my machine. byteorder is used in tons of places, so adding this dependency here will end up adding it to a lot of dependency trees.

I think this stance probably applies to most of my crates. One place where I'd differ is for opt-in support like serde so that types implement the relevant zerocopy traits. I'm not sure if any of my crates fall into that bucket though. (Maybe bstr?)

I think I'd actually probably rather folks decrease dependency on byteorder. I wonder how many folks are using byteorder when they could just be using std's APIs that were added long after byteorder existed. Hah. Of course, byteorder offers other things, so IDK to what extent folks can move off of it.

[joshlf]

But I think my view is generally unchanged from rust-lang/regex#1189.

No worries, I totally understand! In putting up these PRs, I made sure I hadn't already put up PRs to the same repo in the past that got declined, but I forgot about other repos by the same author - sorry for the noise!

it still makes cargo clean && cargo b -r be around 4x slower on my machine.

Do you think you'd be more inclined to take the dependency if there were less of a slowdown? Not that this particular PR is super important to me, but it's good to get a sense of what the maintainers of important crates care about. E.g. I could see us putting functionality behind default-enabled feature flags so that users who care can disable everything and speed up compile times.

[BurntSushi]

That's a good question. I honestly don't know. No matter how much it decreases, it's still non-zero. And I do overall feel like the code in byteorder is relatively low-risk and it isn't changing much if at all. It's been nearly frozen for many years. So while the downsides could maybe be mitigated to an extent, the upside isn't huge in this case.

I think I'd also be more inclined to do it if a lot more people were using zerocopy. (I don't have a good sense of how often it appears in dependency trees.) But that's a chicken-and-egg problem...

[joshlf]

Okay, that's helpful context, thanks!

[joshlf]

We recently added a feature to zerocopy that allowed me to update this to remove the remaining unsafe from byteorder and #![forbid(unsafe_code)]. Obviously no worries if your other concerns still hold and so you'd still prefer not to merge, but I figured I'd at least put it up so you can take a look.

[BurntSushi]

That's awesome. I don't think my stance has changed personally. It might make sense to add a note to the README recasting byteorder as a "lightweight dependency for byte order conversions, but if you already are using zerocopy, you can probably just use that directly without any unsafe for efficient and safe byte order conversions."

[joshlf]

No worries!