rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.93k stars 1.57k forks source link

Consider making common traits object-safe #1468

Open apasel422 opened 8 years ago

apasel422 commented 8 years ago

The following traits have Sized as a supertrait:

The bound on the first four can be moved from the trait to a method-level where clause, while the bound on Into can be removed entirely.

Doing this has the benefit of allowing otherwise object-safe traits to use these as a supertrait, but has the drawback of being a breaking change (albeit one that I doubt affects much code).

These other traits also extend Sized, but are unstable:

rphmeier commented 8 years ago

I personally don't agree with this. Removing the sized bounds on these traits by moving them onto their relevant methods really destroys the semantic meaning of the trait. Clone is for values that can copy themselves. Default is for types for which a default value can always be produced. Implementing this suggestion is basically adding an asterisk to each of those descriptions reading "...except not always". While derived implementations of Clone and Default wouldn't suffer, any other code that relies on these traits to work as described is forced to add an extra T: Sized bound, which is confusing and frankly shouldn't be necessary.

The contracts of these traits really do hinge on the types being sized. Changing that will only cause people more confusion.

apasel422 commented 8 years ago

Type parameters that are bound with these traits are already Sized unless they explicitly opt-in to ?Sized. Traits that use them as a supertrait do get the inherited Sized bound, which is where the breakage will likely come from.

The implication for code like this:

trait Default {
    fn default() -> Self where Self: Sized;
}

trait Foo: Default {
    ...
}

is that all types that implement Foo and are Sized must implement Default::default, but that unsized types may implement Foo as well, in which case they need not (and currently cannot) implement Default::default. As far as I know, there is a hope that Rust will eventually allow moving unsized types by value, and these traits will not be usable for such types. (Consider implementing Default for [T].) In that case, the method-level where clauses will also prevent them from being used with unsized types, so my solution isn't entirely satisfactory, but that is a larger issue that applies to additional traits.

ticki commented 8 years ago

:+1:, these bounds are essentially unnecessary.

apasel422 commented 8 years ago

Once unsized by-value exists, it seems like the best approach would be to remove the Sized bounds from these entirely and change the object-safety rules to permit such methods.

Stebalien commented 8 years ago

This feels like it could be useful but I can't think of any actual use cases. What was your motivation?

apasel422 commented 8 years ago

@Stebalien I came across this while experimenting with a trait hierarchy in https://github.com/apasel422/eclectic, and it would be nice to reuse the existing traits instead of creating ad-hoc methods (e.g. trait Collection: Into<Vec<Self:Item>> instead of trait Collection { fn into_vec(self) -> Vec<Self::Item>; }).

But it might be better to hold off on any changes related to this until we have a clearer picture of how the by-value unsized stuff works.

eddyb commented 8 years ago

Sadly, this is not backwards-compatible due to rust-lang/rust#25776.

ticki commented 8 years ago

@eddyb I believe this regression is small enough for the price being worth paying.

eddyb commented 8 years ago

@Ticki It's actually even worse, because trait Trait: Clone { fn new() -> Self; } compiles now. We might be willing to reconsider if there are zero regression in a crater run.

EDIT: Apparently it needs to be trait Trait { fn new() -> Option<Self> { None } } for some obscure reason.

durka commented 8 years ago

I'm trying this for a crater run. (There's already fallout within the compiler so I'm not expecting an easy ride.)

It seems that iter::Step truly can't be made object-safe because it relies on PartialOrd<Rhs=Self> so the Sized bound can't be removed.

apasel422 commented 8 years ago

We shouldn't go ahead with this if DST moves are going to happen: https://github.com/rust-lang/rust/issues/20021#issuecomment-170670486.

durka commented 8 years ago

Does DST-by-value just eliminate object safety as a concern?

eddyb commented 8 years ago

Keep in mind you can't just return a DST.

There is a hack where you magically pass an Allocator or Placer to allocate that value in, but it's too much spooky action at a distance and unlikely (IMO) to be implemented.

matklad commented 6 years ago

This feels like it could be useful but I can't think of any actual use cases. What was your motivation?

Just hit a use case where I want to have an argument of type a: &A, where trait A: Copy.

I have an AstNode trait. It is : Copy because all different ast nodes are represented as indexes into a node table. I also have a NameOwner: AstNode trait which is implemented by some concrete nodes.

Now I want to process all NameOwners from a particular file, and I'd like to pass a &NameOwner to the callback, so as to avoid monomorphization and use dynamic dispatch on a stack-allocated object.

There are several workarounds which I could use here, but it would be really nice if this just worked :)

burdges commented 6 years ago

If TraitA : TraitB then a &TraitA includes a &TraitB to invoke fn foo<A: TraitA>(a: &A). I'd think a reduced type &(TraitA - TraitB) makes sense, and implementing it manually provides one work around, so maybe a proc macro could build "object safe shadow traits"?