rust-lang / keyword-generics-initiative

Public repository for the Rust keyword generics initiative
https://rust-lang.github.io/keyword-generics-initiative/
Other
96 stars 11 forks source link

Is unsafe in scope for this initiative (post-MVP) ? #22

Open HadrienG2 opened 1 year ago

HadrienG2 commented 1 year ago

When writing unsafe code, I keep hitting the "unsafe code must not rely on impls of safe trait being correct" wall.

I wonder if one solution wouldn't be to allow strategic traits like Iterator to have both safe and unsafe impls, where by marking an impl unsafe, a trait implementor can testify that they have reviewed their code with the same scrutinity as they would review unsafe code, and believe unsafe code can safely rely on method and trait contracts being correctly upheld.

Do you think that this could be in scope for the keyword generics initiative, or is this a different problem that calls for different solutions ?

clarfonthey commented 1 year ago

At least from the proposal, it seems like this would be reasonable:

pub trait ?unsafe Iterator {
    type Item;
    ?unsafe fn next(&mut self) -> Option<Self::Item>;
    fn size_hint(&self) -> (usize, Option<usize>);
}

pub ?unsafe fn find<I, T, P>(iter: &mut I, predicate: P) -> Option<T>
where
    I: ?unsafe Iterator<Item = T> + Sized,
    P: ?unsafe FnMut(&T) -> bool;

The main issue is that you'd have this weird discrepancy between unsafe trait Iterator and trait unsafe Iterator, where the former is unsafe to implement, and the latter is unsafe to use.

HadrienG2 commented 1 year ago

I fear the semantics mismatch between unsafe-means-trusted and unsafe-means-unchecked is by now a syntactic wart engraved in Rust that we will need to accept living with forever... The question we need to answer is here is, does it get in the way of the idea that I'm floating around?

Some questions that come to my mind upon seeing your example written down:

The more I'm looking at this, the more I think that perhaps extending keyword generics to unsafe would be easier with the "where clause" variation of the syntax that was discussed elsewhere in those issues. Because unlike the sigil-based syntax, the where clause syntax can trivially be extended to support specifying "This method can use an unsafe implementation if this other method is trusted".

HadrienG2 commented 1 year ago

To put my questions in pseudocode...

// Question: Can a trait definition afford not to care about impls being unsafe-ready?
pub trait Iterator {
    type Item;
    fn next(&mut self) -> Option<Self::Item>;
    fn size_hint(&self) -> (usize, Option<usize>);

    // Remark: Trusted implementations should not be signaled via the unsafe
    // keyword before fn because unsafe fn means "requires an unsafe block to
    // call" / "has unchecked preconditions" and that's not what we are after here.
    // Using the trusted keyword as a placeholder until this is addressed.
    ?trusted fn nth(&mut self, idx: usize) -> Option<Self::Item>
        where
            // Question: How do I define the preconditions for an implementation to
            // be trusted by unsafe code?
            trusted: if trusted(Self::next) && trusted(Self::size_hint),
            // ...and if I want to hide my implementation details, can I do so?
            trusted: if trusted(Self as Iterator),
    {
        // Question: How do I define the preconditions for the unchecked
        // implementation to be used?
        let unchecked_impl = trusted(Self::next) && trusted(Self::size_hint);
        // ...and how do I avoid duplications if I don't want to set extra
        // preconditions for use of the unchecked implementation with respect to
        // those for being externally trusted? Maybe like this?
        let unchecked_impl = trusted();

        // Go to next iterator element, expecting it to be Some(elem)
        let next_some = |iter: &mut Self| {
            let raw_next = iter.next();
            if !unchecked_impl {
                raw_next
            } else if let Some(elem) = raw_next {
                Some(elem)
            } else {
                unsafe { core::hint::unreachable_unchecked() }
            }
        };

        // Check minimal number of expected iterator element
        let lower_bound = self.size_hint().0;

        // Fast path while iterator is guaranteed to return Some
        let num_some = idx.min(lower_bound);
        for _ in 0..num_some {
            next_some(self)?;
        }

        // Slow path when iterator may return None
        for _ in num_some..idx {
            self.next()?;
        }

        // Desired element
        self.next()
    }
}