Discussion about removing extra_traits in 1.0

[tgross35]

It has been proposed to remove the Eq, Hash, and Debug implementations as part of 1.0 #3248. The reason for this is that for anything with padding or unions, it is not easy to ensure these are sound. An example is at #2816.

I think the problem with this isn't always that they are unsound, but that their usage leads to unsoundness. For example, #2700: they definitely shouldn't have relied on sigemptyset to fully initialize the struct, but there isn't anything that looks incorrect at first glance.

At this time, a very rough search shows about 850 Cargo.toml files that make use of this feature, compared to ~50k without it (https://github.com/search?q=lang%3Atoml+%2F%5Elibc%5Cs%2B%3D.*%22extra_traits%22%2F&type=code vs https://github.com/search?q=lang%3Atoml+%2F%5Elibc%5Cs%2B%3D%2F&type=code). This suggests there isn't a whole lot of ecosystem use that will experience breakage, but it also isn't negligible.

It seems Nix may be influenced by this more directly nix-rust/nix#2468.

Personally I think we could leave a Debug implementation but just make non-primitive types nonexhaustive (e.g. sigset_t { .. }). This would at least allow #[derive(Debug)] on anything that wraps these types, and just make it always available rather than behind a feature.

There is a proposed PR up: #3692

[tgross35]

Trying to move discussion here from nix-rust/nix#2468. @asomers said:

Extra unused fields might be unhelpful when implementing certain traits, but how could they make it unsafe? And shouldn't the compiler be able to handle unaligned fields?

For padding fields or reserved fields, reading the data might not be a problem - but we have no clue what the kernel might put there so a derived PartialEq is totally meaningless. Even without padding fields, having the derived implementation makes it easy to assume that the kernel/c library will set all fields to meaningful values which certainly isn't always true.

It is just difficult to justify keeping around with these kind of footguns. Plus having a pointer in the struct makes the equality check meaningless, or at the very least unintuitive. Making sure that everyone only compares the fields they need and know are initialized seems like a pretty reasonable tradeoff.

However, it would be good to see some code examples to better understand how exactly this is currently being used.

Packed structs alone aren't a problem, it's just a restriction on what can be implemented (must be Copy, but most everything has this anyway).