implement IpAddrs #64
Conversation
|
@bal-e looks like I can't request review from you, but feel free to! |
| Literal::IpAddress(addr) => { | ||
| let to = self.new_tmp(); | ||
| const SIZE: usize = std::mem::size_of::<IpAddr>(); | ||
| let x: [u8; SIZE] = unsafe { std::mem::transmute_copy(addr) }; |
There was a problem hiding this comment.
Is this safe? The layout of IpAddr is not guaranteed, right?
There was a problem hiding this comment.
It's not safe no. Well, I mean, some parts aren't. Transmuting into some bytes and then transmuting back should be fine, but combined with my own equality checking and all that it becomes unsafe.
There was a problem hiding this comment.
So that means that I either need to call out for any operation or implement my own ip addr type with stable ABI that hopefully converts back and forth with a no-op. I'd love it if I could just fail to compile if the layout changes lol.
src/lower/mod.rs
Outdated
| // For an ip addr, we need to be clever, because we can | ||
| // only compare the bytes that are used the current | ||
| // variant. Therefore, we read the discriminant of the | ||
| // left value. If that's 0, we compare 5 bytes, | ||
| // otherwise, we compare 17 bytes. In other words we | ||
| // compare 5 + 12 * discriminant bytes. |
There was a problem hiding this comment.
This is interesting, but I think a branch condition would be simpler and faster.
There was a problem hiding this comment.
Not simpler, because branching is a bit hard to add at this level (with the current API). Also keep in mind that cranelift optimizes branches very poorly at the moment, that being said, faster is not even on my mind right now.
No description provided.