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.

[mark-i-m]

@eternaleye Ah, that's a great example!

Under my model, TrustedLen is an API bug - that is, it incorrectly uses unsafe. Here's what I mean:

• unsafe impl TrustedLen for MyIter is a statement that TrustedLen's _post_conditions are upheld by <MyIter as Iterator>::size_hint's documented _post_conditions. But unsafe has nothing to do with _post_conditions; rather, unsafe is a statement about _pre_conditions.
• A better API would be something like

  trait TrustedLen: Iterator { // not `unsafe`
    fn size_hint(&self) -> usize; // also not `unsafe`
  }

  Then, the implementor of TrustedLen would be forced to also provide a valid impl that satisfies its postconditions. And other crates can rely on compiler enforced postconditions (not documented postconditions) if they need that property of an iterator for their postconditions.
• In general, I don't think Rust as a good way right now of expressing "untrusted" as you describe it (IMHO unsafe is not the correct way to do that). That is, there is no good way to say "my documented postcondition depends on someone else's documented postcondition". I guess this would end up being something very like C++'s friend, and I really don't like those; IMHO they break information hiding.

[eternaleye]

@mark-i-m: And my point all along has been that your model doesn't seem to match the consensus, and the way you arrive at your model is unsound as well.

I have consistently seen unsafe trait described as upholding extra invariants. This is a very specific term: An invariant is true at all points, as compared to a precondition (true before, but not necessarily within or after) or a postcondition (true after, but not necessarily before or within). There has been absolutely no indication that they were intended to be precondition-only; see the RFC that introduced them.

Weakening an invariant to a precondition/postcondition pair is entirely unsound - in fact, Solidity making this mistake was the root cause of a severe contract bug in Ethereum (theDAO).

Also, even your new API still falls afoul of what I'm talking about, and also fails to meet the reason TrustedLen exists in the first place. You propose

trait TrustedLen: Iterator { // not `unsafe`
  fn size_hint(&self) -> usize; // also not `unsafe`
}

However, because it is not an unsafe trait, unsafe code is not permitted to rely on its behavior for memory safety - the entire reason the trait exists in the first place. Let's fix that.

unsafe trait TrustedLen: Iterator { // "`unsafe` code may rely on this"
  fn size_hint(&self) -> usize; // not `unsafe`
}

Cool, now let's implement it.

unsafe impl TrustedLen: Iterator { // "`unsafe` code may rely on this"
  fn size_hint(&self) -> usize { // not `unsafe`
     <Self as Iterator>::size_hint(self).0
  }
}

Oh, hm. This code that unsafe code may rely on itself relies on code that... unsafe code is not permitted to rely on. That's problematic. Someone could come along, change the Iterator implementation's size_hint method without ever realizing that unsafe code was (indirectly) reliant on it, and things start exploding.

The problem, @mark-i-m, is that Rust already has the untrusted effect (exposed in its dual form as the "trusted" restriction) - it has just made the (pedagogically problematic) decision to mark it with the same keyword as another effect, and distinguish them by the location in which it appears (where the unsafe effect cannot). To copy a rant I wrote on IRC (lightly reformatted):

I'm growing more and more annoyed at Rust's half-assed effect system

With three effects that have two polarities, two keywords, and two declaration locations (in a really horrific jumble) among them

And one of them isn't even properly infectious!

• There's {unsafe, impure, untrusted} as effects,
• under two keywords (unsafe = {unsafe, untrusted}, const = {impure}),
• two polarities (effect = {unsafe}, restriction = {impure, untrusted}),
• and two declaration locations (trait = {untrusted}, fn = {unsafe, impure}).
• Unsafe is maskable,
• impure is not maskable,
• and untrusted is silently and unsoundly masked by default.

It quite literally could not be more inconsistent if it tried

[withoutboats]

This conversation seems to have gone far astray from this RFC proposal; I'd encourage y'all to continue the conversation somewhere like an internals thread.

[burdges]

I'm maybe miss-using "infectious" but.. I'm worried foremost about unsafe fn in traits :

trait Example {
    unsafe fn example();
}
impl Example for SomeExample {
    unsafe fn example() {
        do_something_that_benefits_from_safety();
        unsafe { do_something_unsafe(); }
    }
}

Right now, we cannot do_something_that_benefits_from_safety() here because the fn body is an unsafe block. Yes, we can write the code, but we cannot give it the benefits of safety. This sounds like a mistake. In particular it should cause minor performance hits in places where the compiler uses safety for optimizations.

Afaik, the same mistake exists for non-trait unsafe fn as soo as you accept that unsafe fn has a meaning anything besides "the body is an unsafe block".

As to TrustedLen, I'd think there are always going to be places where safety of unsafe code depends upon the correctness of safe code, but it sound unreasonable to mark particularly dangerous examples using unsafe fn or unsafe traits.

I once suggested another scheme would be "trait tests" in which #[test] fns can be placed in more places like traits and inherent impls to apply tests to trait impls. I think TrustedLen cannot be handled with tests, but arguably some stronger design-by-contract features that stuck debug_asserts everywhere manages. In fact, I think this argument's dubiousness suffices to make the point that TrustedLen should be an unsafe trait.

[Lokathor]

Yes, we can write the code, but we cannot give it the benefits of safety.

Of course it benefits from safety? Unsafe doesn't travel inward, only outward.

I'd think there are always going to be places where safety of unsafe code depends upon the correctness of safe code

This is actually one of the first things the Rustonomicon tells you that you must never do.

[mark-i-m]

@withoutboats Thanks for the moderator note 👍

@eternaleye @withoutboats I created the following internals thread to continue discussion: https://internals.rust-lang.org/t/what-does-unsafe-mean/6696

[nox]

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?

Servo is full of such code. For example I'm currently working on a mechanism where a lot of types must never be encountered on the stack. These values are produced unsafely through an unsafe trait with an unsafe method, and other things using that trait have to follow various invariants and handle the value produced unsafely with care. This trait has no default implementation of this method.

[mark-i-m]

Yeah, we discussed a lot more on the thread I linked above. I think I have better understanding now 👍

[scottmcm]

Hopefully we can work with the "const" effect in addition to the "safe" one. For example, it would be nice to make impl From<u16> for u64 be a const fn. (Which gets half way to obviating rust-lang/rust#49100.)

[eternaleye]

@scottmcm As noted in an earlier message, RFC 2237 is for that specific issue (the second element of the RFC, to be precise):

2. over-constraining trait fns as const fn in impls. This means that
   you may write const fn foo(x: usize) -> usize {..} in the impl even tho the
   trait only required fn foo(x: usize) -> usize {..}.

[nikomatsakis]

I think it may be worth considering a middle ground to start, where impls are allowed to drop unsafe, but their callers and users aren't allowed to rely on that. This would fit more closely with how rustc works today, though I think we could support having callers observe the level of unsafety that you have declared as well.

In any case, I think that if they were to do so, then any specializations would also be required to be safe. That is, you must narrow the things you specialize. That seems readily implementable.

[Centril]

I also think we need to find a better phrasing for documentation purposes than "over-constraining" which I frankly did a horrible number on.

[eternaleye]

Oh, a bikeshed! 😛

• Frugal (it spends only the allowances it needs)
• Humble (it doesn't claim power beyond what it needs)
• Parsimonious (it doesn't make unnecessary assumptions, i.e. incur proof obligations)

Personally, I think "frugal impls" works best, including for other markers - unsafe-frugal impls, const-frugal impls, etc.

[sfackler]

What is the motivation for this feature at this point? Self-referential futures are now dealt with via Pin right?

[Centril]

@eternaleye I raise you one "economic impls" 😆

@sfackler I defer to @cramertj who now champions this RFC and has some use cases (I also think @joshtriplett had some...?).

[joshtriplett]

I've run into a few cases where I wanted to implement a trait and didn't need the qualifiers on the trait in my implementation, so this seems reasonable to me.

[petrochenkov]

@nikomatsakis

I think it may be worth considering a middle ground to start, where impls are allowed to drop unsafe, but their callers and users aren't allowed to rely on that.

This would be more like a subset of #2063 then.
Once a method in an impl has its name and definition block written, everything else is not strictly necessary and can be omitted, including unsafe qualifiers.

[cramertj]

@petrochenkov I think @nikomatsakis was suggesting that in that case the implementer would not be able to do unsafe operations without using an unsafe block, not that the function would be "inferred unsafe."