slab icon indicating copy to clipboard operation
slab copied to clipboard

should get_unchecked and get_unchecked_mut be using unreachable!() ?

Open oconnor663 opened this issue 8 years ago • 4 comments

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?)

oconnor663 avatar Oct 07 '17 06:10 oconnor663

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.

carllerche avatar Oct 18 '17 03:10 carllerche

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...

oconnor663 avatar Oct 18 '17 14:10 oconnor663

Relevant: https://github.com/rust-lang/rfcs/issues/1863

oconnor663 avatar Oct 18 '17 14:10 oconnor663

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.

tormol avatar Mar 01 '19 21:03 tormol