Pre-RFC: Sealed traits

1

This is an issue that I’ve been wrestling with for a little while, and I think there’s a core way of expressing our APIs that Rust is currently missing. I need to write this up a little bit more in the “RFC” language, and make the examples a bit more abstract, but I wanted to get other people’s opinions on the concept before doing so. (I’m actually unsure if having the motivation be a specific concrete motivation from a real crate is desirable or not here)

Proposal: Sealed traits

This RFC proposes adding the #[sealed] annotation to the language. When a trait is marked as #[sealed], it becomes illegal to add an impl for the trait outside of the crate where it was defined.

Motivation

Let’s look at a concrete example from diesel. In 0.4, we have a trait hierarchy that can be simplified to this:

trait FromSql<T> {}

trait FromSqlRow<T> {}

impl<T, ST> FromSqlRow<ST> for T where
    T: FromSql<ST>,
{}

impl<A, B, TA, TB> FromSqlRow<(TA, TB)> for (A, B) where
    A: FromSql<TA>,
    B: FromSql<TB>,
{}

Similar to Into and From, implementing FromSql implies an automatic FromSqlRow impl. FromSqlRow is additionally implemented for tuples. However, when we tried to make FromSql be abstract over a database backend, this structure becomes illegal.

trait FromSql<T, DB> {}

trait FromSqlRow<T, DB> {}

impl<T, ST, DB> FromSqlRow<ST, DB> for T where
    T: FromSql<ST, DB>,
{}

impl<A, B, TA, TB, DB> FromSqlRow<(TA, TB), DB> for (A, B) where
    A: FromSql<TA, DB>,
    B: FromSql<TB, DB>,
{}

The reason this is illegal is that the following is now possible in a child crate:

impl FromSqlRow<(i32, i32), MyLocalType> for (i32, i32)

Without the addition of the DB parameter, there is no way for a child crate to impl FromSqlRow in a way that overlaps with the blanked impls that exist, as there would be no crate local type, and tuples are not #[fundamental].

However, the intention of this API is that FromSqlRow is not something that can be implemented directly, and types wanting to implement it should impl FromSql. Under this proposal, FromSqlRow can be marked as #[sealed], making this contract explicit to the compiler, and allowing blanket implementations that would currently be illegal under the coherence rules.

An additional benefit of this change is that a crate is freely able to add members to any #[sealed] trait without it being a breaking change, as they can be confident that they control all implementations of that trait.

Drawbacks

By definition, this change means that crates will potentially have traits as part of their public API which cannot be directly implemented. This could be potentially confusing or frustrating for users of that crate. This is easily mitigated by providing a proper error message in this case, which shows all derived implementations of that trait that the user can take advantage of. In the given example, the error message would ideally specifically mention implementing ToSql<ST, DB>.

Alternatives

Many of the “impossible” cases that this would solve will be made possible by specialization. However, I do not believe that specialization covers all of these cases. Additionally, it does not remove the desire to have a “public but not implementable” trait in a crate’s API, and the ability to freely modify a trait as a non-breaking change is a major concrete benefit that is not covered by specialization.

Thank you for taking the time to review this proposal.

12 Likes
2

I don’t have a strong opinion about #[sealed] itself, but I’m very interested in the stated motivation. #[sealed] is essentially a more nuanced kind of privacy for traits (you can see it but you can’t implement it). My intuition is that privacy toggles are not the correct solution to coherence issues, but the motivation is a coherence issue.

In particular, it sounds like the reason #[sealed] would solve your problem here is the implicit negative reasoning in local contexts codified by RFC 1023, but the inferred mutual exclusion allowed in local contexts is probably a mistake.

The first example works because there is an inferred (A, B): !FromSql<(A, B)>. My understanding is that the second does not work because it will not allow the inference that (A, B): !FromSql<T(A, B), DB> because of the additional parameter. (Note: if my understanding is correct, there is a mistake in the RFC that the child crate is said to be able to implement FromSqlRow, not FromSql. Let me know if instead I am mistaken.) But if you could just explicitly impl<A, B, DB> !FromSql<(A, B), DB> for (A, B), the problem would be solved, no?

I think that’s what you actually want here. The motivation for sealing traits has to do with finer grained privacy and performance optimizations in trait objects, not with coherence.

FWIW, my experience working through this example is exactly why I said that 1023 inferred mutual exclusion is a misfeature. Working out what exclusions it will allow and what it won’t is mindbogglingly hard.

7 Likes
3

Instead of having users deal directly with values of &T where T is your trait, have them deal with &S where `S is:

pub struct S {
    t: &T
}

Then make trait T non-public. Then only your crate will be able to implement T internally and the users of your crate get basically the same interface.

See https://github.com/briansmith/ring/commit/95e730286e3cdc5005f1593fbad9842812cbaf63 for a real-world example of this.

4

That would only allow users to get trait objects, which is not enough for many cases. You cannot bound by it, you cannot get the behavior on primitive or std types, etc.

6

I should note, that sealed traits are also a good solution to the problem of private trait bounds in public interfaces.

Consider this public function:

pub fn function<T: PrivateBound>(arg: T) { /* do something */ }

Such functions are prohibited currently, because PrivateBound is a “private trait in a public interface”. However, desire to write such functions was expressed by various people (here and here).
Private bounds can be used to emulate function overloading: instead of

pub fn function(arg: f32) {}
pub fn function(arg: f64) {}
pub fn function(arg: my_f16) {}

the author provides one generic function

pub fn function<T: MyFloat>(arg: T) {}

where MyFloat denotes the closed set of types {f32, f64, my_f16} and the author doesn’t want users to extend this set.

However, look at this function from user’s perspective - function is a function that can accept what, MyFloat? What is MyFloat? Something private and undocumented. Do I have to read the source code to understand what arguments can function accept? Should the author write some informal documentation next to function about what MyFloat represent?

This situation is suboptimal. The trait MyFloat should be public and documented. The user should be able to write wrapper functions around function and use MyFloat as a bound. I’m not sure if the user should be able to use MyFloat's methods. Finally, the user shouldn’t be able to implement MyFloat because the function's author wants the overload set to be closed and controlled by him. The whole setup reminds sealed traits a lot.

10 Likes
7

(In reply to petrochenkov) This situation certainly exists, in my experience the methods need not be callable outside the crate.

ndarray has some public traits in its crate that are there to make the crate work, not to make it extensible. All these have to be public and documented, but never implemented: Data, DataMut, DataOwned, DataShared, DataClone, Dimension, RemoveAxis, Scalar, NdIndex.

8

Another note about this situation: you can make an unimplementable trait by bounding an associated type with a private trait, and AFAIK it’s never been clarified whether this is a bug in the privacy checker or not. (Previously, private supertraits were also allowed, but that was classified as a bug and killed.) If it’s not a bug, it could be enshrined by (a) documenting it and (b) teaching the coherence checker about it so it could be used to make sealed traits.

2 Likes
9

@durka
Private bound on an associated type of a public trait is E0445.
(It’s reported after type checking, so you have to comment out impl inner::PrivateAssoc for S to see E0445 reported.)

10

Ah I see the full example is now a warning E0446 on nightly. So both the “backdoor” ways to do sealed traits are now gone, sucks.

1 Like
11

So I think sealed traits probably make sense, but perhaps not quite for the reasons you suggest in this motivation. Some reasons that I have wanted sealed traits are as follows:

  1. I want to define a closed set of types that is not open to user extension.
    • For example, imagine that I am dealing in unsafe code, and I want to write a function that accepts either A, B, or C. I could write fn foo<T>(x: T) where T: MySealedTrait and just implement MySealedTrait for A, B, and C, but of course then the user can implement it for their own types. If MySealedTrait is just a marker trait, this may not make sense.
    • That said, we have options today:
      • declare the trait as unsafe, for example
      • use some privacy trick to make the trait unnameable by users
    • Still, I feel like this comes up as something I want semi-regularly. Whether it's something I need feels a little less clear.
    • (Reading a bit further, this is basically what @petrochenkov was talking about.)
  • Publish a trait whose design is still being changed without fear that users will try to implement it.
    • If a trait is sealed, you can make arbitrary changes to its members without violating semver, so long as you keep (or expand) the same set of impls.
    • But maybe it's enough to just comment the item as unstable and exempt from semver rules? Unclear -- it makes us very nervous for the lang itself, I suspect popular crates would have the same problems of de facto stability.

One open question: should sealed affect how auto traits like Send apply to trait objects? In principle, we can now find all implementing types, after all. I can see arguments on both sides: for example, that makes removing Sealed potentially a breaking change! (But only if other crates are relying on this property when using a trait object.)

I think this is not quite correct. I think the concern is that somebody could implement the trait FromSql, not FromSqlRow (this may just have been a typo on your part). If they did so, they would create an indirect dependency. This seems like Coherence and blanket impls interact in a suboptimal fashion with generic impls · Issue #19032 · rust-lang/rust · GitHub -- which is definitely a frustrating problem.

Therefore, I think @withoutboats is correct that explicit negative impls would help here. @aturon and I have also been discussing the possibility that specialization could indirectly come to the rescue: that is, it might be that we could allow some overlap, on the basis that if the user did add an impl, it wouldn't matter, because the impl would have lower precedence than the existing one. So, in some user did add a FromSql impl, then yes, that would trigger overlap, but it's still the case that your second impl is more specific than the first one, and hence it would still be considered the winner.

To be honest I haven't made up my mind on this issue and am still mulling it over. It seems to me that having the ability to make explicit negative declarations can be very clear and explicit. However, it's also another level of complexity in an already complex system. (Interestingly, sealed is an example of a negative impl that you would not be able to say in any explicit proposal I've seen.)

Ah yes, this is basically what I was talking about above.

This seems like a bit of a distinct issue. In particular, I had assumed that sealed controlled where a trait could be implemented, but not where its members could be accessed. To control access, it seems like the obvious solution is to permit trait items to have public/private declarations. I've been thinking that https://github.com/rust-lang/rfcs/pull/1422 provides a potential solution there: we could allow (optional) pub declarations on trait items, which would indicate where they can be used (but not where they can be implemented). So, for example, pub(crate) could be used to restrict a trait item to the current crate, and would be a natural complement to sealed.

Well, it's always possible to declare a public trait -- and thus one hypothetically exposed to the world -- but "seal it off" through an inaccessible path http://is.gd/yqIPc5, no? (I didn't look at your example in great detail, perhaps there is some other wrinkle I'm missing)

1 Like
12

@nikomatsakis [quote="nikomatsakis, post:11, topic:3108"] I've been thinking that https://github.com/rust-lang/rfcs/pull/1422 provides a potential solution there: we could allow (optional) pub declarations on trait items [/quote] +++

There seems to be several capabilities related to traits and their methods:

  • Ability to use the trait or its methods in "read-only" way. This can be controlled by pub(path) on trait itself or its methods.
  • Ability to implement the trait or its methods. This can be controlled by sealed(path) on trait itself or its methods. If a method of non-sealed trait is sealed, the trait still can be implemented if this method has default implementation.
  • Ability to specialize the trait or its methods. This is described in https://github.com/rust-lang/rfcs/pull/1210 and controlled by default / open / whatever the bikeshedding will result in.

It would be nice for all these aspects of privacy to be supported and controlled separately.

13

As I commented on the “no private items in public interfaces” RFC at the time, by analogy with structs, the way to express sealed traits that feels “right” to me would be to allow traits to have private associated items (whether we re-introduce the priv keyword for this, or whatever). A trait can be thought of as just a struct type containing fns and so on with extra compiler magic to pass it around and access its members implicitly, with impl fors being the only way to construct instances of it, so just like a private field in a struct means only the owning module can construct one, a private method or other associated item in a trait would mean only the owning module can impl it.

1 Like
14

(By the way, this is a lousy way to "seal" a trait, since it means that nobody can wrap your generic function with a generic function of their own, since they can't name the traits in your where clauses.)

15

Yes, that was a typo. Apologies.

16

Heh, didn't know about that trick. And TBH I would have expected a "private type in public interface" error when exporting f(). Like I said it's really hard to know whether these holes in the checker are intentional or not.

17

The rule is actually very simple: are you write a signature of something declared as public? If so, then the things it refers to must be declared as public. (It doesn’t matter if they are “accessible” in any grander sense, it’s all about how they are declared.)

I don’t see this as a “hole” in privacy: it’s about what your goals are. The goal here is to ensure that if something is declared as priv, you only need to look within the current module (and its submodules) to know who has access to it. If it is declared as pub, you have to work harder, because you have to care about how things are exported or re-exported. I’d prefer more expressive pub declarations, like in https://github.com/rust-lang/rfcs/pull/1422, which would mean that we can extend “private” out to enclosing modules, and not just enjoy its benefits within the current module.

Anyway, this is afield from this thread…

18

Right, but apparently a trait bound on a parameter doesn’t count as “referring”. Previously you could do a private supertrait, which is just a kind of implicit where-clause, i.e. a trait bound, but that was somehow different… maybe you can see how this is somewhat subtle and confusing to an outsider :slightly_smiling:

Sorry for going off-topic. To come back, it seems (partly evidenced by these workarounds) that there is a need for sealed-trait functionality.

Personally, I feel that having #[sealed] affect traits like Send is “too much magic” or complicates the rules too much. If I understand right, the rule could be "if Trait is sealed and all implementors are Send, then Box<Trait> is the same as Box<Trait + Send>"? But then the Send-ness can be lost even by adding another impl. Maybe this case could be covered by allowing unsafe impl Send for Trait {}.

2 Likes
19

Another potential use case for #[sealed] (in conjunction with specialization) is type traits:

#[sealed]
trait RemoveIndirection {
    type Type: ?Sized;
}

impl<T: ?Sized> RemoveIndirection for T {
    default type Type = T;
}
impl<'a, T: ?Sized> RemoveIndirection for &'a T {
    type Type = <T as RemoveIndirection>::Type;
}
impl<T: ?Sized> RemoveIndirection for *const T {
    type Type = <T as RemoveIndirection>::Type;
}
20

Is there any update to this / was there ever a RFC released? I’ve run into the situation @nikomatsakis described to a T:

I’m writing a FFI safe wrapper and have created a bunch of newtypes over raw pointers. I’ve also created trait(s) that acts as sets, allowing a subset of these newtypes to work for a given function.

The trait(s) need to be public, so that users can supply generated objects to methods, but it would likely be disastrous if they decided it would be a good idea to implement their own objects for the traits.

For my use case, sealing the traits sounds like a good solution.

Related question: would it be permissible to leak a public super trait’s private sub traits if the super trait is #[sealed]? Since users aren’t allowed to implement the trait anyway…

1 Like
21

The standard convention right now is something like this:

pub trait MyThing: Sealed {
    // ...
}

mod private {
    pub trait Sealed {}
}
3 Likes