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

Allow a trait to implement its parent trait #1024

Open
drewcrawford opened this issue Mar 28, 2015 · 19 comments
Open

Allow a trait to implement its parent trait #1024

drewcrawford opened this issue Mar 28, 2015 · 19 comments
Labels

Comments

@drewcrawford
Copy link

@drewcrawford drewcrawford commented Mar 28, 2015

The Rust Reference says:

the syntax Circle : Shape means that types that implement Circle must also have an implementation for Shape

Traditionally this is accomplished by adding an implementation to the underlying struct, and thus the trait Circle can only be applied to structs that supply an implementation for Shape.

However, I think it is very often useful (including in the example in the reference) to allow the trait to supply the supertrait implementation all by itself. In the reference example, does it really make sense for every struct : Circle to provide its own implementation of area? No. The area of a circle is always π*r^2.

Notably, it is still true that "types that implement Circle must also have an implementation for Shape." The difference is whether this implementation is supplied by each struct separately, or by Circle itself. The latter is better DRY.

Proposed syntax:

trait Shape { fn area(&self) -> f64;}

trait Circle : Shape { 
    fn radius(&self) -> f64;
    fn area(&self) -> f64 { //here we supply an implementation for Shape
        self.radius().powf(2.0) * 3.14
    }
}

struct MyCircle;
//impl Shape for MyCircle not required since all Circles are known to have an implementation for Shape.
impl Circle for MyCircle { fn radius (&self) -> f64 { 2.0 } }

fn main() {
    let b = MyCircle;
    println!("{}",b.area());
}
@aidancully
Copy link

@aidancully aidancully commented Mar 28, 2015

Does a blanket-impl suffice?

impl<T: Circle> Shape for T {
  fn area(&self) -> f64 {
    self.radius().powf(2.0) * 3.14
  }
}
@drewcrawford
Copy link
Author

@drewcrawford drewcrawford commented Mar 28, 2015

No. Only a single impl of that form can be written. In the current version of the language in any case.

Consider the following example with Circle and Square:

trait Shape { fn area(&self) -> f64;}

trait Circle : Shape { 
    fn radius(&self) -> f64;
}

struct MyCircle;

impl Circle for MyCircle {
    fn radius(&self) -> f64 {
        2.0
    }

}

impl<T: Circle> Shape for T {
  fn area(&self) -> f64 {
    self.radius().powf(2.0) * 3.14
  }
}

trait Square : Shape {
    fn size(&self) -> f64;
}

impl<T: Square> Shape for T {
    fn area(&self) -> f64 {
        self.size() * self.size()
    }
}

fn main() {
    use Shape;
    let b = MyCircle;
    println!("{}",b.area());
}
$ rustc test.rs
rustc test.rs
test.rs:16:1: 20:2 error: conflicting implementations for trait `Shape` [E0119]
test.rs:16 impl<T: Circle> Shape for T {
test.rs:17   fn area(&self) -> f64 {
test.rs:18     self.radius().powf(2.0) * 3.14
test.rs:19   }
test.rs:20 }
test.rs:26:1: 30:2 note: note conflicting implementation here
test.rs:26 impl<T: Square> Shape for T {
test.rs:27     fn area(&self) -> f64 {
test.rs:28         self.size() * self.size()
test.rs:29     }
test.rs:30 }

This fails because the compiler is not sure some struct is not both a Circle and a Square. Of course in this program there isn't one, but that does not rule out one in another crate. If there was such a struct, then there would be ambiguity about which implementation of area would control.

Meanwhile under the present proposal, the compiler can walk from Circle to Shape and decide that the definition of area does not conflict with any other function that is in scope.

@Diggsey
Copy link
Contributor

@Diggsey Diggsey commented Mar 28, 2015

@drewcrawford But the same problem occurs here: what if you have a third trait, "Square", which also implements "Shape", and then have a type for which both "Square" and "Circle" are implemented - you have two conflicting implementations of "area".

In this simple case the compiler could simply error, but what if there are generic impls for "Square" and "Circle", and a downstream create happens to declare a type which matches both generic impls? It introduces a whole new set of ways in which traits could be invalid.

@drewcrawford
Copy link
Author

@drewcrawford drewcrawford commented Mar 29, 2015

@drewcrawford But the same problem occurs here:

No, a different problem occurs here.

  1. In a bare impl, the conflict occurs at the time the impl is defined. That is, you cannot declare one impl <T: Square> and another impl <T: Circle> that defines area. Even if there is no such type Type: Square + Circle you still cannot do it. (Under the present definition of the language.)
  2. In this proposed syntax, you can declare trait Circle: Shape, trait Square: Shape etc. Only in the case that there exists a Type: Square + Circle do you get an error.

Now you might say "Well, we should change the language to do deferred type checking and allow multiple impl <T: K> rather than changing it to allow traits to implement supertraits." But that language change is a lot hairier than this language change.

Minimally, the compiler needs to see if two traits both implement area. If we have as I propose:

trait Circle : Shape { 
    fn radius(&self) -> f64;
    fn area(&self) -> f64 { //here we supply an implementation for Shape
        self.radius().powf(2.0) * 3.14
    }
}

Then whether or not the trait implements area is part of the trait definition. There is a single canonical place where the compiler has to look to answer that question.

Meanwhile if we try to do impl <T: Circle> then the information necessary to find out whether some trait has a definition of area is potentially sprinkled throughout our codebase. This has a lot of sharp edges, for example if one impl is provided in one crate and another impl is provided in another crate it isn't obvious whether those 2 crates can be legally linked, even though they can both be legally compiled individually.

@Diggsey
Copy link
Contributor

@Diggsey Diggsey commented Mar 29, 2015

The syntax you're using impl<T: Square> doesn't make sense in this context? I'm talking about impl<T> Square for T where ... combined with impl<T> Circle for T where.... This is currently perfectly valid today, because Square and Circle are separate traits, but would break under your proposal because there may be a type T which meets both "where" clauses, and the traits are mutually incompatible: at the very least you have the same problems caused by the negative trait bounds rfc, but you're also introducing a whole new set of coherence and forward-compatibility problems.

Either you try to prevent the conflict early, by disallowing both a Square and a Circle generic trait implementation in the same program, or you wait until you find a concrete, monomorphised type which implements both before erroring. In the first case, it means you now have crates which can no longer be used together in the same application (exactly what the coherence rules are there to prevent). In the second case, you break forward compatibility, because even just adding additional trait implementations to upstream crates can break code downstream (again, what the coherence rules are there to prevent).

For this to work, you'd have to come up with a set of sane coherence rules, and I don't think that's possible in this case.

A better explanation might be this: currently the two traits, "Square" and "Circle" can effectively share the "Shape" implementation they both depend on. Under your scheme they can no longer share the same "Shape" implementation because they place additional requirements on the trait beyond it just being implemented (it specifically has to be implemented according to their implementation). These additional requirements don't exist anywhere else in the language.

@drewcrawford
Copy link
Author

@drewcrawford drewcrawford commented Mar 29, 2015

I'm talking about impl Square for T where ... combined with impl Circle for T where.... This is currently perfectly valid today

The following code fails to compile on Rust nightly:

trait Shape { fn area(&self) -> f64;}

trait Circle : Shape { 
    fn radius(&self) -> f64;
}

impl<T> Shape for T where T : Circle {
  fn area(&self) -> f64 {
    self.radius().powf(2.0) * 3.14
  }
}

impl<T> Shape for T where T : Shape {
    fn area(&self) -> f64 {
        self.size() * self.size()
    }
}

fn main() {
}

Perhaps whatever you mean by "This is currently perfectly valid today" would be clearer if you modified this program so that it compiles.

because even just adding additional trait implementations to upstream crates can break code downstream (again, what the coherence rules are there to prevent)

Sure, but changing almost anything upstream can break code downstream. I don't see how this feature especially contributes to that problem in any meaningful way.

@Diggsey
Copy link
Contributor

@Diggsey Diggsey commented Mar 29, 2015

Well it doesn't compile because you changed the code I wrote?

trait Shape { fn area(&self) -> f64;}

trait SomeOtherTrait { fn do_something(&self); }

trait Circle : Shape { 
    fn radius(&self) -> f64;
}

trait Square : Shape {
    fn edge_length(&self) -> f64;
}

impl<T> Circle for T where T : Shape + SomeOtherTrait {
  fn radius(&self) -> f64 {
    42.0
  }
}

impl<T> Square for T where T : Shape + SomeOtherTrait {
    fn edge_length(&self) -> f64 {
        1.0
    }
}

fn main() {
}

That compiles fine, and I can make a type "Foo" which implements "Shape + SomeOtherTrait", and it's still fine, because Square and Circle can share the same implementation of Shape, ie. they don't add additional constraints on their parent trait.

Sure, but changing almost anything upstream can break code downstream. I don't see how this feature especially contributes to that problem in any meaningful way.

Well it's supposed to be the case that adding new trait implementations can be done in a backwards compatible manner. Look at why the negative trait bound RFC has not been accepted.

@drewcrawford
Copy link
Author

@drewcrawford drewcrawford commented Mar 29, 2015

Well sure, that compiles, but it doesn't have anything to do with the motivating problem:

does it really make sense for every struct : Circle to provide its own implementation of area? No. The area of a circle is always π*r^2

In your approach, I can't write

struct MyCircle;
impl Circle for MyCircle { fn radius(&self) -> f64 { 2.0 } }

because

error: the trait `Shape` is not implemented for the type `MyCircle` [E0277]
test.rs:26 impl Circle for MyCircle { }

Instead I would have to write out the π*r^2 formula for every struct that implements Circle, even though logically that implementation should be the same every struct: Circle (assuming Euclidian geometry, of course.)

it's supposed to be the case that adding new trait implementations can be done in a backwards compatible manner.

And that's the case under this proposal as well. More formally, if we have a ShapeLibrary that declares

pub trait Circle : Shape { 
    fn radius(&self) -> f64;
    fn area(&self) -> f64 { //here we supply an implementation for Shape
        self.radius().powf(2.0) * 3.14
    }
}

then our ClientProgram can declare a struct ProgramCircle:

struct ProgramCircle;
impl Circle for ProgramCircle { fn radius(&self) -> f64 { 2.0 } }

When our ShapeLibrary later adds some NonEuclidianCircle:

pub trait NonEuclidianCircle : Shape { 
    fn radius(&self) -> f64;
    fn area(&self) -> f64 { //here we supply an implementation for Shape
        8 / 3.14 * self.radius().powf(2.0) //area of a circle under the negation of Euclid's 5th postulate
    }
}

Since our ClientProgram does not use NonEuclidianCircle it is not affected by the addition of this trait.

@iopq
Copy link
Contributor

@iopq iopq commented Apr 2, 2015

I have the same problem in my application. I cannot express my logic in a clear way and I have to copy paste methods like the area in the example because I have two implementations and two copies of each.

I COULD hack around it, but then I'd need negative trait bounds which Rust doesn't have either

@drewcrawford
Copy link
Author

@drewcrawford drewcrawford commented Apr 2, 2015

For the time being, I'm using the preprocessor to work around. This has some unhappy side effects, namely

  1. Need to Invoke a macro for each class implementing the trait
  2. The macro expands to impl blocks, meaning that the macro needs knowledge of the number of generic parameters of each implementing class. If there are varying numbers of parameters, it gets bad.

Via SO somebody proposed a workaround involving associated items, https://stackoverflow.com/a/29292642

That workaround is novel but it increases the difficulty of manually implementing the trait (as opposed to implementing it from another trait). My problem is such that I have a mix of both cases, so the drawbacks of the preprocessor are preferable to me. But if you don't mix "trait-implemented" and "directly-implemented" traits it might be an approach to study.

@aidancully
Copy link

@aidancully aidancully commented Apr 2, 2015

For what it's worth, I think this is not something the trait system is well-suited to support, and may want to wait for an OOP facility to be included in the language.

@iopq
Copy link
Contributor

@iopq iopq commented Apr 2, 2015

@aidancully: it has nothing to do with that, we already have trait inheritance. The proposals there are for struct/enum inheritance. The proposal here is to satisfy the Shape by all implementors of Circle through a default implementation.

It seems to be a default method oversight that this already doesn't work.

@aidancully
Copy link

@aidancully aidancully commented Apr 2, 2015

@iopq, I see where you're coming from, but as @Diggsey says, allowing a default implementation for a base trait function allows either coherence or forward-compat issues when you have diamond inheritance of traits. I mentioned the OOP proposals because OOP can more naturally provide a mechanism for dealing with diamond inheritance (either by explicitly disallowing it, or by forcing users to specifically choose how to resolve an inheritance conflict) than traits can. I also think that this use case (Shape, Square, Circle, and conflicting default implementations of area) seems better suited to OOP than to trait inheritance.

@shisoft
Copy link

@shisoft shisoft commented Dec 3, 2016

Is there any update on this issue? I have the same problem when implementing a transparent proxy layer for user trait objects. I have to force developers to add a macro for convenience which is very annoying.

@burdges
Copy link

@burdges burdges commented Dec 3, 2016

Appears impossible as stated. See comments by @aidancully and @Diggsey above

I'd think specialization resolves some issues around this. Say via

impl<T: Circle> Shape for T {
  default fn area(&self) -> f64 {
    self.radius().powf(2.0) * 3.14
  }
}

It requires that overlapping impls form chains, but even the proposed lattice rule there cannot provide the full feature being requested here. And you must still write an impl Shape for any particular Circle.

Also, there are cases where trait parameters or associated types can give you the needed disjointness properties, like in https://stackoverflow.com/a/29292642

There are still efforts towards an acceptably limited form of negative reasoning too. See Niko's comment #1672 (comment)

@TedDriggs
Copy link

@TedDriggs TedDriggs commented Dec 7, 2016

Is it a goal of this proposal to allow new parent trait implementations that don't require any changes to structs implementing the child trait?

If not, could something like the following work?

pub trait Shape {
    fn area(&self) -> f64;
}

pub trait Circle : Shape {
    fn radius(&self) -> f64;
}

optin impl<T : Circle> Shape for T {
    fn area(&self) -> f64 {
        self.radius().powf(2.0) * 3.14
    }
}

struct MyCircle;

impl Circle for MyCircle {
    fn radius(&self) -> f64 {
        1.0
    }
}

impl Shape for MyCircle with Circle;

There are two parts to the suggestion:

  1. The abillty to declare an optin impl, which means it only applies to types that choose to claim it.
  2. The ability to declare impl ... with {Trait}, which specifies an implementation to use for the target struct.

For larger traits, that could save a lot of boilerplate code, but it doesn't seem to alter the semantics of determining which impl applies. MyCircle still needs to implement both Circle and Shape, and it has impl statements for both.

It also seems fairly tooling-friendly: The compiler would be able to suggest impl with declarations in the event the author forgot one, and an IDE command could further streamline inserting those.

@withoutboats
Copy link
Contributor

@withoutboats withoutboats commented Dec 21, 2016

It requires that overlapping impls form chains, but even the proposed lattice rule there cannot provide the full feature being requested here. And you must still write an impl Shape for any particular Circle.

What specifically is the issue you see here? This is squarely the sort of use case specialization is supposed to solve, so the more examples the better of things someone might want to do, but can't given the ordering rules we've considered.

@burdges
Copy link

@burdges burdges commented Dec 21, 2016

Afaik there is no "issue" here @withoutboats I just restating in a confusing way. Yes, if you want a examples for an argument for the lattice rule, then I suppose these parent trait cases can easily give you a couple.

@Hainish
Copy link

@Hainish Hainish commented Sep 6, 2019

https://github.com/hainish/multi-default-trait-impl may be useful for some use case subset of the requested feature. It's a procedural macro which allows you to define multiple default implementations of a trait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.