unsized params in traits

[ionicmc-rs]

issue for this RFC rust-lang/rust#134475

This RFC proposes checking for T: Sized in required trait methods (which do not check for some reason) which will be a future compatibility lint

Rendered

[oli-obk]

The formatting is a bit unusual. Maybe have a look at other RFCs for inspiration.

Also there's a short motivation that is only clear if one already read the issue or a later section. I think parts of the guide explanation are actually motivation pieces

[oli-obk]

We call these future incompatibility lints https://rustc-dev-guide.rust-lang.org/diagnostics.html?highlight=Futur#future-incompatible-lints

[oli-obk]

That kind of question doesn't belong in the RFC text

[oli-obk]

This kind of detail is not necessary.

[oli-obk]

An alternative could be to generate where Self: Sized bounds from the signature where applicable and thus never actually forbid it.

[zachs18]

This is what the Reference (inaccurately) says currently happens, for context.

source

https://doc.rust-lang.org/reference/items/traits.html#object-safety

[...] A trait is object safe if it has the following qualities [...] All associated functions must either be dispatchable from a trait object or be explicitly non-dispatchable: [...] Dispatchable functions must: [...] Not have a where Self: Sized bound (receiver type of Self (i.e. self) implies this). [...] Explicitly non-dispatchable functions require: Have a where Self: Sized bound (receiver type of Self (i.e. self) implies this).

(emphasis mine)

[compiler-errors]

This would, for the record, break the dyn compatibility of FnOnce.

[oli-obk]

So does this RFC. And we'd have to revert this RFC whenever we figure out generally allowing folk to call owned methods on Box<dyn TheirTrait>

[oli-obk]

These examples are the opposite of prior art. They explain where there's no prior art. Maybe you can find similar requirements in Rust? I think function pointers are similarly definable with arguments that no real function can have

[oli-obk]

The trivial bounds feature has similar situations, too

[ionicmc-rs]

Hey everyone im going to make another RFC for this

[oli-obk]

One drawback is that it causes a lot of churn. The RFC should have a rationale for what the advantage is of adding a Self: Sized bound to e.g. IntoIterator::into_iter or Add:add.

[lcnr]

I personally find the argument "why allow fn foo(self) for ?Sized traits if that causes the trait to be unimplementable for any non-Sized types" compelling and my immediate reaction was that linting here may be useful.

Thinking about this (and from a short private chat about this), there are some significant issues here

• feature(unsized_fn_params) should get stabilized at some point, causing any Self: Sized bounds to no longer be necessary. Removing such bounds will be a breaking change
• potentially very high impact, even just the standard library has many such traits

I feel that especially the existence of feature(unsized_fn_params) and it's use in stable trait impls, e.g. Box<F>: FnOnce makes this lint a non-starter.

---

@ionicmc-rs Please do not open a separate RFC here but instead update this RFC in place if necessary. Also, it may be good to take the RFC a bit slower and first participate in some less formal discussion to save yourself unnecessary work.

[ionicmc-rs]

@ionicmc-rs Please do not open a separate RFC here but instead update this RFC in place if necessary. Also, it may be good to take the RFC a bit slower and first participate in some less formal discussion to save yourself unnecessary work.

Ok wait let me try that

[ionicmc-rs]

It is now updated, closing the new PR

[ionicmc-rs]

feature(unsized_fn_params) should get stabilized at some point

But it is marked as internal though, why?

[lcnr]

feature(unsized_fn_params) should get stabilized at some point

But it is marked as internal though, why?

rust-lang/rust#126830 based on reasoning from rust-lang/rust#123894 (comment). My perspective is that the current impl is simply known to be insufficient, but something like it remains necessary and imo expected to get stabilized at some point

[ionicmc-rs]

feature(unsized_fn_params) should get stabilized at some point

But it is marked as internal though, why?

rust-lang/rust#126830 based on reasoning from rust-lang/rust#123894 (comment). My perspective is that the current impl is simply known to be insufficient, but something like it remains necessary and imo expected to get stabilized at some point

Hmm... So this just stays as a lint?

[lcnr]

Hmm... So this just stays as a lint?

Even as a lint this is highly undesirable if there'll ever be a future where feature(unsized_fn_params) is stable:

// Imagine this trait, if we'd lint on this there would be 2 possible solutions
trait Trait {
    fn foo(self);
}

// Add a `Sized`  bound to the trait
trait SizedAsSuper: Sized {
    fn foo(self);
}
// Add a `Sized` bound to the method
trait SizedOnMethod {
    fn foo(self)
    where
        Self: Sized;
}

Note how in these cases the Sized requirement is not something we actually care about. It's only about making something that's currently implicit explicit. It feels clear to me that once feature(unsized_fn_params) is stable we'd want to support impls relying on this feature. Meaning that we'd have to remove the Sized bounds of the trait again.

Now, while this is already an issue as it means you'd have to wait for the trait definitions in your dependencies to be updated before being able to use a new feature, the dependencies can also not be updated without a breaking change.

trait RelyOnSuper: SizedAsSuper {
    fn needs_sized() -> [Self; 8];
}
trait RelyOnSuperErr: Trait {
    fn needs_sized() -> [Self; 8]; // ERROR: `Self: Sized` is not known to hold
}

Right now I don't think adding a where-bound to the method can be exploited, but I do expect 2 potential ways in which it will be in the future

// We probably want to allow impls for unsized types
// to not define methods with trivially unsatisfiable bounds.
//
// Especially as otherwise adding `Self: Sized` as a method where-bound
// doesn't actually help with the issue of "this trait is accidentally not
// implementable for unsized types"
trait RelyOnMethodBound {}
impl SizedOnMethod for dyn RelyOnMethodBound {}
// Removing the `Self: Sized`  bounds would then make this an error
impl Trait for dyn RelyOnMethodBound {}

alternatively, once we stabilize feature(trivial_bounds), the following would break https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=8384aee37ed6f14df660ab7b988c2b64:

// The `Self: Sized`  bound is only allowed because it's
// implied by the trait definition.
trait RelyOnMethodBound {}
impl SizedOnMethod for dyn RelyOnMethodBound {
    fn foo(self)
    where
        Self: Sized 
    {}
}

[ionicmc-rs]

Well this issue needs to be addressed in some way, even if not like this

[ionicmc-rs]

alternatively, once we stabilize feature(trivial_bounds), the following would breakhttps://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=8384aee37ed6f14df660ab7b988c2b64:

link doesn't work

[lcnr]

Well this issue needs to be addressed in some way, even if not like this

why? we can keep this somewhat suboptimal status quo which will end up being perfectly fine once feature(unsized_fn_params) has been stabilized. The way to address this issue is therefore just stabilizing that feature, isn't it?

[ionicmc-rs]

Well this issue needs to be addressed in some way, even if not like this

why? we can keep this somewhat suboptimal status quo which will end up being perfectly fine once feature(unsized_fn_params) has been stabilized. The way to address this issue is therefore just stabilizing that feature, isn't it?

Well I guess, but that feature needs to be stabalized before this issue is addressed, which is suboptimal but I guess it's ok

[oli-obk]

so... there are multiple moderately related issues at hand (mostly repeating what lcnr said in a different order):

1. You can't implement traits for unsized types at all if they have a method that takes self by value

trait Foo {
    fn foo(self);
}
impl Foo for str {
    fn foo(self) {} // Error (for now, until we get unsized locals)
}

2. You can't even implement traits for unsized types if the self by value methods have a Self: Sized bound:

trait Foo {
    fn foo(self) where Self: Sized;
}
impl Foo for str {} // Error: not all members implemented

Well you can, but only with trivial_bounds, which also feels very useless

#![feature(trivial_bounds)]
trait Foo {
    fn foo(self) where Self: Sized;
}
impl Foo for str {
    fn foo(self) where Self: Sized {}
    //^ Uncallable function, but compiles
}

3. A trait with methods taking self by value are not dyn safe not actually a problem

trait Foo {
    fn foo(self);
}

fn bar(_: &dyn Foo) {} // Error: not dyn safe

---

---

I think we can do something about point 2.: Similar to how

trait Foo {
    fn foo(self) where Self: Sized;
}

fn bar(_: &dyn Foo) {} // ok!

works just fine, because all methods with Self: Sized bounds are not available on the dyn Foo.

So we could just allow

trait Foo {
    fn foo(self) where Self: Sized;
}
impl Foo for str {}

in the same way.

Unfortunately, as lcnr already mentioned, we cannot just make

trait Foo {
    fn foo(self);
}
impl Foo for str {}

work, as with the unsized_fn_params feature it will be just fine:

#![feature(unsized_fn_params)]
trait Foo {
    fn foo(self);
}
impl Foo for str {
    fn foo(self) {} // works
}

so... The only thing this RFC would achieve is to push authors of crates with public trait declarations to make them add where Self: Sized bounds to their self by value methods, which they would then later have to revert with a breaking change. The standard library would disable the lint entirely, because it cannot do a breaking change. Most traits also don't really make sense here, as most traits have a single method, and then you'd just end up with a trait that doesn't have a method for unsized types acting as if it is dyn safe (it is now, but it's also not useful as you can't do anything with it) and that isn't useful even as a generic parameter that much, because all of those have an implicit T: Sized bound, so the method is available, but you can't call those methods with dyn types.

My conclusion thus is still to close the RFC, and just implement allowing

trait Foo {
    fn foo(self) where Self: Sized;
}
impl Foo for str {}

with a simple FCP on the PR adding it, similar to what I did in rust-lang/rust#112319

I tried to do this a while back (rust-lang/rust#112929), so it isn't trivial, but we can probably do a simple version just for Self: Sized similar to what we have for dyn safety

[zachs18]

3. A trait with methods taking self by value are not dyn safe

trait Foo {
    fn foo(self);
}

fn bar(_: &dyn Foo) {} // Error: not dyn safe

There's no error here; trait Foo actually is dyn-compatible currently, there's just no way to call <dyn Foo>::foo on stable. playground link