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

Trait inheritance for bounded type parameters #4070

Closed
wants to merge 5 commits into from

Conversation

brson
Copy link
Contributor

@brson brson commented Nov 29, 2012

r? @pcwalton

This makes trait inheritance work for lots of test cases involving bounded type parameters, type substitutions and static methods.

Here's one with type substitutions: https://github.com/brson/rust/blob/39dbf4fe74925b1c238c8bc82911e85fd91c3d0b/src/test/run-pass/trait-inheritance-overloading.rs

And one with static methods: https://github.com/brson/rust/blob/39dbf4fe74925b1c238c8bc82911e85fd91c3d0b/src/test/run-pass/trait-inheritance-static2.rs

The basic strategy it uses is to expand bound lists to include their supertraits, in

trait A { } trait B: A { }
fn<T: B>() { }

<T: B> is treated in the internal vtable calculations as <T: B A>. This is made easier and more obvious by the limitation that each impl implements a single trait, not that trait and its supertraits.

trait A { fn f() }
trait B: A { fn g() }

impl S: A { fn f() }
impl S: B { fn g() }

This limitation makes the implementation straightforword and has no new implications for coherence. trait B: A just means that to instantiate B there must also be an impl of A. The trait inheritance list has strong parallels to the bounded typaram list.

I still don't understand type substitutions and why lookup_vtables and push_inherent_candidates_for_param do their substitutions differently. You can see that push_inherent_candidates_for_param is the only code that does the trait hierarchy traversal that doesn't use the new iter_bound_traits_and_supertraits code because it needs to thread substitutions through. I could keep going and try to abstract iter_bound_traits_and_supertraits into a generic fold but I figure now is the time to get review and see if this is looking ok.

Future work:

  • Ensure that all supertraits have impls when implementing a trait. i.e. if I define impl S: B { } there should also be an impl S: A { } somewhere. This isn't required for soundness since the lack of A will be detected when somebody tries to actually use S as A, but it makes sense to enforce.
  • Objects and inheritance. They don't work yet.
  • Failure test cases

@bstrie
Copy link
Contributor

bstrie commented Nov 29, 2012

Does this work? If not, is it planned to work eventually?

trait A { fn f() }
trait B: A { fn g() }

impl S: B {
    fn f() {}
    fn g() {}
}

fn foo<T: B>(bar: T) { bar.f(); }

@brson
Copy link
Contributor Author

brson commented Nov 29, 2012

@bstrie No, it does not work - each trait must have it's own impl. I think it should work, but with different syntax to make it clear that you are implementing multiple traits. Maybe something like this:

// Implement multiple, specific traits
impl S: B, A { ... }

// Implement B and all its supertraits
impl S: B* { ... }

@pcwalton
Copy link
Contributor

I'd like to hold off on the proposed implement-multiple-traits-at-a-time syntax; it adds complexity and I'm not sure we need it (Haskell does not have it, for example).

@bstrie
Copy link
Contributor

bstrie commented Nov 29, 2012

From the traits paper I was under the impression that implementing a given trait made it your responsibility to implement all methods of that trait (sans defaulted methods), regardless of whether those methods belonged directly to the trait or whether they were inherited from supertraits. Is this incorrect, or is Rust just going with a different design?

@bstrie
Copy link
Contributor

bstrie commented Nov 29, 2012

In fact my example managed to accidentally reproduce this test case exactly:

https://github.com/mozilla/rust/blob/master/src/test/run-pass/trait-inheritance-simple.rs

@bstrie
Copy link
Contributor

bstrie commented Nov 29, 2012

Or perhaps not exactly reproduce, since the bound is on the parent trait rather than the child trait.

@brson
Copy link
Contributor Author

brson commented Nov 29, 2012

@bstrie: That test case is rewritten in this patch to use multiple impls

@brson
Copy link
Contributor Author

brson commented Nov 29, 2012

So I haven't read the traits paper, but what we have is probably different, since our traits are type classes. /me reading now

@pcwalton
Copy link
Contributor

r=me

@brson brson closed this Nov 30, 2012
RalfJung added a commit to RalfJung/rust that referenced this pull request Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants