[Pre-RFC] dyn trait declarations

Update: I have submitted this as an RFC!


Hello everyone, i recently drafted an RFC for an addition to trait declaration syntax which would allow users to explicitly mark that a trait must always be trait object safe.

You can find a completed draft version of the RFC here

The TLDR of the proposal is that a very common design pattern observed is using a trait as a trait object, then having a private function to make sure it is always object-safe, this is used in libcore for iterators for example:

fn _assert_is_object_safe(_: &dyn MyTrait) {}

This works, but it is a bit ugly and kind of a hack, moreover, if the trait is not object safe and you try to use it as a trait object, you get errors with multiple labels. This can be confusing because the ranges are not in the impl block, they are just on the usage. To solve both of these issues, this RFC proposes a simple addition to trait declaration syntax:

dyn trait MyTrait { /* */ }

marking a trait declaration as dyn would enforce at compile time that the trait must be object safe, and if it is not, then errors with accurate ranges are thrown. This is more explicit and easier to see if a trait is explicitly meant to be used as a trait object.

Users would be encouraged to use this syntax through help messages in the current trait object unsafety error, as well as lints which will be upgraded to errors in edition 2024.

I'd love to hear any feedback or complaints about the proposal! :slightly_smiling_face:

11 Likes

There's a much more readable—imho—alternative that's incidentally close to the proposed syntax:

trait Example {
//..
}

/// Asserts that `dyn Example` is a valid type, i.e. Example is object-safe
impl dyn Example {}

Since it has no semantic impact on the program this might also be freely added by a proc-macro without any larger risks.

2 Likes

So with your suggestion, if your crate defines a trait but never implements it you wouldn't be able to assert that it's object safe? It's an unlikely scenario but it seems like the two suggestions have slightly different semantics.

This does not solve the issue of ranges being inaccurate and confusing however. also, it still feels like a hack/workaround to me.

I also like this idea for another reason: A reader of the code knows immediately if the trait is object safe. Otherwise, they'd have to create a trait object and see if it fails to find out.

However, I think the syntax is a bit confusing. My suggestion is to use an #[object_safe] attribute instead; this expresses clearly what the feature does. It also doesn't require changing the syntax, which is already too complicated in many people's eyes.

1 Like

I didn't want to weigh in on it too much just show the alternative with less semantic impact. Personally, it seems clear enough but the impact of the change to assert dyn-'ness also appears small enough.

I'm not sure what you mean and you might have misunderstood the semantics of the example. Evidence that that it is rather confusing even though functional. The impl-block impl dyn Example {} is an entirely free standing impl block on the dynamic trait object type—defining no methods—and not something you add to a trait impl definition. You can add it independent of any actual impl of the trait.

The opposite on the other hand would be more interesting to assert for me. Consider that object-safety of a trait has SemVer implications that are implicit on the traitÂą. Being able to mark a trait such that its dynamic version does not automatically exists might be useful as it allows adding generic methods with defaulted implementations later on without a breaking change. In that case there is no known any alternative or workaround afaik. The best approximation is a : Sized bound but that has semantic impact as it forbids unsized implementors.

trait ?dyn Quacker {
    fn quack(&self);
    // Adding this later shouldn't be breaking:
    // fn concert<I: Iterator<Item=QuakStyle>>(&self, _: I);

    // Another interesting evolution that currently is not available
    // const DECIBEL: usize = 100;
}

// Should be a compile error.
fn foo(_: &dyn Foobar) {}

ÂąSee the API evolution guidelines

There are [..] circumstances when adding a defaulted item is still a major change:

  • The new item would change the trait from object safe to non-object safe.
2 Likes

How about something similar to public-in-private Sealed marker traits:


#[doc(hidden)]
mod private {
    pub trait NotObjectSafe {
        fn not_object_safe() {}
    }
    impl<T: ?Sized> NotObjectSafe for T {}
}

pub trait MyTrait: private::NotObjectSafe {
    fn method(&self);
}

// fails:
// impl dyn MyTrait {}

Edit: Note that the privacy of the trait is not necessary; the main goal is just to avoid someone importing the NotObjectSafe trait because having this not_object_safe() method available would be a bit weird. But unlike the Sealed pattern, in this case it isn’t necessary for ensuring the guarantee that the trait stops being object safe. Thus the suggestion of @mcy below of including a trait like this in the standard library makes a lot of sense. Furthermore, a standard library implementation could still archieve the clean method-free interface by having a public-in-private supertrait itself.

Edit2: One problem with having a non-private NotObjectSafe trait though is that people could, technically, rely on the fact that NotObjectSafe is a supertrait of your trait; which would mean that modifying the trait (by addition of default-implemented non-object safe new methods/functions) could be somewhat "breaking" in case you’d also like to remove the then-obsolete NotObjectSafe supertrait "marker".

Honestly, such a thing could be completely reasonable to include in the standard library.

1 Like

Actually this was already proposed in https://github.com/rust-lang/rust/issues/57893#issuecomment-546972824.

The purpose and the scope was different though: the idea was that only traits declared with dyn would be usable as trait objects. In other words, we would disable trait objects by default, and require marking dyn on individual traits instead.

It was thought to be a path towards a fix for an important soundness hole, however it also implies your proposal.

Thinking about it, it’s possible that implementing your proposal first and encouraging users to use it would ease the path to the broader proposal described above.

6 Likes

Very interesting, i didnt see that comment, yeah, i believe that forcing users to use dyn trait for trait objects is a bit too harsh. It should be more of a gradual change encouraged by lints and suggestions.

Edit: and this would achieve what the comment proposes by upgrading to an error lint level in edition 2024

I complained about this specific quirk of Rust not so long ago. IMO, get rid off ?Sized as the default for traits in a future edition and make explicitly adding ?Sized enforce object safety, since removing object safety is a breaking change. This wouldn't even require new syntax.

EDIT: For less error-prone edition migration, it could be enforced, that either Sized or ?Sized must be supplied to the trait and not doing so would result in a compile error. This strictness could be removed in a future version and Sized would become the default.

FWIW / for those interested on an immediately-working prototype, to also prove to the RFC that the feature is desirable, I have released:

Which is a syn-less proc-macro which uses the tricks mentioned by @HeroicKatora or @steffahn to respectively ensure dyn compatibility or prevent an accidental one :slightly_smiling_face:

5 Likes