Fix UB (issue 52)

[terrorfisch]

See #52

I additionally replaced the transmute with a match statement which results in a no-op with minimal optimization but might slow down debug builds. Tested with godbolt.

[terrorfisch]

I additionally defined the representations of CastleRights, Color and Piece because they are used to index into arrays with get_unchecked in several places.

[analog-hors]

I don't believe that's actually needed because of the indexing, since casting an enum with as should be perfectly fine. To my knowledge, the only use of transmute happens with Rank and File. Now that it's a match instead, the repr(u8) is probably also not needed for those enums too.

[terrorfisch]

My fault, I misremembered that the values are guaranteed.

As in C, discriminant values that are not specified are defined as either 0 (for the first variant) or as one more than the prior variant.

[jordanbray]

This looks good to me. I don't see any reason to avoid merging. Is this ready to go?

[terrorfisch]

If you do not mind the revert commit this can be merged.

[jordanbray]

I'll probably do a squash and merge. I generally prefer that.

[terrorfisch]

Then the other question is if you want to keep the repr(u8) because the current code does not need it anymore. The only downside is imo that it would be a breaking change to remove it again. The upside is that users can transmute slices without UB.

[jordanbray]

I don't really think I care either way. repr(u8) seems like it would allow mem::transmute, but I doubt it's that useful in the first place. Any time I can imagine needing a slice of Rank or File I feel like I'd intuitively reach for a BitBoard instead.

[analog-hors]

I don't think there's any point in keeping the repr

[terrorfisch]

I think one should allow users of this library to transmute if they want to but that is not a strong opinion. @jordanbray You decide and I open up a new squashed PR. Or do you want to do it?