Conversions to dyn Trait are too implicit -- we need syntax

language design
1

When writing code using a dyn Trait where Trait has a blanket impl for basically everything, I commonly have a hard time trying to figure out if I'm actually calling a method on that dyn Trait, or if I'm doing something like converting &mut dyn Trait to another wrapping dyn Trait and calling the method on that which is generally what I'm not trying to do. e.g. I just found a bug caused by inadvertently calling a function taking &mut dyn Trait with an argument of type &mut Box<dyn Trait> from a let destructuring.

I think Rust needs some explicit syntax for conversions to dyn Trait, specifically so you can easily tell where those conversions are happening and (more importantly) you know where they aren't happening. Imo this isn't really a problem for other kinds of unsizing (e.g. Vec<T> -> [T]) since you can't repeatedly convert to an unsized type -- once you have &[T] you can't unsize it to a slice again.

Maybe on a new edition require writing dyn in front of any spot you want a conversion to dyn Trait?

3 Likes
2

Do you have a concrete example? I'm not sure I understand what you mean nor where this comes up.

3 Likes
3

in code like:

pub trait Foo: 'static + Clone + Send + Sync + fmt::Debug {}

impl<T: Foo> Foo for Arc<Foo> {
    fn bar(&self) -> i32 {
        T::bar(self)
    }
}

trait DynFooTrait: 'static + Send + Sync + fmt::Debug {
    fn as_any(&self) -> &dyn Any;
    fn bar_dyn(&self) -> i32;
}

impl<T: Foo> DynFooTrait for T {
    fn as_any(&self) -> &dyn Any {
        self
    }
    fn bar_dyn(&self) -> i32 {
        self.bar()
    }
}

#[derive(Debug, Clone)]
pub struct FooDyn {
    foo: Arc<dyn FooTrait>,
    extra: Extra,
}

impl FooDyn {
    pub fn do_it<T: Foo>(&self) -> Option<i32> {
        let Self { foo, extra } = self; // destructure so we don't forget new fields we might add later
        dbg!(extra);
        Some(foo.as_any().downcast::<T>()?.bar()) // here's the bug, did you spot it?
    }
}

The bug is forgetting to deref Arc<dyn FooTrait> to dyn FooTrait before calling as_any(), so you get a underlying type of Self = Arc<dyn FooTrait> rather than Self = T

1 Like
4

For syntax, to support method chaining, maybe have something like .dyn (with similar syntax to .await)?

I think that could be pretty reasonable, here's some example usage:

pub struct MyStruct {
    a: Box<dyn Any>,
    b: i32,
    c: String,
    d: bool,
}

impl fmt::Debug for MyStruct {
    fn fmt(&self,  f: &mut fmt::Formatter<'_>) -> fmt::Result {
        let Self { a: _, b, c, d } = self; // destructure so we don't forget new fields
        f.debug_struct("MyStruct")
            .field("b", b.dyn)
            .field("c", c.dyn)
            .field("d", d.dyn)
            .finish_non_exhaustive()
    }
}

pub fn try_cast<T: 'static, U: 'static>(v: &T) -> Option<&U> {
    Any::downcast_ref(v.dyn)
}

pub fn try_cast_no_dyn<T: 'static, U: 'static + Copy>(v: Box<T>) -> Option<U> {
    let v: Box<dyn Any> = v.dyn;
    // with new syntax it correctly returns Some when T = U since it gets autoderefed instead of unsized
    Any::downcast_ref(&v).copied()
}
3 Likes
5

We already have the syntax: expr as Box<dyn Trait>. It doesn't have the usual pitfalls of as-casts, since for complex types it can only do unsizing or type ascription. We could just require specifying the cast explicitly.

6

yes, as can be used for dyn casts, but imo it's way too verbose to be the replacement for implicit dyn casts -- do you really want to write all of .field("foo", foo as &dyn fmt::Debug) every time? hence why I proposed .dyn since it is quite short but still explicit syntax that afaict isn't conflicting with any existing syntax so could be enabled in all editions where dyn is a keyword -- everything from edition 2018 onward. implicit dyn coercion could be made a warning in those editions and just not happen without .dyn in the next edition.

1 Like
7

That's not an implicit conversion to dyn FooTrait at the call site. It's dispatch to a method on the Arc<_> type.

4 Likes
8

yup, so then you could write it <dyn FooTrait>::as_any(foo) and it would be a dyn cast causing the bug

9

I've run into this issue myself, as well as when helping others. The crux of the problem happens in the following situation:

  • You have a &[mut] PtrTo<dyn Trait> (by PtrTo I mean something which implements Deref{,Mut}).
  • You want to use it as &[mut] dyn Trait.
  • In a typical situation, this would trigger a deref coĆ«rcion, and result in a ref to the original dyn Trait.
  • :warning: but if/when PtrTo<dyn Trait> : Trait, another coĆ«rcion is possible: the Unsize<dyn Trait> coĆ«rcion! :warning:
    • And not only that, but this coĆ«rcion takes priority!

I wonder if an alternative and less invasive solution would then be to have a dedicated lint to catch this:

#![warn(ambiguous_coercion)]

It would warn in this situation, requiring the user to disambiguate, with the lint showing the user how:

  • if what they wanted was the deref coĆ«rcion (such as in the OP), to explicitly type out the deref, e.g., by doing: &[mut] **;
  • if what they wanted was the dyn unsizing (unlikely), they could:

    • allow the lint at the operation site (if this is the only option we'll have to rename the lint to make that pattern more readable);

    • unsize_{ref,mut}::<PtrTo<dyn Trait>, dyn Trait>(expr)

      where unsize_{ref,mut} would be some helper function(s) which the stdlib could start offering:

      //! `::core::ops`

fn unsize_ref<T, DynTrait : ?Sized>(r: &T) -> &DynTrait
where
    T: Unsize<DynTrait>,
{
    r /* as _ */
}

// ditto for `unsize_mut`

Assuming that there be no other cases of ambiguous/surprising unsizing-to-dyn coƫrcions, I'd deem this approach:

  • less disruptive/churny for all the non-ambiguous instances of unsizing-to-dyn coĆ«rcions out there,
  • useful "immediately" / even for old editions;
  • not requiring a language modification (the .dyn syntax), but a mere stdlib API.
    • Counterpoint: this API is kind of exposing the unstable Unsize API, so it would need to be:

      • either just as unstable (e.g., we might want to stabilize that trait?),

      • or risk exposing this pattern.

        • (note that we have a way to do Try::from_residual() in stable Rust (using try_fold()), so there is kind of "precedent" for exposing part of an unstable API under a stable one.)

        The risk in question is that if, for instance, Unsize were to be removed / or unimplemented in certain cases, this could break certain usages of unsize_{ref,mut}.

15 Likes