tokio-rs/slab

should get_unchecked and get_unchecked_mut be using unreachable!() ?

oconnor663 opened this issue · 4 comments

oconnor663 commented

I think the bad branches in those functions are reachable, if the caller passes in a key that's been removed. Should those branches be ordinary panics? Or should those functions maybe return Option<&T> / Option<&mut T>, like the get and get_mut functions do? (Or is it possible these functions don't have any callers in the wild, and we can remove them?)

carllerche commented

If they are reachable, it should probably be changed to a panic instead. That said, unreachable is just a panic.

The functions are here to avoid any extra bounds checking.

oconnor663 commented

I wonder if there's a way to be even less safe, by unwrapping the Option without a branch. Maybe we could construct a mem::uninitialized instance of Some(T) (or rather...a Some(mem::uninitialized())?), figure out the pointer offset for the T inside, and mem::forget the "temporary" instance, and the compiler would just optimize all that code away to a constant? I need to learn to read assembly...

tormol commented

Making them less safe can now be done by using std::hint::unreachable_unchecked(), but that requires Rust 1.27.

Currently the compiler isn't inserting any trap (ud2) instructions either.

EDIT: I have a commit implementing this change, but I'm not opening a PR because I don't think this is worth making a breaking change for.