Removing `Self: Sized` and backward compatibility

language design
1

There are a lot of in-progress proposals that would extend the set of object-safe trait methods. Once these proposals are implemented and stabilized, library maintainers may want to remove Self: Sized bounds on their trait methods. However, there's a backward compatibility concern here.

Consider a crate frobnify-lib:

pub trait Frobnify {
    // `where Self: Sized` because we want `Frobnify` to be dyn-safe,
    // and you can't call methods with non-`Self` generic params from a vtable
    fn frobnify<T: Send>(&self, _: &T) where Self: Sized {
        println!("blutzing the kroo...");
    }

    // ...
}

And a crate double-frobnify that depends on frobnify-lib:

use frobnify_lib::Frobnify;

struct DoubleFrobnifier<T: Frobnify + ?Sized>(T);

impl<F: Frobnify + ?Sized> Frobnify for DoubleFrobnifier<F> {
    fn frobnify<T: Send>(&self, other: &T) where Self: Sized {
        self.0.frobnify(other);
        self.0.frobnify(other);
    }

    // ...
}

Now, let's say that a new Rust version allows methods with non-Self generic params in vtables. frobnify-lib publishes a new version to support this:

pub trait Frobnify {
    fn frobnify<T: Send>(&self, _: &T) {
        println!("blutzing the kroo...");
    }

    // ...
}

Now, double-frobnify is broken—even though Frobnify provided a default impl for frobnify.

impl<F: Frobnify + ?Sized> Frobnify for DoubleFrobnifier<F> {
    // Error! `Self` not known to be `Sized`!
    fn frobnify<T: Send>(&self, other: &T) where Self: Sized {
        self.0.frobnify(other);
        self.0.frobnify(other);
    }

    // ...
}

Notably, there are many such Self: Sized bounds in the standard library. As things stand, they can never be removed—unless we can find a solution to this conundrum.

2 Likes
2

A potential solution is to leverage specialization. We could say that when a trait has a method for which it provides a default impl:

pub trait Frobnify {
    fn frobnify<T: Send>(&self, _: &T) {
        println!("blutzing the kroo...");
    }
}

And an implementation of the trait overrides the default method impl, but with a more restrictive bound:

impl<F: Frobnify + ?Sized> Frobnify for DoubleFrobnifier<F> {
    fn frobnify<T: Send>(&self, other: &T) where Self: Sized {
        self.0.frobnify(other);
        self.0.frobnify(other);
    }
}

The result desugars to a set of specialized impls:

// desugaring of above code block

impl<F: Frobnify + ?Sized> Frobnify for DoubleFrobnifier<F> {
    default fn frobnify<T: Send>(&self, other: &T) {
        // used default impl from trait definition
        println!("blutzing the kroo...");
    }
}

impl<F: Frobnify + Sized> Frobnify for DoubleFrobnifier<F> {
    // Specializes above
    fn frobnify<T: Send>(&self, other: &T) {
        self.0.frobnify(other);
        self.0.frobnify(other);
    }
}

Notably, Sized should always be safe to specialize on. However, there is a potential pitfall here, in that until double-frobnify is updated, DoubleFrobnifier will have surprising behavior for non-Sized types; it will only frobnify them once.

3

Incidentally, I think that Self bounds on trait methods without a default impl are an anti-pattern and should be linted against. They mean that implementing one trait can suddenly break your implementation of another (or will be able to, once feature(trivial_bounds) is stabilized). Such methods belong in a separate trait.

4

I am not convinced that this can be done without an edition. Are there any other proposals on how to change it without breaking existing code?

5

Do you have a specific example where the proposal would be a breaking change? I haven't been able to find one so far, assuming we restrict it to "always safe to specialize on" traits (which includes Sized, though an RFC would be needed to commit to never changing that).

A default method impl can only depend on the definition of the trait and its supertraits, it doesn't know anything else about Self. So anything it could do, could also be done by code outside of the impl (or even the crate) that imports the trait. So, unless my reasoning is mistaken, there is no risk of this somehow exposing an unsound API. Surprising, maybe, but not UB.

6

Oh sorry, I missed the point, it is not code breakage... I was taken aback by the wrong behavior of a library that was not updated (your frobnify example). And for some reason I thought that would break existing code.

7

I think that is something we can lint for (warn when you call a method that has an impl for different constraints, but the default impl applies). That way it would not be as bad.

8

Problem with adding a warning there is that is likely not something you can fix, it needs to be fixed in the upstream library (and they may not be able to fix it straight away if their dependent has a more lax MSRV policy than themselves, they would need to update their minimum requirement). I believe there is a general policy of avoiding unactionable warnings (which is why even getting forward-compatibility warnings from dependencies has been such a slow process).

A more appropriate location for the warning could be on the impl if you override the default method with a different set of constraints. That would essentially act similar to the old specialization being marked deprecated.

9

But it could be very confusing when you use some function of a trait and get the default behavior... Somebody has to file the issue at the upstream library. And there are still some things downstream can do, e.g. provide an extension trait that implements the correct behavior. I think such a warning could prevent long debug sessions, because the Self: Sized bound can be easily overlooked.

10

I suspect that often, the trait definer will be in a good position to judge whether loosening bounds is likely to lead to unexpected behavior in impls. For example, Iterator has many Self: Sized methods that are generally overridden only for performance optimizations.

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.