Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
HadrienG2 opened this issue Mar 5, 2023 · 3 comments
Open

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

HadrienG2 opened this issue Mar 5, 2023 · 3 comments
Labels
question Further information is requested

Comments

@HadrienG2
Copy link

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 ?

@HadrienG2 HadrienG2 added the question Further information is requested label Mar 5, 2023
@clarfonthey
Copy link

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
Copy link
Author

HadrienG2 commented Mar 5, 2023

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:

  • Is ?unsafe a decision that the code defining the trait should always make, or is it something that can solely concern implementors and users of the trait? In other words, can a trait not defined as ?unsafe still be implemented as unsafe?
    • This has the benefit that crates defining traits need not care about how unsafe code interacts with them.
    • The tradeoff is that they cannot define how unsafe code interacts with them, i.e. set specific invariants that all unsafe impls agree to upheld. I would argue however that in a keyword generics context, this is probably a good thing: we don't want unsafe impls to follow a completely different contract with respect to safe impls (in which case they should probably be an impl of a subtrait), implementing the safe contract correctly should be all we need.
  • Often, trait methods have inter-related semantics and are used together. Does it make sense to have a way to say "all those methods should have an unsafe impl or else I'm falling back to the safe path of assuming broken impls"?
    • If so, it might make sense to have a shorthand to implement the whole trait as unsafe in addition to a fine-grained way to only provide unsafe impls of specific methods.
  • Trait methods can also have default impls, which could sometimes benefit from assuming an unsafe implementation of other methods like any other code. What would the syntax for this look like? E.g. how can I say that my Iterator::nth default impl uses an unsafe impl that assumes (via unreachable_unchecked) that next() will return Some N times if an unsafe impl of size_hint tells me that this is true and next() is impl'd unsafely so I can assume it honors size_hint?

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
Copy link
Author

HadrienG2 commented Mar 6, 2023

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()
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants