Skip to content

Should we Remove Our Unsafe Precondition Checks? #721

@InsertCreativityHere

Description

@InsertCreativityHere

We utilize 'unsafe' code in a couple places, where the logic is low level, easy to reason about, and called often.
For example, in the code for reading bytes from a buffer.

But, in every place where we have unsafe code, we make sure to include a debug_assert right above it.
These double-check that we uphold the necessary conditions to guarantee that our code is actually safe. For example:
https://github.com/icerpc/slice-rust/blob/364cba01ee838c109d6a00cb17fedd1613323903/slice-codec/src/buffer/slice.rs#L61-L62

This seems great! But... these checks are actually redundant.

If you look in Rust's source code, every unsafe function in the standard library (that I've seen)
already has these checks baked into them, on the first line even. For our example:

    unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] {
        assert_unsafe_precondition!(
            check_library_ub,
            "slice::get_unchecked requires that the range is within the slice",
            (
                start: usize = self.start,
                end: usize = self.end,
                len: usize = slice.len()
            ) => end >= start && end <= len
        );

(http://github.com/rust-lang/rust/blob/79734f1db8dbe322192dea32c0f6b80ab14c4c1d/library/core/src/slice/index.rs#L361-L370)

But!! In the past, you couldn't actually rely on these checks. Because they weren't reliably run, even in debug mode (they explain why in the release notes). So, our checks were still necessary. Since they weren't always redundant.

But, Rust just released a new version, 1.78, and it included a 'fix' to ensure that these checks are always run in debug mode.
Meaning, our checks are truly, completely, always redundant.


So, Question:

Should we get rid of them because we know that the standard library will have our back?
Or should we keep them because keeping them doesn't harm anything, and it's better to be safe than sorry?

Metadata

Metadata

Assignees

No one assigned

    Labels

    easyGood for newcomersslice-codecRelated to the 'slice-codec' crate

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions