Split keyword `unsafe` into `unsafe` / `checked`

language design
1

For example:

impl<T, A: Allocator> Vec<T, A> {
    pub unsafe fn set_len(&mut self, new_len: usize) {
        debug_assert!(new_len <= self.capacity());

        self.len = new_len;
    }

    unsafe fn append_elements(&mut self, other: *const [T]) {
        let count = unsafe { (*other).len() };
        self.reserve(count);
        let len = self.len();
        unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };
        self.len += count;
    }

    pub fn append(&mut self, other: &mut Self) {
        unsafe {
            self.append_elements(other.as_slice() as _);
            other.set_len(0);
        }
    }
}

would be rewritten as:

impl<T, A: Allocator> Vec<T, A> {
    pub unsafe fn set_len(&mut self, new_len: usize) {
        debug_assert!(new_len <= self.capacity());

        self.len = new_len;
    }

    unsafe fn append_elements(&mut self, other: *const [T]) {
        let count = (*other).len();
        // ^ It doesn't make sense to wrap unsafe invocations in unsafe blocks
        // just to declare "I'm performing unsafe operations here"
        self.reserve(count);
        let len = self.len();
        ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count);
        self.len += count;
    }

    pub fn append(&mut self, other: &mut Self) {
        checked {
            self.append_elements(other.as_slice() as _);
            other.set_len(0);
        }
    }
}

According to The Rustonomicon 1.1. How Safe and Unsafe Interact:

The unsafe keyword has two uses: to declare the existence of contracts the compiler can't check, and to declare that a programmer has checked that these contracts have been upheld.

You can use unsafe to indicate the existence of unchecked contracts on functions and trait declarations. On functions, unsafe means that users of the function must check that function's documentation to ensure they are using it in a way that maintains the contracts the function requires. On trait declarations, unsafe means that implementors of the trait must check the trait documentation to ensure their implementation maintains the contracts the trait requires.

You can use unsafe on a block to declare that all unsafe actions performed within are verified to uphold the contracts of those operations. For instance, the index passed to slice::get_unchecked is in-bounds.

You can use unsafe on a trait implementation to declare that the implementation upholds the trait's contract. For instance, that a type implementing Send is really safe to move to another thread.

Either of the following could be true:

  1. A whole lot of people (including people who wrote the std library) misunderstood when the keyword unsafe should be used - instead of unlocking the "unsafe privilege" like this: unsafe { unsafe_fn(); }, it should actually mean "I've checked that everything required by the unsafe fn is always fulfilled" according to The Rustonomicon. It is why no unsafe block is needed in unsafe fns, there's no point checking anything when you define an unsafe fn which naturally means "I don't want to check anything cuz it's the developer's responsibility to check the conditions are met (unless the unchecked contracts of the outer unsafe fn is different from that of the invoked unsafe fn, in which case you still need to prove the "contract coersion" is safe).
  2. The Rustonomicon should be revised to reflect how unsafe should be used now.

If the 1st case is true and what The Rustonomicon said is correct, I strongly recommend that the keyword unsafe get split into unsafe / checked to match different use cases as it is more intuitive.

Any ideas?

1 Like
2

I've been using hold_my_beer as the bikeshed-avoidance version of this keyword.

An example of where it's come up before: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/broken.20and.20un-fixable.20parts.20of.20Rust/near/396012611.

3

What you said in parentheses argues the opposite: unsafe blocks should be needed in unsafe functions to call unsafe APIs -- you still need to manually check whether the contract of the function implies that what you're calling is safe.

This is why I always use:

#![deny(unsafe_op_in_unsafe_fn)]
7 Likes
4

That makes sense, but I wonder in reality exactly what is checked in:

impl<T, A: Allocator> Vec<T, A> {
    unsafe fn append_elements(&mut self, other: *const [T]) {
        let count = unsafe { (*other).len() };
        self.reserve(count);
        let len = self.len();
        unsafe { ptr::copy_nonoverlapping(other as *const T, self.as_mut_ptr().add(len), count) };
        self.len += count;
    }
}

when there isn't an explicit description of the contract of append_elements?

5

Every unsafe function should document its contract.

3 Likes
6

I think, if there is not documented contract, the contract must be “you must check that the implementation of this function is correct at your use-site”. This is of course a relatively hard to check contract; also it obliges the implementation of the unsafe function to either never change, or to consider all use-cases if ever changed; but if it’s “just” to remove code-duplication between exactly 2 use-cases, for a otherwise private function in a crate, as is the case with append_elements, it may be “fine”.

3 Likes
7

Oh wow this is actual standard library code! I thought this was a made-up example.

I would think std code would be super careful about this and have Safety comments for all its unsafe calls and functions. But there are no comments at the function header, at the unsafe calls inside the function implemention, or at the call site!

8

STD is some of the most battle hardened rust code in existence. Because it's been used for a long time. But this also means that parts of STD are old. Some parts have not been improved since well before 1.0 best practices like Safety comments are comparatively new. It's not uncommon for STD code that works not to have been brought up to modern standards. PR's are absolutely welcome.

1 Like