Sealed traits

Combining a few thoughts:

  • pub(use in path, impl in path) syntax as suggested by @jdahlstrom
    • opens the door to readonly fields — pub(use in path, mut in path) could work
  • #[non_exhaustive] on trait definition to indicate that more items can be added in the future
    • implies pub(impl in crate)
    • no impact on coherence
    • not sure if this is actually necessary given pub(impl in path)?
  • #[sealed] as a future possibility to impact coherence
    • notably does not have to be decided right now
6 Likes

Maybe I'm missing something, but it seems to me that in your example, unsealing FnPtr wouldn't be a breaking change. As long as FnPtr remains sealed, you can add new blanket impls over it; once it is unsealed, you can't add new blanket impls, but the existing ones will still work fine. So I would argue that your type 4 is really a type 1.

I don't know that it's particularly bad, but #[non_exhaustive] has set the precedent that attributes are the way to do these kinds of thigns, so I think it makes sense to follow that unless that's a strong reason to do something else.

Maybe if it were something that made sense on items like statics too then it'd make more sense as a (contextual) keyword.

What about re-using trait aliases?

Currently, this code compiles:

#![feature(trait_alias)]

mod private {
    pub trait Sealed {} // Users in other crates cannot name this trait.
    impl Sealed for u8 {}
}

pub trait Actual = private::Sealed;

Could we simply give it the meaning of "sealed trait" as defined in the RFC?

  • +solves the problem of where to put documentation for Actual
  • +no need for a second trait
  • +no new syntax
  • +compatible with "unsealing" (just pub use the inner trait)
  • +no change to coherence is necessary, since trait aliases cannot be implemented anyway
  • ~does not affect coherence
  • -unobvious?
  • -trait_alias is unstable for a long time already and every change is likely to further bog it down
1 Like

An alias doesn't define a new trait, so there would be nothing preventing another crate from doing impl Actual for Foo {}.

No, currently you can't implement a trait alias.

error[E0404]: expected trait, found trait alias `Actual`
  --> src/lib.rs:10:6
   |
10 | impl Actual for u16 {}
   |      ^^^^^^ not a trait

[playground]

This restriction is because a trait alias can be something like Sealed + Send, which cannot be implemented directly.

The fact this is the case makes it feel to me that trait_alias is a decent way to handle the simple case... except that this makes it so that trait methods and implementors are unavailable in the public documentation.

1 Like

Granted. Regardless, it feels more convoluted/magical than the status quo to me, so I'd rather not encourage that as the new idiom.

Just for clarity, this is an actual idea to put forward. I don't think #[non_exhaustive] would be necessary, as pub(impl in path) on the trait definition would already prevent downsteam implementations. How do others feel about this?

As an alternative to #[sealed]? Might take some getting used to, but...surprisingly appealing, especially if it'd work to say pub(impl in crate) vs pub(impl in self).

pub(impl in path) on trait declatations would be analogous to a hypothetical pub(construct in path) on struct/enum declarations, in the same way that #[non_exhaustive] on traits would be analogous to the existing #[non_exhaustive] on structs an enums. I would feel that we should do things the same way for both traits and structs/enums, whether that means #[non_exhaustive] only, #[non_exhaustive] and extended pub coexisting, or extended pub only and deprecating #[non_exhaustive] everywhere.

Hmm, I don't know that putting this in pub is necessarily the right way to go, because that seems like it might end up stuffing lots in there, and makes the vis macro matcher rather more complicated. I'm not sure that pub(impl in super, crate) is something I'd want to see there -- especially if it started extending to other things in pub too, like the scopes from which fields are allowed to be mutated, say.

I think I prefer the separate construct, even if that other construct also takes a path.

I'm kinda liking the analogue of pub(mut in self) for const fields; feels nicely orthogonal.

But it could also be pub mut(in self) x: i32 and still be orthogonal. (Or, of course, an attribute.) And if it needs to support pub(in super::super, mut in super) because we need to support both kinds of things -- or more things! -- then the construct just starts to feel cluttered to me.

When pub(mut in self) trait Foo { â€Ļ } doesn't make sense and struct Bar { pub(impl in self) a: i32 } doesn't make sense, it doesn't feel like they should both be in the same construct.

Not to mention that I'd rather be able to search "rust sealed trait" and get something useful. If I look for "rust pub impl" I'd probably get a bunch of things talking about "no, there's no privacy on impls", not what I want.

4 Likes

An alternative to #[sealed] as proposed in the original post, yes.

Granted. The syntax is bikesheddable, but something along the lines of this? I view the value of more fine-grained control and future ability for read-only fields quite highly.

1 Like

I agree, it sounds like a nice features to have, since seeing the field -- even if non-mutably -- can help with borrow splitting and such, unlike a method for it.

2 Likes

I would really, really like to have method visibility implemented together with this. The main reason is that things are normally private by default. There's no priv keyword, just pub. If all methods are public then adding visibility support later would require some special notation, making it confusing. As an alternative if all methods are private by default people can add a second non-sealed trait but that brings us back to the current situation.

Further, it's not possible to have truly private methods and bounds:

pub trait Actual: sealed::Sealed {}

mod sealed {
    pub trait Sealed: Copy {
        fn foo(&self);
    }
}

// impl Actual, Sealed

in consumer crate:

fn do_foo<T: Actual>(val: T) {
    // consumer crate can assume `Copy`
    let val2 = val;
    // consumer crate can call "private" methods
    val.foo()
}

In many cases it's useful to have private implementation details as methods on the trait or even private trait bounds.

I propose that methods and trait bound of sealed traits are invisible in dependencies except were marked pub

pub(impl in self) trait Sealed: pub Copy + Debug, /*not visible*/ Self: Display {
    // can be called from consumer crate
    pub fn foo(&self);
    // can *not* be caleld from consumer crate
    fn bar(&self);
}

That is a fundamentally different problem, and one that I do not intend to solve alongside sealed traits. At some point I do intend to write an RFC for proper visibility on trait items. But there is zero reason to do them together.

And by the way, we do have a priv keyword.

That said, if sealed traits are e.g. pub impl(in crate) rather than #[sealed] pub, having member visibility limited to the impl visibility by default should at least be strongly considered. I'm fine with #[sealed] just controlling the ability to impl outside the coherence boundary, but anything getting more first-class syntax has a stronger implication.

It is also an interesting observation that just pub impl(in crate) is (nearly[1]) sufficient to express member visibility as well. In fact, the most natural way to express impl visibility is via member visibility: if you can't name a required member, you can't implement the trait. Voldemorting a supertrait is currently the most common way to seal a trait, but it's also possible[2] and not uncommon to seal a trait via a #[doc(hidden)] required method taking a pub-in-priv Voldemort type as an argument.

This also just imho matches Rust's priv-by-default stance better. Members are public to the scope that can implement the trait by necessity, since you can't implement the trait without visibility of the members (as above). If this necessity no longer exists, the members should take on the default state of private to the implementing scope.

I agree in restricting the scope of #[sealed] to standardizing : Sealed behavior and providing better compiler error support by understanding the pattern more directly. But actual "impl visibility" syntax is a different beast with different implications.


  1. Fully sufficient if you're willing to introduce extra traits to capture the visibility splits; it's possible to construct use cases which want the higher control given by per-member visibility. ↩ī¸Ž

  2. This is less common and I'd argue inadvisable, since it's possible that a way to implement a function without naming the type could be introduced in the future, either by accepting trait method argument type as a defining use position for TAIT, or some other form of allowing type inference of "use what it says in the trait." This also fits well imho with overconstraining impls; there's no inherent reason why I shouldn't be able to implement a trait function more generally by e.g. taking impl Sized, other than this allowing me to define a method being used to seal the trait via Voldemort argument. ↩ī¸Ž

1 Like

As in, have the impl restriction imply non-public trait items? That seems quite subtle. I want non-public trait items too, but to me the most reasonable way forward is to do it in an edition (changing the default to private).

This is the same situation that led to #[non_exhaustive] — people don't like doing that. It requires an additional method for no purpose other than restricting the ability to implement the trait. It also makes it easy to accidentally unseal a trait if you make the last private method public without adding another private method.

My observation here is that impl-visibility requires member-visibility. So not quite

but rather that trait members are always non-public, but that providing impl access also implies providing pub access to the members.

I.e. like not writing pub on items defaults to pub(in mod), not writing impl defaults to impl(in extern), and not writing pub on members defaults to pub(in impl) (and that doesn't mean protected, that means public to modules with impl visibility).

The overwhelming majority case for traits is to have the members visible to the world for implementing, and not just because that's the only currently supported option. #[doc(hidden)] is quite functional as a for-internal-use-only marker. std has stability. Everyone has access to pub-in-priv sealing. And yet most traits do want the pub-in-impl-scope behavior.

It becomes less problematically subtle if you reframe from a restriction to a visibility scope. It's just that pub trait means pub(in extern, impl in extern) because that's by far the most common configuration.

Thus why I'm not saying to implement sealing that way, just that an unnamable required member is the simplest way to model a trait being sealed.

(Actually, can #[sealed] be spelled #[non_exhaustive]?)

2 Likes