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

What's the reasoning for E0225? #32220

Closed
sgrif opened this issue Mar 12, 2016 · 14 comments
Closed

What's the reasoning for E0225? #32220

sgrif opened this issue Mar 12, 2016 · 14 comments
Labels
T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@sgrif
Copy link
Contributor

sgrif commented Mar 12, 2016

The following code fails to compile:

trait Foo {}
trait Bar {}

fn main() {
    let x: Box<Foo + Bar> = unimplemented!();
}

You can only box multiple traits when the additional bounds are built in. This seems like an extremely odd restriction. Is there an intention to remove it at some point? The workaround is incredibly annoying:

trait Foo {}
trait Bar {}

trait FooAndBar: Foo + Bar {}
impl<T> FooAndBar for T where T: Foo + Bar {}

fn main() {
    let x: Box<FooAndBar> = unimplemented!();
}

The number of these traits grows quite rapidly as you need multiple combinations of various traits. In Diesel's case, many of these will end up needing to be part of the public API which is leading to a lot of confusing and hard to discover types.

@Aatch
Copy link
Contributor

Aatch commented Mar 12, 2016

The main reason why is that we don't have upcasting, and while it's considered a little odd that you can't do Box<T1> -> Box<T2> where trait T1: T2, it's even worse for Box<T1 + T2> -> Box<T1>, even though the reason is the same. The problem is that the vtables for T1, T2 and T1 + T2 are all different in memory so converting between them is difficult.

@sgrif
Copy link
Contributor Author

sgrif commented Mar 12, 2016

Is that not the case for built in traits as well?

@Aatch
Copy link
Contributor

Aatch commented Mar 12, 2016

@sgrif well the built-in traits have no methods, so therefore no vtable, so the vtable for Box<T1 + Send> is the same as Box<T1>.

@sgrif
Copy link
Contributor Author

sgrif commented Mar 13, 2016

I guess I didn't realize that "built-in traits" meant marker traits. Can we not expand this to traits without methods that aren't built in?

@Stebalien
Copy link
Contributor

@sgrif this would make going from no methods to some methods a breaking change (although I agree having some way to do this would be nice).

@jFransham
Copy link
Contributor

I think that it would be nice to have a language item of marker, which would be like a zero-sized struct for types. It would reduce the special-casing of the trait system and allow for code like this:

//This is a regular trait to enforce a single implementation per type
trait Cons {
    type Head;
    type Tail;
}

marker Contains<T>;

impl<N, T, Hstck: Cons<Head=N, Tail=T>> Contains<N> for Hstck;
impl<N, H, T, Hstck: Cons<Head=H, Tail=T>> Contains<N> for Hstck where T: Contains<N>;

Which is currently disallowed in Rust because there's no way to ensure that a method won't be added, and then it would be unclear which implementation to choose for Hstck: Cons<Head=A, Tail=Cons<Head=A, Tail=()>>.

I want this in a current project to allow filtering of types (so I can create a type that takes any type from a given set and violations are caught at compile time, but it can use Any internally).

In my opinion, I would far rather that ambiguities are caught at resolution-time, at least on an opt-in basis. Like, it could fail with an error by default, but you would be able to annotate with #[allow(ambiguous_impls)] and then it will only fail at method resolution.

@comex
Copy link
Contributor

comex commented Mar 16, 2016

The main reason why is that we don't have upcasting, and while it's considered a little odd that you can't do Box -> Box where trait T1: T2, it's even worse for Box<T1 + T2> -> Box, even though the reason is the same. The problem is that the vtables for T1, T2 and T1 + T2 are all different in memory so converting between them is difficult.

Can Rust do what C++ does in at least some of these situations? e.g. the vtable layout of T1 + T2 should be a full vtable for T1 followed by a full vtable for T2 (including duplicate copies of the drop glue pointer and whatever else is needed), so upcasting to T1 or T2 is just a matter of adjusting the vtable pointer. The inheritance case is similar, although it would require duplication of method pointers in diamond cases, e.g. if T1: T2A + T2B, T2A: T3, T2B: T3, T1's layout would have two copies of T3.

That would imply some arbitrary-seeming restrictions on upcasting, though - T1 + T2 + T3 could be casted to T1 + T2 or T2 + T3 but not T1 + T3 (or only single trait results could be allowed). In any case, there is no way in any reasonable scheme to avoid T1 + T2 being a different trait object type than T2 + T1.

+1 on the marker trait idea too.

klingtnet added a commit to klingtnet/ytterbium that referenced this issue Sep 6, 2016
E0225 prevents from using multiple trait bounds of non standard types.
See rust-lang/rust#32220 for details.
@White-Oak
Copy link
Contributor

@steveklabnik Currently, explanation mentions built-in traits. Should these be renamed to marker traits or maybe there should be a short explanation of what is a built-in trait?

@steveklabnik
Copy link
Member

I was not sure that it was marker traits either.

@rust-lang/lang what should we do here?

@nrc
Copy link
Member

nrc commented Oct 3, 2016

There are long-term plans to do something a bit like @comex suggests in order to handle the full downcasting experience (one possible solution uses 'custom DST' with multiple vtable pointers), but that is probably some way off.

I agree this should probably apply to all zero-method traits, not built-in traits, in fact I'm surprised the compiler even has a concept of built-in trait now that we've moved to lang items + auto traits (nee OIBIT).

@steveklabnik steveklabnik added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed A-lang labels Mar 24, 2017
@snuk182
Copy link

snuk182 commented Jun 13, 2017

Also the workaround from the ticket head does not work for std types:

trait FooAndBar: Eq + Ord {}
impl<T> FooAndBar for T where T: Eq + Ord {}

fn main() {
    let x: Box<FooAndBar> = unimplemented!();
}
error[E0038]: the trait `FooAndBar` cannot be made into an object
 --> src/main.rs:7:16
  |
7 |     let x: Box<FooAndBar> = unimplemented!();
  |                ^^^^^^^^^ the trait `FooAndBar` cannot be made into an object
  |
  = note: the trait cannot use `Self` as a type parameter in the supertraits or where-clauses

error: aborting due to previous error

Which makes a task of auto-sortable container of trait implementations (f.i. BTreeMap) really non-trivial.

@nikomatsakis
Copy link
Contributor

At this point we are basically waiting on an RFC to lay out how this should work. Probably this should issue should be moved to the RFC repository.

@sgrif
Copy link
Contributor Author

sgrif commented Jun 17, 2017

Probably this should issue should be moved to the RFC repository.

Sounds good to me

@sgrif
Copy link
Contributor Author

sgrif commented Jun 17, 2017

rust-lang/rfcs#2035

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 PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants