Should we prioritize panic-free property over a number of unsafe lines?

[newpavlov]

Panic-free certainly is a nice property and I am working on a library in which we have attempted to eradicate all potential panics, but achieving it in practice is quite hard, partially because Rust currently lacks ability to prove and use certain code properties. One good example is the block-buffer code, it contains field pos, which is a cursor into the buffer field. The code is structured in a such way that pos is never larger or equal to the buffer length, but compiler is unable use this property and instead assumes that it can be any value allowed by the usize type. We could use get_unchecked, but it would significantly increase the number of unsafe lines. Also unfortunately this method does not check correctness of index/range in debug builds, see discussion in rust-lang/rust#51713 for more details.

I think we should strive for panic-free code, but with a rule that unsafe lines must be covered with debug asserts if it's possible to verify correctness at runtime. What do you think?

cc @tarcieri

[tarcieri]

I think this question is pretty contextual, and based on your given example I'm still not sure what the panic-free unsafe solution would be.

I'd certainly prefer panics over soundness bugs, so if that's the tradeoff, I'd definitely prefer panics. If the unsafe code is trivially easy to recognize as sound though, and especially if it confers other benefits e.g. performance, that's a different story.

I'm not super wild about debug asserts, mostly because I care about the properties of release builds much more than debug ones. Used incorrectly, debug asserts can even break release builds.

[newpavlov]

I'm still not sure what the panic-free unsafe solution would be

It will mostly involve replacing indexing operations like this one with get_unchecked/get_uncheked_mut. Soundness will rely on the pos being never larger or equal to BlockSize (though we would have to rework the input_lazy method for that).

I'd certainly prefer panics over soundness bugs, so if that's the tradeoff, I'd definitely prefer panics

Of course I do not propose willy-nilly replacement of potentially panicking operations with their unsafe counterparts. Each such replacement must be carefully considered and checked to be sound. Ideally I would prefer to stay in the realm of safe code (e.g. range type would've been really helpful), but unfortunately Rust is not currently powerful enough.

[tarcieri]

It will mostly involve replacing indexing operations like this one with get_unchecked/get_uncheked_mut. Soundness will rely on the pos being never larger or equal to BlockSize (though we would have to rework the input_lazy method for that).

Aah, interesting. Are there already NumAtMost<N: typenum::Unsigned>-style number types for this purpose?

Edit: as it were, just ran across this: https://docs.rs/funty/