Sealed traits

language design
21

Personally, I have worked around the lack of sealed traits not (only) because I didn't want other users to implement it, but because I wasn't certain that I wouldn't add non-defaulted methods to the trait in the future. If I later determine that the existing set of methods is sufficient, I see no reason I shouldn't unseal the trait.

As to coherence, this certainly makes more sense now. I think that limiting the effect to within the defining crate makes the most sense, as it eliminates most "magic". How do others feel about placing this restriction on the future possibility of impacting coherence?

3 Likes
22

I think if we do impact coherence at all, we certainly shouldn't do so outside the crate, since doing so would cause the crate to provide an API commitment that it then couldn't break, without having written that anywhere. And that API commitment could be broken not just by removing #[sealed], but by adding an impl of a sealed trait to another type that doesn't implement one of those traits.

I don't think we should impact coherence even inside the crate, but in any case I agree that it suffices to commit to not impacting coherence outside the crate, which thus makes #[sealed] removable without breaking API compatibility of a crate.

1 Like
23

Can you say more about why? Is there anything specific that you're concerned about?

To me, the fact that for a sealed trait we know the exhaustive set of implementations has always implied that it'd make sense for it to add a few exceptions to the orphan rules the same way that #[marker] adds exceptions to the overlap rules.

For example, suppose you have this:

#[sealed] pub trait MyStuff {}
pub struct MyType1;
pub struct MyType2;
impl<T: MyStuff> std::fmt::Debug for T { ... }

Today, that's not allowed:

error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct<T>`)
 --> src/lib.rs:4:6
  |
4 | impl<T: MyStuff> std::fmt::Debug for T {}
  |      ^ type parameter `T` must be used as the type parameter for some local type
  |
  = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local
  = note: only traits defined in the current crate can be implemented for a type parameter

And thus people are forced to macro-implement Debug for all those types instead.

But there's really no fundamental problem here. It needs to be prevented for non-sealed traits, obviously, but once it's sealed, we could "look through" that blanket and say "well yeah, that's fine for every implementation", and allow it.

If you added an impl MyStuff for u32 then you'd get a compilation error, sure, but that's fine. It just means that by adding that blanket you're restricting what you can do in future -- as is already the case for blanket impls in many situations.

One thing that comes to mind here is that that library team has been talking about possibly exposing features/[unstable] to libraries in some form.

I think that would be a better way to express "I'm intending this to be implemented outside my crate in future, but I'm not ready yet".

For example, you could either just make the whole trait unstable, or do something like

pub trait MyTrait {
    fn useful_method_1(&self);
    fn useful_method_2(&mut self);

    #[unstable]
    fn __possibly_more_methods_later();
}

And then implementing it would need to opt-in to the potential future breakage using the stability system instead, but it and all the other methods could be used in the mean time.

4 Likes
24

For the same reason that we don't allow type inference in function parameters, even for non-public functions within a crate: non-local effects, vs keeping inference localized to certain boundaries.

If I write fn func<T: MyStuff>(arg: T), I would like the compiler to prevent me from using anything not guaranteed by MyStuff. If I then use something not guaranteed by MyStuff, I'd like an error, from which I can then add more guarantees (such as either making MyStuff require another trait, or making func require that other trait in addition to MyStuff). As a result, the type signature of func provides clear documentation of what func relies on.

25

They did write it: they wrote #[sealed]. Whether #[sealed] should be an API guarantee is open to debate, but there's no intrinsic reason it cannot be.

Huh? Nobody has proposed adding #[sealed] to structs or enums, it's just an attribute for trait items.

Let me spell out to the best of my ability how I expect #[sealed] to actually impact coherence reasoning:

I'll declare three crates, upstream, middle, and app. app depends on middle, which depends on upstream. I also declare other, which is unrelated.

  • middle defines #[sealed] trait Q, and implements it for some set of types from middle and/or upstream.
  • app uses Q. For the purpose of coherence, all of app's types are considered to have an explicit impl<T in ::app> !Q for T.

And that's it. Any coherence implications fall out of that one fact we tell the coherence engine: if a crate is downstream of middle, its types cannot gain an implementation of Q.

If app adds a dependency on other all of other is unimplicated and still in the "a future version could implement the trait" state.

AIUI nobody is actually asking to have access to duck-typed "one of these types"; abstracting over a sealed trait still only gives the contents of the trait (which may include TypeId and downcasting permission, but that's explicitly provided by Any).

3 Likes
26

I agree with this, but to me it's not coherence, because it's not about what you can impl (ref: Chalk's definition of Coherence).

So to me that's an argument that you don't want trait solving to be sealed-ness aware (the same as it's not marker-ness aware), which seems perfectly reasonable. Especially so for marker traits, because you could just add more supertraits whenever.

2 Likes
27

While #[sealed] ensures there's no downstream T: MyStuff, we would also have to check that all MyStuff are local, that you didn't impl MyStuff for ForeignType, before we could allow that Debug.

28

Sure, we could make #[sealed] guarantee that, but I'm advocating that we shouldn't.

Sloppy terminology, sorry. I've fixed the original post to clarify: " adding an impl of a sealed trait to another type that doesn't implement one of those traits".

One of the posts in this thread proposed that if every type implementing a sealed trait also implemented Debug, that users of that trait could also count on Debug. That's the thing I'm saying we shouldn't do.

2 Likes
29

How does this interact with a potential future where we want to give trait method definitions an optional visibility? Could we add a qualifier to visibilities for trait methods such that a trait definition could allow some methods to be implemented by downstream crates, but not others? (I guess an alternative way to phrase this might be, could the sealed attribute be moved to method definitions instead of a crate itself?)

2 Likes
30

From what I wrote on Zulip a few weeks ago regarding visibility and interaction with #[sealed]:

↓ location / definition → pub #[sealed] pub(crate) private
same module unrestricted unrestricted unrestricted unrestricted
same crate unrestricted unrestricted unrestricted inaccessible
downstream unrestricted call only inaccessible inaccessible

What it sounds like is you'd like a call-only method in an otherwise unsealed trait? I could see the #[sealed] attribute being placed on a method in an unsealed trait to prevent downstream implementations. The method would obviously have to have a default implementation.

2 Likes
31

Ah, this isn't what I meant. See Sealed traits - #23 by scottmcm instead -- I was picturing something that would still need an impl Debug for …, not something inferred by the traits that are implemented by the types.

(I don't see a need for "well everything implementing MySealedTrait is Debug, so we can assume it" as being useful, because for a sealed trait it's easy to add Debug as a supertrait in that scenario.)

7 Likes
32

One possibility that hasn't been explored is to have two attributes, #[sealed] and #[non_exhaustive]. #[sealed] on a trait forbids impls outside of the defining crate and makes it a semver-breaking change to add new impls within the defining crate, or to remove the attribute. #[non_exhaustive] only forbids impls outside the defining crate (analogous to to how #[non_exhaustive] on a struct or enum forbids construction outside the defining crate).

#[non_exhaustive] would mean "the list of requirements for implementing this trait isn't exhaustive and might change in the future, new required methods or trait bounds may be introduced". #[sealed] would mean "the list of implementations of this trait has been sealed forever and will not change, the trait solver can make full use of that information".

4 Likes
33

This is stronger than what I'd been thinking of for it. Suppose we had a PrimitiveInteger sealed trait, we wouldn't have been able to add u128 to it if the rules was "set of implementations cannot change".

(That's more like #[fundamental], which we've been hoping to remove.)

4 Likes
34

While there is some value in automating the sealed trait pattern, IMHO that value is quite low, since the pattern is easy to implement and relatively well-known. If #[sealed] is just an attribute without any extra semantics (like modified coherence rules), then it should just be added to the libcore as a proc macro. In fact, there is already the sealed crate which does that.

If #[sealed] is added as a language feature, I would expect it to have extra powers which cannot be (easily) emulated in a library.

It's also a bit jarring that #[sealed] essentially duplicates the privacy mechanics. With traits, there is potentially visibility for implementations and visibility for usages as separate concepts, which are (mildly unfortunately) conflated in the current visibility of traits. Perhaps there is a cleaner way to separate these concepts which would not require a separate attribute, and maybe even would support privacy of implementation for individual trait items?

4 Likes
35

Hmm. It seems we have 3 kinds of sealed traits:

  1. I want to be able to add new items to my trait in the future. This means restricting external impls. However, I also want the freedom to unseal my trait and remove this restriction in the future. The RFC as written caters to this kind of sealed trait.

  2. I want to be able to write code within my crate that relies on knowing all the implementors of my trait. But I might also want to add new impls of my trait within my crate in the future, so I don't want downstream crates to be able to rely on the current list of impls. I'm fine with not being able to unseal my trait in the future. PrimitiveInteger would be this kind of sealed trait.

  3. The list of implementors of my trait is set in stone. I will never change it, and I want downstream crates to be able to rely on it.

Sealed trait types 1 and 2 can be unified; a type 1 sealed trait is just a type 2 sealed trait but where no code that relies on knowing the list of implementors has yet been written in the defining crate. So potentially, sealed traits types 1 and 2 could be represented with the same syntax. However, there's a danger that it could be too easy to accidentally turn a type 1 sealed trait into a type 2 sealed trait. This footgun could potentially be addressed with new syntax. And we don't have to figure that syntax out immediately; we could land type 1 sealed traits now and then figure out how to tack on type 2 later.

Type 3 sealed traits are a different beast entirely, they can't be unified with the other types. Syntax used up by type 1 and 2 sealed traits can't be repurposed for type 3. So when implementing type 1/2, we must either avoid reserving syntax that would have made more sense for type 3, or decide that type 3 isn't worth it.

Personally, I feel that #[non_exhaustive] makes sense for type 1/2 and #[sealed] for type 3, but the color of that bike shed is not important at this point.

36

Have you read the RFC? These points are addressed.

37

My motivation for wanting type 3 sealed traits is code like:

// crate `upstream`

#[sealed]
pub trait Sealed {}

impl Sealed for u32 {}
impl Sealed for i32 {}

// crate `downstream`

use upstream::Sealed;

trait Frobnify {
    fn frobnify(&self);
}

impl Frobnify for u32 {
    fn frobnify(&self) {
        println!("whiz {self}")
    }
}

impl Frobnify for i32 {
    fn frobnify(&self) {
        println!("bang {self}")
    }
}

pub fn do_stuff(n: impl Sealed) {
    n.frobnify()
}
39

I don't think type 3 wants to be a trait, especially if it wants the duck typed behavior you demonstrate. Instead, it wants some sort of static type set, where you accept any member of the type set and can use any functionality provided by all types in the set. Basically, a shortcut for monomorphizing over a known set of types. Naturally, this would be incompatible with dynamic dispatch. I'd suggest maybe static trait Q { u32, i32 } as a working name for the feature idea (or maybe enum trait is better?), but while related to sealed traits, due to the implications on type checking / method lookup, it's a much different feature.

But I have a fourth kind/usage of sealed traits: type three, but adding new implementations for non-downstream types is fine. Basically, going back to my assertion that the coherence implications is just impl<T in ::downstream> !Q for T.

An example use case for this would be FnPtr in std. This would allow std to have

#[sealed(for coherence)]
#[unstable = "implementation detail"]
trait FnPtr {
    fn as_addr(this: Self) -> usize;
}

impl_fn_ptr! {
    extern
        "Rust", "C", "system",
        "C-unwind", "system-unwind",
        // etc
    fn(A, B, C, D, E, F, G, H, I, J, K, L) -> Ret
}

impl<F: FnPtr> Debug for F { … }
impl<F: FnPtr> fmt::Function for F { … }
impl<F: FnPtr> PartialEq for F { … }
impl<F: FnPtr> Eq for F { … }
// etc

and macro copy/paste a single sealed trait implementation over the function pointer types which is used to blanket implement the traits available for function pointers. Without the coherence guarantee that downstream types won't be covered by this blanket implementation, they cannot implement any of the blanket implemented traits. However, adding new implementers which are not downstream of std is perfectly fine.

I'd argue that treating type 2 as "type 4" is also fine... but what type 2 actually wants imho is unsafe trait, where the unsafe requirement for implementing is don't.

I'll also argue that the case of "#[non_exhaustive] trait is similar to structs in that it could be served by a private item, but having a way to state the intent directly is also a good idea. (#[non_exhaustive] is an interesting way to spell it, since the first thing I think of is the enum meaning and not the struct one, and trait already is an open-world set compares to enum's closed-world set.)

5 Likes
40

May I ask why a keyword is a bad choice? I'm asking because currently all privacy control is done through keywords too, if I understand it right. And sealing seems to be a matter of privacy, or isn't it? (Just asking, I don't really overlook it…)

Hmmm… :thinking: on the other hand #[sealed] could be seen as a Type System Attribute. So maybe that's a good reason why to make it an attribute instead of a keyword.

2 Likes
41

A possible alternative for consideration: expanding the pub(xyz) mini-language for traits to allow separate privacy control for impl and usage (as mentioned somewhere in this thread):

// can use anywhere, can impl only in this module and submodules
// ie. sealed; private for impl purposes
pub(use) trait Foo {} 
// can use anywhere, can impl only in path
pub(use, impl in path) trait Foo {}
// same as pub trait Foo {}
pub(use, impl) trait Foo {} 
// same as pub(in path) trait Foo {}
pub(use in path, impl in path) trait Foo {}
4 Likes