Why aren't we allowed to implement unsafe trait method with safe one?

[elidupree]

Yes, exactly.

[dhm]

@elidupree

So, suppose your impl looks like this:

pub struct EvenOnly;
impl<T> SliceSetIndex<T> for EvenOnly {
    fn set_unchecked(self, slice: &[T], value: T) {
        if !self.is_even() {
            unsafe { immediate_ub(); }
        }
    }
}

As you say, this doesn't violate a trait contract because the trait does not impose any requirement on the impl. But it doesn't need to. This code violates the unsafe rules in general – you've written an unsafe block to use an unsafe operation where you cannot guarantee that the operation is not UB.

But if the method is unsafe, then the line is far more blurry: unless the trait is unsafe, the "necessary preconditions" for the call not to be unsound / UB are dictated by the implementor:

#![forbid(unsafe_op_in_unsafe_fn)]
use ::std::hint::unreachable_unchecked as immediate_ub;

/// A trait for setting the value of a slice at a particular index.
///
/// (But if implementations don't actually do that, that's just a logic error.
/// Consumers of this trait aren't allowed to rely on that for memory safety.)
pub trait SliceSetIndex<T> {
    /// Calling this method with an out-of-bounds index is *undefined behavior*.
        /* Note: that doesn't imply that calling this method with
           a non-out-of-bounds index isn't! Indeed the trait is not unsafe */
    unsafe fn set_unchecked(self, slice: &[T], value: T);
}

pub struct Troll;
impl<T> SliceSetIndex<T> for Troll {
    /// Safety: must never be called.
    unsafe
    fn set_unchecked(self, slice: &[T], value: T) {
        unsafe {
            // Safety: The caller asserted this code is unreachable.
            immediate_ub();
        }
    }
}

This means that the moment you have an unsafe fn inside a non-unsafe trait, unless the author of the trait "owns all the implementations" (e.g., sealed trait pattern, or general blanket impls relying on coherence to protect them) (and in that case the author could perfectly have made the trait unsafe), then, for consumers, it is unsound in generic code to call such an unsafe method!, since otherwise Troll would cause UB despite its usage of unsafe having been sound / correct¹.

• (But obviously non-generic code can rely on the actual used types not to "troll"; it just clearly defeats the whole purpose of the trait to begin with).

¹ For those who think that the above Troll impl is not correct, know that:

1. pub struct Troll<T>(T);
   impl<T> /*SliceSetIndex<T> for */ Troll<T> {
   
       /// Safety: must never be called.
       unsafe
       fn set_unchecked(self, slice: &[T], value: T) {
           unsafe {
               // Safety: The caller asserted this code is unreachable.
               immediate_ub();
           }
       }
   }
   

   is sound.

2. Then, since:

   A non-unsafe trait cannot make toggling an inherent impl into one of that trait cause unsoundess.

   It follows that the Troll impl was sound.

[mbrubeck]

@dhm

But if the method is unsafe , then the line is far more blurry: unless the trait is unsafe , the "necessary preconditions" for the call not to be unsound / UB are dictated by the implementor:

Does this mean that such a method (unsafe method in a safe trait) can never be called on a generic type? If so, then is there any use for such trait methods at all?

[elidupree]

@dhm

A non- unsafe trait cannot make toggling an inherent impl into one of that trait cause unsoundess.

Where are you getting that quote from? My interpretation is that that is true for type-level conditions that can be checked by the compiler, but is not true in the presence of unsafe operations in unsafe methods, because unsafe methods have preconditions that are not given in the code.

Here's one explanation of why your Troll impl is unsound: By converting the inherent impl to a trait impl, you add a new way the function can be called (namely, calling it as a trait method in generic code). Since generic code is permitted to call it with any inputs that are valid according to the trait documentation, this means that the body of Troll::set_unchecked() is no longer permitted to rely on any preconditions that aren't provided by the trait documentation.

[dhm]

@elidupree

@dhm

A non- unsafe trait cannot make toggling an inherent impl into one of that trait cause unsoundess.

Where are you getting that quote from?

@elidupree

Since generic code is permitted to call it with any inputs that are valid according to the trait documentation

That point may not seem that clear then I think both our point of views are consistent with themselves, the issue here is then that Rust has never been very formal about this point: whom imposes the preconditions on a trait's associated function: the function itself (which can be changed at will by the implementors), or the trait?

So since your vision is the less foot-gunny one, it would indeed be nice if we could go with that one

[elidupree]

Also – this is mostly a side note, but I've just realized it's not always true, even in normal circumstances, that "A non-unsafe trait cannot make toggling an inherent impl into one of that trait cause unsoundness." Here's a counterexample. It relies on exploiting the rules about method resolution order; the same thing can also be achieved (in a more-legitimate-looking way) using specialization.

So if that's written down in any documentation you know of, we should make sure to fix the documentation.

[carbotaniuman]

I think a better way to think about that is that calling set_unchecked with a valid index is never UB, but the index may not get set (for memory safety purposes). unsafe is a statement that the compiler can't prove your code correct, not a license to UB.

Regarding the other question, the only way traits make logical sense is if the preconditions and postconditions are set by the trait and function. Safe traits can rely on the postconditions only for logical correctness, and unsafe traits can rely on them for memory safety.

But if the method is unsafe , then the line is far more blurry: unless the trait is unsafe , the "necessary preconditions" for the call not to be unsound / UB are dictated by the implementor

I'm unsure of where you got this from. Causing UB in violation of the preconditions is as wrong in unsafe code as it is in safe code - if I pass a valid index to IndexMut, causing UB would be an unsound. Ditto for your set_unchecked.

[dhm]

@carbotaniuman

I think a better way to think about that is that calling set_unchecked with a valid index is never UB, but the index may not get set (for memory safety purposes).

The question at hand here being, who gets to decide that? The trait or the function? My previous post pointed out that this question hasn't been properly / officially answered yet, and that's the reason we've had different views. So it is no longer a question of "logic":

• as I said, if the function owns the /// Safety contract of its unsafe fn-ness, then all I've said does apply, including:

  @mbrubeck

  Does this mean that such a method (unsafe method in a safe trait) can never be called on a generic type? If so, then is there any use for such trait methods at all?

  @RalfJung

  Really, unsafe fn in a safe trait makes no sense as such functions come with extra trait-documented preconditions (that's why they are unsafe to begin with) and you need unsafe trait because clients need to promise that these preconditions actually are sufficient to justify executing their code.

• But if we consider the trait owns the preconditions, then everything most¹ of what @elidupree said holds, as well as your

  @carbotaniuman

  Regarding the other question, the only way traits make logical sense is if the preconditions and postconditions are set by the trait and function [respectively]. Safe traits can rely on the postconditions only for logical correctness, and unsafe traits can rely on them for memory safety.

Given that non-unsafe / non-/// Safety-related preconditions are already the task of the trait, I agree that it makes more sense for the unsafe ones to follow that logic too, as I stated in my previous post. So now that should be "officially" blessed so that this design choice is fixed.

---

¹

@elidupree

Here's a counterexample.

That actually is not a counter-example, you are relying on:

@dhm

the trait "owns all the implementations" ( e.g. , sealed trait pattern, or general blanket impl s relying on coherence to protect them)

You are (ab)using privacy to emulate a sealed trait, and since unsafe interaction is based on privacy, you can get away with marking a function or a trait not unsafe to call / impl in that case.

That is, your example shows that Marker should have been an unsafe trait, since there is unsafe code relying on it.

If, for instance, we took your example and moved it to the fn realm, we would have had:

mod private {
    fn deref<T : Copy> (p: *const T) -> T
    {
        unsafe { // nobody misuses this function within this module
            p.read()
        }
    }

    pub fn exported<T : Copy> (it: &'_ T) -> T
    {
        /* deref(0 as *const u8); */
        deref(it)
    }
}

Obviously uncommenting that line will cause UB, and so does uncommenting the Marker for inside that impl within your code.

And in the same fashion that we could, and should, mark deref as unsafe despite that pattern being sound, your example:

@dhm

could perfectly have made the trait unsafe

[elidupree]

@dhm

@elidupree

Here's a counterexample.

That actually is not a counter-example, you are relying on:

The original statement didn't include "except for sealed traits" (it looks like you're saying I needed to interpret it in the context of the rest of your post, but since it originally looked like you were quoting it from somewhere else, I thought that wouldn't apply). But moreover, this counterexample can easily be tweaked to use a public, non-sealed trait. Here's the tweaked version. (edit: updated the tweaked version to preempt another possible objection)

[carbotaniuman]

Yeah, I think that statement as written is wrong, it only really holds for publicly exposed methods and traits - the fact that Rust unsafe is infectious within a model means your counterexample is valid.

[RalfJung]

@dhm

The question at hand here being, who gets to decide that? The trait or the function? My previous post pointed out that this question hasn't been properly / officially answered yet , and that's the reason we've had different views.

Given that one of the interpretations makes trait { unsafe fn } useless, IMO the other interpretation (what you call "the trait owns the precondition") is the only one that makes sense, and it is just a docs bug that this has not been written down properly yet.

Are there any benefits to the "the function in the trait owns the precondition" interpretation?

[dhm]

Yep, that's what I was saying too: I think the suggested approach makes more sense than the other one that I used to have (I just wanted it to be acknowledged there was wiggle room for interpretation there, to begin with).

So the idea would be to convert this thread into constructive documentation (for instance, here) to be explicit about this point (this reminds me that I did try to bring awareness about this, but nothing was done at the time).

[Tom-Phinney]

@carbotaniuman

Regarding the other question, the only way traits make logical sense is if the preconditions and postconditions are set by the trait and function [Added by T-P: respectively]. Safe traits can rely on the postconditions only for logical correctness, and unsafe traits can rely on them for memory safety.

I agree with this, as apparently do @RalfJung and @dhm. So what's next? Will the UCG formalize this? Will this interpretation be included in the Rustonomicon?

[RalfJung]

In terms of process probably a t-lang FCP would make most sense, or maybe they'd prefer an RFC... I'll ask the team on Zulip.