RFC: Overconstraining and omitting unsafe in impls of unsafe trait methods

[Centril]

Rendered

---

This RFC allows safe implementations of unsafe trait methods. An impl may implement a trait with methods marked as unsafe without marking the methods in the impl as unsafe. This is referred to as
overconstraining the method in the impl. When the trait's unsafe method is called on a specific type where the method is known to be safe, that call does not require an unsafe block.

A simple example:

trait Foo { unsafe fn foo_computation(&self) -> u8; }
struct Bar;
impl Foo for Bar {
    // unsafe <-- Not necessary anymore.
    fn foo_computation(&self) -> u8 { 0 }
}
fn main() {
    // unsafe { <-- no unsafe needed!
    let val = Bar{}.foo_computation();
    // other stuff..
    // }
}

---

This RFC was written in collaboration with @cramertj whom it was my pleasure to work with.

cc @withoutboats @aturon

[sfackler]

Do you have some real world examples of traits and implementations that would take advantage of this?

[Centril]

Here's one example: https://doc.rust-lang.org/std/slice/trait.SliceIndex.html#tymethod.get_unchecked
with impls:

• https://doc.rust-lang.org/src/core/slice/mod.rs.html#967-999
• https://doc.rust-lang.org/src/core/str/mod.rs.html#1827-1853

While this falls under "trivial example", avoiding unsafe where possible is still nice =)

...stay tuned for more

[sfackler]

Does there exist any code that has ever called slice.get_unchecked_mut(..)?

[Centril]

Oh; you were referring to the calling side of things and not the motivation, I see. then slice.get_unchecked_mut(..) does not apply (I think). My bad ;)

[sfackler]

I am trying to figure out what problem that currently exists this RFC is proposing to solve. "If this feature existed it would enable me to make a thing that was previously impossible", or "If this feature existed I could make this thing I wrote better".

[cramertj]

I think I agree with you that specialization is a better long-term solution.

[sfackler]

Why is adding a new unstable language feature a shorter term solution than using another unstable language feature?

[cramertj]

@sfackler Perhaps I'm using it wrong, but I don't believe the current version of specialization allows this specific impl. The following errors with "conflicting implementations of trait ImmovableFuture for type MyFutureCombinator<_>":

#![feature(specialization)]

trait Future {}
trait ImmovableFuture {}

default impl<T> ImmovableFuture for T where T: Future {}

struct MyFutureCombinator<T>(T);
impl<T> Future for MyFutureCombinator<T> where T: Future { }
impl<T> ImmovableFuture for MyFutureCombinator<T> where T: ImmovableFuture { }

[cramertj]

Either way, I'd expect this feature to be much quicker and easier to implement and stabilize than even the most minimal version of specialization.

[sfackler]

Then this can provide some motivation for finishing specialization! IIRC that kind of implementation was covered in at least some revisions of the specialization RFC.

Are there any more motivating examples for this feature? I am kind of concerned with adding a new language feature that will be used for a single trait.

[eddyb]

@nikomatsakis Didn't we used to have bugs because of this?

[petrochenkov]

@eddyb

Didn't we used to have bugs because of this?

I remember something like this, but can't find where the change was done.
IIRC, sometime in <=2014 unsafe on fns was a "modifier" like const and not part of the type, this caused issues, and then unsafe was made a part of the type, and fn and unsafe fn were't related by subtyping, only coercions, and impl items and trait items required subtyping, so impl items had to use unsafe if the trait used unsafe.

[withoutboats]

What is the story on APIs that currently use unsafe fn to mean "unsafe to implement"? I think those APIs are wrong, but they exist. It should at least be listed in the drawbacks that this will allow users to write unsafe code without writing unsafe by not upholding the invariants in their implementation.

[burdges]

Isn't the point of unsafe fn that they can only be called from unsafe blocks? At present, "bodies of unsafe functions are effectively unsafe blocks" but actually that sounds like a mistake, i.e.

unsafe fn dangerous(foo: Foo) -> Bar {
    std::mem::transmute(foo)  // A priori I'd think this should be an error and instead require an unsafe block
}

In particular, you might want safe blocks inside an unsafe fn simply because it makes the unsafe fn easier to write.

Also, if you really need an unsafe fn to be callable outside an unsafe block on a specific type, then you might instead allow an inherent method to override it.

trait Dangerous { unsafe fn dangerous(self) }
impl Dangerous for ActuallySafe { ... }
impl ActuallySafe {  fn dangerous() { unsafe { self.dangerous() } }  }  // Not sure that's the syntax you want.

[Lokathor]

Hopefully we can get the inverse as well, though I would understand if that was a separate RFC.

[Lokathor]

@burdges If you're already writing an unsafe function then having to put an unsafe block inside of it is silly to say the least.

[kennytm]

The unsafe fn bug issue mentioned in #2316 (comment) is rust-lang/rust#23449.

Currently, fn is a subtype of unsafe fn. This should be a coercion relationship, pursuant to our strategy of limiting subtyping to region relationships. This also implies that one cannot implement an unsafe trait method with a safe fn, which is useful for solving the "trusted iterator length" problem.

Searching for "trusted iterator length" problem points to https://internals.rust-lang.org/t/the-trusted-iterator-length-problem/1302, where one of the solutions is about unsafe fn exact_size() in the same manner as #2316 (comment). The eventual solution we've picked is an unsafe trait TrustedLen though, so the problem is irrelevant to iterators now.

I think "unsafe to implement" stuff should be performed through unsafe trait/unsafe impl, but this will force those packages to break their own API compatibility :(.

[mark-i-m]

What is the story on APIs that currently use unsafe fn to mean "unsafe to implement"? I think those APIs are wrong, but they exist. It should at least be listed in the drawbacks that this will allow users to write unsafe code without writing unsafe by not upholding the invariants in their implementation.

Actually, I would like to dig a bit deeper here. If I have a trait:

trait FooTrait {
    unsafe fn foo();
}

The unsafe could mean different things to different people:

• For someone implementing impl FooTrait for Bar, does it mean "unsafe to implement"?
• For someone using <Bar as FooTrait>::foo, does it mean that foo is "unsafe to call"?

I guess this RFC is proposing that it does not mean the former and may or may not mean the latter...

Fundamentally, I don't think it even makes sense to declare an abstract fn unsafe unless there is a default implementation. It really doesn't make sense to me for an interface to claim that any implementation of a function must contain unsafe code. How would it know?

I agree with @kennytm: unsafe trait/impl should be used for "unsafe to implement" since it seems like "unsafe to implement" only makes sense for markers (i.e. no implementation).

I think this will answer one of the questions I had when reading the RFC: what should the rustdocs for Bar say? Is foo unsafe or not?

[withoutboats]

@mark-i-m I wrote a blog post about what I believe is the correct way to interpret unsafe in this context: https://boats.gitlab.io/blog/post/2018-01-04-unsafe-abstractions/

Fundamentally, I don't think it even makes sense to declare an abstract fn unsafe unless there is a default implementation. It really doesn't make sense to me for an interface to claim that any implementation of a function must contain unsafe code. How would it know?

There's a subtle but important distinction that I think you're missing: being marked unsafe doesn't mean you can do any unsafe anything in that function. An unsafe fn has some invariant that callers must uphold to make it safe to call. In the case of associated functions, the trait declares what that invariant is, and any impl that requires additional invariants to be safe is incorrect.

So its not at all tied to having a default impl; the point is that every impl has to rely on the same invariants that the trait declares.

[Lokathor]

So, could we summarize that whole thing with "Any time unsafe is on a Trait the implementer must verify it for safety, any time unsafe is on a function or method (trait or inherent) the caller must verify it for safety at each use." Is that correct?

[cramertj]

@Lokathor Yes, that's exactly how I interpret it.

[nikomatsakis]

I agree with @kennytm: unsafe trait/impl should be used for "unsafe to implement" since it seems like "unsafe to implement" only makes sense for markers (i.e. no implementation).

I think this is the right interpretation, though in the past I've had another POV (roughly: "unsafe means read the comments; somebody has an extra job here"). To be honest I think we should revisit the design of unsafe to help us make this clearer (who is imposing what obligations on whom), but I'm not 100% sure yet what I think.

[mark-i-m]

Hmm... @withoutboats's blog post was very thought provoking! It bothered me that the use of unsafe keywords mean different things for traits vs. functions/methods vs. unsafe blocks, though. So... I tried to simplify things a bit, and I think I have: all items and associated items actually behave the same way wrt unsafe. unsafe blocks are the only usage of unsafe that have a different meaning.

My proposal (hopefully this doesn't have too many holes):

• We look at each (associated) item as having preconditions and postconditions.

  • unsafe on an (associated) item always indicates that the user must uphold some documented preconditions. Whenever, this is the case, the user must indicate syntactically that they do so by using an unsafe block.
  • unsafe traits have no preconditions and they provide some postcondition, as defined by the trait designer (the "abstractive" defined by @withoutboats's post). Because they have no preconditions, they can be used anywhere without the unsafe block. This is already the case: I can write a T: Sync bound outside an unsafe block.
  • unsafe fn (functions and methods) have the preconditions and postconditions defined by the designer (both the pre- and post-conditions correspond to the "abstractive"). unsafe in this context means two things (which are consistent with the above):
    • Primarily, it means that the caller needs to uphold the preconditions (as it did with traits)
    • For convenience, it also means that the entire body is an unsafe block. This is why we can use other unsafe items in an unsafe fn.
  • unsafe impl is a special case of the unsafe block conceptually. Inside the conceptual unsafe block there is a magic function call that upholds the postconditions of the unsafe trait.

• One implication is that "unsafe to implement" is really an illusion. There is no such thing. Instead, it is sugar for magic that says "I promise to uphold the postconditions".

• WRT this RFC, it makes sense that if an unsafe function or method doesn't actually use any unsafety, it doesn't rely on any of the preconditions! So, the user shouldn't have to promise that it upholds any preconditions (i.e. no unsafe block needed).

For an explanation of how I came to this definition of unsafe see the following long-winded details...

Details

• One of the points on which @withoutboats and my thinking had differed was that I have always thought of unsafe as a guarantee by the implementor -- never the caller. In this light, unsafe associated items don't really make sense for the reasons mentioned above. In contrast, one might view unsafe as described by @withoutboats and summarized by @Lokathor in the above comment:

  Any time unsafe is on a Trait the implementer must verify it for safety, any time unsafe is on a function or method (trait or inherent) the caller must verify it for safety at each use.

• I think that this ignores the fact that code inside of an unsafe block or fn does need to make some guarantees and uphold some invariants (e.g. it doesn't do any actually-unsafe stuff and it does do whatever it claims to do -- that is the "contract" mentioned in this RFC). (Also, to be clear, we are talking purely about what rustc considers "safe" or "unsafe" here -- that is, memory safety and data race freedom).

• The precondition/postcondition view means that for functions/methods/traits both the user and the implementor mutually must guarantee something and they must mutually trust the guarantees given to them:

  • The user:
    • Guarantees that the preconditions are upheld.
    • Trusts that the postconditions are upheld
  • The implementor
    • Trusts that the preconditions are upheld
    • Guarantees that the postconditions are upheld

• To use a couple of examples from @withoutboats's blog post:

  • impl Sync for Foo:
    • The precondition is true (a user can always uses a T: Sync bound)
    • The postcondition is it is safe to share a reference to Foo across threads
  • slice::from_raw_parts(ptr, len)
    • The precondition is ptr points to a valid pointer and len is the correct length
    • The postcondition is that the returned slice has the correct location and length AND nothing else has happened.

• Interestingly, this reveals that an unsafe function is not equivalent to a function whose entire body is inside an unsafe block. Why? Because an unsafe function makes a statement about preconditions that a non-unsafe function does not.

• Also interestingly, an unsafe block is not equivalent to a private inline unsafe function that only gets called once. Why? Because an unsafe block cannot have preconditions. This is what makes an unsafe block useful, in fact. Otherwise, you might need an unsafe block to be inside an unsafe block sometimes (see the following).

• So how does this all tie back to the RFC? Well, we need to decide what the unsafe keyword means to know when we can elide it. In other words, when I see the unsafe keyword, what is it making a statement about? Under the precondition/postcondition view, I see a few possible options for how we could define unsafe on an (associated) item:

  1. unsafe means that the user is making a guarantee, not the implementor. In other words, the user promises it is upholding the preconditions. Under this definition of unsafe, we may accept this RFC:

     • traits already make sense because you don't have to use unsafe to use a T: UnsafeTrait bound, and this is consistent with unsafe trait impls not having preconditions.
     • function/method implementation that actually rely on the precondition require the user to use unsafe because we need to just trust that the user actually upholds the preconditions. On the other hand, if the implementation doesn't assume any preconditions, the user doesn't need to promise anything, so no unsafe is needed.
     • unsafe blocks are the way a user says "trust me; I uphold all preconditions".
     • This viewpoint seems consistent with @withoutboats's viewpoint.

  2. unsafe means that the implementor is making a guarantee, not the user. In other words, the implementor promises that it is upholding a postcondition. This model doesn't really make sense to me, and I don't know how unsafe blocks fit in if the user doesn't need to promise anything.

  3. unsafe means the user and implementor are mutually making guarantees. The user promises it is upholding all preconditions. The implementor promises it is upholding all postconditions.

     • To use an unsafe trait in a bound, you would somehow need to be inside an unsafe block. This not consistent with current rust. Intuitively, it is like saying that relying on Mutex is unsafe because you have to trust that Mutex is actually Send + Sync. So this definition doesn't seem to work.

• So overall, only the first definition seems to work.

I write all of this because I think that if we are to move forward with a feature like this RFC, we need to nail down the definition of unsafe once and for all. I would like an official definition somewhere in the RFC. It would also be good to update the nomicon and Book to match the definition so that people build a good intuition for when to use it.

[Lokathor]

That's, like, way complicated my friend. Let's try going back to the simple version and building from there. Also, let's try to avoid introducing any new special terms.

---

• unsafe means that there are special rules that must be followed to maintain memory safety which cannot be expressed in the type system and cannot be checked by the compiler.
• unsafe on a Trait (such as Send and Sync) means that you must satisfy the documented conditions before you implement it.
• unsafe on code that you call (bare function, inherent method, or trait method) means that you must satisfy the documented conditions before you call that code.
  • Note that in the case of trait methods the implementing code must assume that only the documented conditions given in the trait's definition have been met, it cannot impose extra conditions or it would break the promises of the code being generic.
  • If you want to add your own limitations to an implementation that's fine as long as you maintain safety while doing it. For example, most things that implement Index will panic if you pass an out of bounds index (a panic is not fun, but it is memory safe).

[Centril]

cc @eternaleye wrt. untrusted vs. unsafe

[eternaleye]

Copying a portion of my message elsewhere:

Currently, Rust has three effects:

• unsafe, which allows calling unsafe fns and dereferencing raw pointers
• impure, the rough inverse of the const restriction
• untrusted, which indicates that commonsense invariants (like Ord being a total order) may be unsatisfied (the inverse of the unsafe trait/impl restriction).

Today, impure and untrusted are default-permitted, and unsafe is default-forbidden. In addition, unsafe and impure can only be controlled on functions, while untrusted can only be controlled on traits.

I negated the definitions of some of these specifically to make them effects: Something that grants the body/callee additional powers, while constraining the user/caller.

Rust has a rudimentary effect system, but discussing it is complicated by several things:

• Aliasing of keywords (unsafe) for distinct effects
• Lack of a consistent 'polarity' (const and unsafe trait are 'backwards' as effects, hence me describing them as 'restrictions' instead)
• Lack of clarity on terminology, which could be improved by leveraging the POPL88 paper that first described effect polymorphism - unsafe fn introduces an effect, and unsafe {} masks it.

EDIT: With these reframings in place, I'd argue that:

• fn should be valid in impl where declared as unsafe fn in the trait ("I make no use of the unsafe effect, even though I could.") (This RFC)
• const fn should be valid in impl where declared as fn in the trait ("I make no use of the impure effect, even though I could.") (RFC 2237)
• unsafe impl on plain trait should be kosher ("I make no use of the untrusted effect, even though I could.") (No RFC yet?)

It would also seem to argue that:

• Trait bounds of parameters to unsafe traits should be constrainable to unsafe trait (if upholding invariants relies on them) as untrusted currently has no masking construct because it's universally masked (which is potentially very risky).

[mark-i-m]

@Lokathor The question is who does unsafe indicate is supposed to uphold the special terms and conditions? I would argue that it is the user, rather than the implementor. In @eternaleye 's terminology, the one using the effect must mask it.

@eternaleye

Trait bounds of parameters to unsafe traits should be constrainable to unsafe trait (if upholding invariants relies on them) as untrusted currently has no masking construct because it's universally masked (which is potentially very risky).

I argue that unsafe impl is actually masking of a higher-kinded unsafe function that makes a type impl a trait. The calling of this higher-kinded function is the effect:

unsafe impl is a special case of the unsafe block conceptually. Inside the conceptual unsafe block there is a magic function call that upholds the postconditions of the unsafe trait.

[eternaleye]

@mark-i-m That's not what masking means.

Masking is stopping the outward (resp. inward) propagation of an effect (resp. restriction) marker.

• unsafe fn is infectious outwards until masked by unsafe {}
• const fn is infectious inwards (as a restriction, dual to impure), and is not possible to mask
• unsafe trait has no infectiousness at all, because it's silently masked everywhere - and that's potentially dangerous, because as a restriction it should infect inwards, into its parameters' trait bounds.

[mark-i-m]

@eternaleye I think we are talking about the same thing. "stopping the outward propagation of an effect" is isomorphic to promising that you fulfill all preconditions. Not fulfilling all preconditions forces you to assume a precondition, which someone else must fulfill (which is propagation)...

[eternaleye]

@mark-i-m: Mm, I disagree. I'm going to show by comparison with unsafe fn (because it's clearer with the 'effect' polarity) and const (to give an example with the 'restriction' polarity) why this isn't the case.

So, with unsafe fn, there are three places where the unsafe effect comes into play:

trait Foo {
    // Here, in the trait-side signature
    unsafe fn foo();
}

impl Foo for () {
    // Here, in the impl-side signature
    unsafe fn foo() {
        // Here, in the impl-side body
    }
}

It's clear that unsafemust appear in the trait-side signature if it appears in the impl-side signature, and there is no way to mask it between those two places - it is "infectious outward" from impl signatures to trait signatures.

In addition, unsafe, must appear in the impl-side signature if it appears in the impl-side body - again, it's "infectious outward" from impl bodies to impl signatures. However, we can use unsafe {} inside the impl body to stop the infection, through masking.

Now, let's look at const:

trait Foo {
    // Here, in the trait-side signature
    const fn foo();
}

impl Foo for () {
    // Here, in the impl-side signature
    const fn foo() {
        // Here, in the impl-side body
    }
}

Here, the infection moves inwards - if it appears in the trait signature, it must appear in the impl signature, and if it appears in the impl signature, one must only use const fns in the impl body. There is no way to mask this - no incantation that will allow calling a mere fn in a const fn context.

Now for unsafe trait and unsafe impl:

// Here, in the trait-side signature
unsafe trait Foo {
     fn foo();
}

// Here, in the impl-side signature
unsafe impl<B: Bar> Foo for () {
    fn foo() {
        // Here, in the impl-side body
        <B as Bar>::bar()
    }
}

Note how, compared to const, the signatures are now the trait header rather than the function header - but also note that the third location is still present. Just like const, the infection propagates inwards - if unsafe appears on the trait-side signature, it must appear on the impl-side signature.

However, the impl-side body calls Bar::bar(), which is not subject to any restrictions. As a result, the developer of Bar::bar() could, at some future date, change its implementation in a way that violates the documented invariants that this implementation of Foo is relying on to uphold its own invariants.

This is where unsafe trait/unsafe impl should infect - into Bar - and does not. In fact, there is not even a syntax to explicitly infect Bar. As a result, any unsafe trait or unsafe impl with supertraits or generics bounded by traits is on shaky ground.

Anyway, in none of these examples does masking occur between trait-side signature and impl-side signature - in both cases where it's possible, it occurs between impl-side signature and impl-side body; explicitly in one case, and implicitly in the other.

EDIT: Note that this actually affects std::iter::TrustedLen - it has Iterator as a supertrait, and the Iterator implementation is not constrained. The only protection in that case is that both traits are in std, and thus must be implemented in the same place as the type - however, this does nothing to protect against a later maintainer looking only at the Iterator implementation, and breaking the invariants that TrustedLen relies on because they didn't realize it was there due to the lack of an unsafe impl mark.

[Lokathor]

@mark-i-m

The question is who does unsafe indicate is supposed to uphold the special terms and conditions? I would argue that it is the user, rather than the implementor.

Perhaps I was somehow unclear in my explanation?

• Under the current rust rules, the caller of an unsafe fn (of any sort) must follow the extra conditions.
• Under the current rust rules, the implementation writer of an unsafe trait impl must follow the extra conditions.

That's not a proposal by me for some future version of rust, that's a statement of the current rules of rust as it exists today.

Personally, I can't even envision another way for it to work. When you say "the user, rather than the implementor", I don't know what you mean. If you impl Send for Foo, is everyone who tries to send a Foo across threads supposed to verify that it's safe to move across threads? That would be horrible.

[mark-i-m]

@eternaleye Thanks for the clarification :) I think I don't quite follow you here:

However, the impl-side body calls Bar::bar(), which is not subject to any restrictions. As a result, the developer of Bar::bar() could, at some future date, change its implementation in a way that violates the documented invariants that this implementation of Foo is relying on to uphold its own invariants.

bar's API is the only invariant foo can rely on if bar is not unsafe. If it relies on something that bar doesn't guarantee, foo is buggy, and if bar breaks the guarantee bar is buggy or a breaking change has occurred. Why should any of these cases "infect" Bar? They can happen in completely safe Rust if someone makes any breaking change or has buggy code. Am I missing something?

Anyway, in none of these examples does masking occur between trait-side signature and impl-side signature - in both cases where it's possible, it occurs between impl-side signature and impl-side body; explicitly in one case, and implicitly in the other.

But the impl is not the "user" of the unsafe trait, I think. Rather, someone who uses a T: UnsafeTrait bound is the user. The unsafe is masked there, since we don't have to write unsafe T: UnsafeTrait (fictitious syntax).

---

@Lokathor

My goal was to come up with a (proposed) general formulation so we can see what we is possible in the future and consistent with current rust. I think the formulation I came up with does that, and I still believe it is consistent with both your/@withoutboats's model and @eternaleye's... but I seem to be doing a bad job of explaining it 😛

When you say "the user, rather than the implementor", I don't know what you mean. If you impl Send for Foo, is everyone who tries to send a Foo across threads supposed to verify that it's safe to move across threads? That would be horrible.

In the case of a Trait, I argue the "user" is any code with a bound like T: Trait. I agree that it would be horrible for all users of a trait to verify preconditions, and my understanding of both my model and yours says that we don't need to, which is great because we currently don't...

[eternaleye]

@mark-i-m This calls back to the first post I made in this thread - specifically, the quoted part:

• untrusted, which indicates that commonsense invariants (like Ord being a total order) may be unsatisfied (the inverse of the unsafe trait/impl restriction).

In particular, unsafe trait and unsafe impl mean that unsafe fn elsewhere is allowed to rely on documentation invariants (rather than compiler-checked invariants) for upholding memory safety. An unsafe fn is allowed to violate memory safety if TrustedLen misbehaves, but not if Ord misbehaves.

However, there is no way for unsafe trait to be transitive. As a result, any case where the behavior of an unsafe trait relies on the behavior of a mere trait, the trust model falls apart. TrustedLen is a good example, here - it has Iterator as a supertrait, but its only behavior is to say that Iterators size_hint method has an extra invariant.

Considering unsafe impl TrustedLen {} is a single, easy-to-miss line, and one that may be after the iterator implementation, this runs the risk of a user altering the behavior of size_hint without ever realizing there are additional invariants to uphold.

By contrast, if unsafe trait TrustedLen: Iterator {} was instead unsafe trait TrustedLen: unsafe Iterator {}, the impl header of iterator would say unsafe impl - warning the user that there are invariants to uphold here.

This problem is further magnified if instead of TrustedLen and Iterator (which are both in std), you had Foo and Bar, with different maintainers for each trait, and for each impl of each trait (by having Bar be a bound on an impl generic parameter, rather than a supertrait). With four maintainers total, the lack of infectiousness is a serious barrier to getting it right, because the compiler won't restrict the bound to unsafe impls.

[burdges]

I still do not understand why unsafe fn is infectious inwards. It's perfectly normal to write fns that benefits from alternating between safe and unsafe, like safe1(); unsafe { not_safe(1) }; safe2(); unsafe { not_safe2(); }. We do this all the time to help attach lifetimes correctly, but other usages exist. Why should this pattern be forbidden inside unsafe fns?

[eternaleye]

@burdges Firstly, unsafe fn is not infectious inwards (a restriction), it's infectious outwards (an effect). Second, you are conflating "masking causing the infection to stop spreading" with "not being infectious". Some code:

static mut foo: u32 = 0;

fn safe1() {
    println!("Just starting");
}

// The signature must have `unsafe` because...
unsafe fn not_safe(x: u32) {
    // ... the body uses `unsafe` and infected the signature
    foo += x;
}

fn safe2() {
    println!("Almost done");
}


// The signature must have `unsafe` because...
unsafe fn not_safe2() {
    // ... the body uses `unsafe` and infected the signature
    foo -= 1;
}

// The signature isn't infected because...
fn example() {
    // ...safe functions do not infect...
    safe1();
    // ...and masking stops the spread...
    unsafe {
        // ...of any infection present.
        not_safe(1);
    }
    safe2();
    unsafe {
        not_safe2();
    }
}

EDIT: We lack a "block-scoped effectless context" construct, but calling a safe fn from an unsafe fn context does keep the body of that fn safe - if it infected inwards, that would not be the case. Having unsafe? {} would not add expressive power the way unsafe {} does - it'd be mere syntax sugar.

Note that infection refers to the marker - the keyword unsafe. It's infectious in the exact same way as Haskell's IO, with unsafe {} taking the place of unsafePerformIO.

[matklad]

I have learnability concern here. The semantics of unsafe traits and unsafe methods in traits is complicated, and new users are constantly confused about this. I feel that adding an extra "you can, but not required, put an unsafe here" clause to this area increases complexity in an area which already complex enough to cause API bugs due to misunderstanding (example from work: near/borsh-rs#24).

[Lokathor]

I object to the idea that it's complicated. I think it is just very poorly explained most of the time. We just need to make our docs better.

[nikomatsakis]

I wonder if the trusted/unsafe distinction would make this distinction easier. I am increasingly in favor of such a move. It could definitely be managed with an edition with relative ease.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[Aaron1011]

I saw a few comments (#2316 (comment), #2316 (comment)) about the dangers of accidentally omitting unsafe on an implementation. I'm also concerned about this - as others have said, I think it would be good to require some kind of explicit opt-in (via a keyword or attribute).

[nikomatsakis]

Specifically the concern, @Aaron1011, is that people might omit unsafe but in fact it is still important, but for subtle reasons that don't show up in terms of requiring unsafe operations.

[scottmcm]

@Aaron1011 We discussed this in the lang team meeting today, but the consensus in the meeting was that it was a known and tolerable danger.

---

For me personally, it was my biggest worry with this. From a type system and semantic perspective on correct code it's absolutely fine, basically just a form of variance. I was eventually convinced that the risk is tolerable because it's already the case for functions and non-trait methods that one can leave off a necessary unsafe without the compiler complaining at all. The canonical example here being Vec::set_len -- that would compile just fine without being unsafe fn, but would be unsound. So I think the risk may actually be less here, since the compiler error message for "here, copy-paste this for the methods you need to implement" will probably have unsafe fn.

Of course, that just makes it yet another thing that makes me want unsafe fields...

[RalfJung]

So this is referring to this problem?

[scottmcm]

@RalfJung No, referring to the "footgun" mentioned in #2316 (comment)

My understanding is that the lang team is currently in agreement that the way to impose requirements on a trait implementer is to make the trait an unsafe trait, whereas an unsafe fn in a trait means that the caller is required to manually uphold the safety preconditions mentioned in the trait documentation for that method (and the implementation of that method cannot rely on more than what was in the trait method's preconditions). But we did not discuss that issue today.

[RalfJung]

No, referring to the "footgun" mentioned in #2316 (comment)

I don't understand the footgun from that comment, unless the footgun is what is described in #2316 (comment). As in, I don't even see what could possibly go wrong when one writes a safe fn to satisfy an unsafe fn signature. Why is it a footgun to do this implicitly?

The rest of what you say sounds like this is indeed about #2316 (comment).

[casey]

Today, that is only supported by marking the trait unsafe, but some users have use cases in which it is possible to uphold the invariant through a provided method, and only implementers which override that method need to be concerned with upholding the invariant. In that case, it seems convenient to use an unsafe provided method, so that only implementers overriding that method use the unsafe keyword, even though this has inverted the meaning of unsafe in that context from what was originally intended.

What is the lang team's view regarding this issue? I've used this in my own code, and found it a convenient way to indicate that an implementor of a method must fulfill obligations or else safety invariants may be violated. I've only used it a small handful of times, so I wouldn't be incredibly sad if it went away, although I have found it useful.

[Mark-Simulacrum]

As in, I don't even see what could possibly go wrong when one writes a safe fn to satisfy an unsafe fn signature. Why is it a footgun to do this implicitly?

I think the argument is essentially that if you didn't put unsafe on a function (whether it's a trait method or otherwise), but use something unsafe internally, it is possible that you didn't prove the obligation that the use of an unsafe operation created, having intended to push it up to the caller, without noticing that the function is not unsafe.

When writing trait methods that are unsafe in the trait definition, it is plausibly somewhat easier to do this by accident, as you may be reading the documentation on the trait definition which indicates what the caller must uphold and believe that the caller must uphold those things in your method, forgetting that you need to also mark the method as unsafe in the implementation. Anyone methodically making sure that safety obligations are upheld should easily get this right, perhaps even with compiler/lint assistance, but this is my understanding of this concern.

Today, that is only supported by marking the trait unsafe, but some users have use cases in which it is possible to uphold the invariant through a provided method, and only implementers which override that method need to be concerned with upholding the invariant. In that case, it seems convenient to use an unsafe provided method, so that only implementers overriding that method use the unsafe keyword, even though this has inverted the meaning of unsafe in that context from what was originally intended.

What is the lang team's view regarding this issue? I've used this in my own code, and found it a convenient way to indicate that an implementor of a method must fulfill obligations or else safety invariants may be violated. I've only used it a small handful of times, so I wouldn't be incredibly sad if it went away, although I have found it useful.

I cannot speak for the lang team, but my understanding from discussions has been that the position is that defining an unsafe method in a trait -- like with any other unsafe function -- does not allow callers to make any assumptions about the implementation of that function. The only way for a trait author to guarantee that implementations uphold some constraints is to make the trait itself unsafe.

[alercah]

I cannot speak for the lang team, but my understanding from discussions has been that the position is that defining an unsafe method in a trait -- like with any other unsafe function -- does not allow callers to make any assumptions about the implementation of that function. The only way for a trait author to guarantee that implementations uphold some constraints is to make the trait itself unsafe.

+1000 Unsafe functions impose requirements on their callers. Unsafe traits impose requirements on their implementors. This has long been the meaning of unsafe in these contexts and should not be muddied.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

[kennytm]

... are we going to merge this?

[nikomatsakis]

I'm reviewing my lang team action items and I see that I was supposed to summarize details of learnability our meeting from 2021-05-25. In the meantime, many of those same points have been made, but I thought I'd drop it in here for reference...

In short, it's true that the meaning of unsafe is quite context dependent:

• Marking a trait unsafe: indicates that the impl has proof obligations (which users of the trait can rely on)
• Marking a block unsafe: Indicates that the block has proof obligations (which the surrounding code can rely on)
• Marking a trait unsafe: Indicates that the impls of the trait have proof obligations (which users of the trait can rely on)
• Marking a function or method unsafe: Indicates that the callers of the function have proof obligations (which the function can rely on)

This particular RFC doesn't seem to make that situation worse; it could even help to clarify, as otherwise it makes adding unsafe in an impl mandatory which can be misleading. I've sometimes found that confusing myself, and I recall toying with the idea of saying that it's ok to use this as a way to add requirements on the impl, though I now think that's a bad idea. This RFC codifies the requirement and in many ways makes it clearer, since you can demonstrate what it means. As @cramertj said, he finds it surprising to have to write 'unsafe' when there is no unsafe code around.

[nikomatsakis]

Speaking personally, I have definite interest in exploring a trusted keyword or some such to make the distinction between "this item has proof obligations" versus "users of this item have proof obligations" clearer. I'd also like to see unsafe fields or other mechanisms to capture more of the places where proof obligations can be incurred (and even, though not in the short term, some more mechanized ways of expressing and discharging those obligations).

[Lokathor]

@nikomatsakis you wrote "Marking a trait unsafe" twice in your list. presumably one of those bullet points is for trait methods.

[scottmcm]

Huzzah! The @rust-lang/lang team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here: rust-lang/rust#87919