Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider making common traits object-safe #1468

Open
apasel422 opened this issue Jan 19, 2016 · 15 comments
Open

Consider making common traits object-safe #1468

apasel422 opened this issue Jan 19, 2016 · 15 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@apasel422
Copy link
Contributor

The following traits have Sized as a supertrait:

  • Clone
  • Default
  • From
  • FromStr
  • Into

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:

  • One
  • Step
  • Zero
@rphmeier
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

ticki commented Jan 19, 2016

👍, these bounds are essentially unnecessary.

@apasel422
Copy link
Contributor Author

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
Copy link
Contributor

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

@apasel422
Copy link
Contributor Author

@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
Copy link
Member

eddyb commented Jan 25, 2016

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

@ticki
Copy link
Contributor

ticki commented Jan 25, 2016

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

@eddyb
Copy link
Member

eddyb commented Jan 25, 2016

@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
Copy link
Contributor

durka commented Jan 25, 2016

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
Copy link
Contributor Author

We shouldn't go ahead with this if DST moves are going to happen: rust-lang/rust#20021 (comment).

@durka
Copy link
Contributor

durka commented Jan 25, 2016

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

@eddyb
Copy link
Member

eddyb commented Jan 25, 2016

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.

@nrc nrc added T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC. labels Aug 18, 2016
@matklad
Copy link
Member

matklad commented Dec 21, 2017

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
Copy link

burdges commented Dec 21, 2017

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"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

9 participants