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

Inherent trait implementations #1880

Open
frewsxcv opened this issue Feb 1, 2017 · 27 comments · May be fixed by #2375
Open

Inherent trait implementations #1880

frewsxcv opened this issue Feb 1, 2017 · 27 comments · May be fixed by #2375
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@frewsxcv
Copy link
Member

frewsxcv commented Feb 1, 2017

I've been working a lot with traits lately with this rust-geo pull request.

In that pull request, it splits up concrete geospatial types and geospatial operations. A simplified example:

struct PointType {
    x: f32,
    y: f32,
}

trait PointTrait {
    fn x(&self) -> f32;
    fn y(&self) -> f32;
    fn distance<P: PointTrait>(&self, other: P) -> f32 {
        // distance algorithm
    }
}

impl PointTrait for PointType {
    fn x(&self) -> f32 { self.x }
    fn y(&self) -> f32 { self.y }
}

Now, if someone ever wanted to use this, they'd have to import both the type and the trait:

use geo::{PointType, PointTrait};
let p1 = PointType { x: 1, y: 1 };
let p2 = PointType { x: 2, y: 2 };
println!("distance: {}", p1.distance(p2));

Most of the time, the user is always going to want the associated methods associated with PointType, so it's a little unfortunate that the user has to add another import whenever they want this functionality.

It'd be cool if I could make a trait implementation as exposed so that the triat methods are always available on that specific type. Something like:

exposed impl PointTrait for PointType {
    fn x(&self) -> f32 { self.x }
    fn y(&self) -> f32 { self.y }
}

(There's probably a better keyword to use here than exposed, or maybe some other way to indicate this.)

Then the user can just do:

use geo::PointType;
let p1 = PointType { x: 1, y: 1 };
let p2 = PointType { x: 2, y: 2 };
println!("distance: {}", p1.distance(p2));

I haven't thought too long about this, so I might be overlooking something here. I don't feel strongly at all about this, just expressing a thought.

@Ms2ger
Copy link
Contributor

Ms2ger commented Feb 1, 2017

Please explain what happens when calling point.a() in the following scenario:

crate A {
    struct PointType {
        x: f32,
        y: f32,
    }
}

crate B {
    trait OnePointTrait {
        fn a(&self) -> f32;
        fn b(&self) -> f32;
    }

    exposed impl OnePointTrait for PointType {
        fn a(&self) -> f32 { self.x }
        fn b(&self) -> f32 { self.y }
    }
}

crate C {
    trait OtherPointTrait {
        fn a(&self) -> f32;
        fn b(&self) -> f32;
    }

    exposed impl OtherPointTrait for PointType {
        fn a(&self) -> f32 { self.y }
        fn b(&self) -> f32 { self.x }
    }
}

@frewsxcv
Copy link
Member Author

frewsxcv commented Feb 1, 2017

Could add a restriction such that exposed only works on concrete types defined in the same crate.

On a side note, this could also help out the 'num-traits' crate which IIRC reexports all the trait impls in the 'num' crate.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 1, 2017

You can always put all the traits into one module and do use module::* if all you care about is less typing.

Also, advanced enough IDEs (or rustfix) will be able to automatically insert the necessary imports (I'm working on this at this very moment)

@eddyb
Copy link
Member

eddyb commented Feb 1, 2017

The correct term here, IMO, is "inherent", and this would have to obey the rules of inherent impls.
That is, unless we get some privacy-based flexibility, the type must be defined in the crate of the impl.

@SimonSapin
Copy link
Contributor

use module::* still need to be repeated in every module that uses the type in question.

Agree that "inherent" is the correct term. One way to think of this feature is syntactic sugar to generate an impl with #[inline] inherent methods that call the trait methods of the same name (and takes the same arguments, have the same return type, …). Then naturally this can only be used where new inherent methods can be defined.

@Wopple
Copy link

Wopple commented Feb 1, 2017

IMO, the extra effort of having to write an additional import is worth the flexibility of being able to define your own implementations of functions. I can imagine a case where you want to use a struct but you don't like the implementation of a trait provided by the crate. If that implementation was inherent, how would you opt out of using it in favor of an implementation with the same signature of your choosing?

Edit: never mind (see below)

@frewsxcv
Copy link
Member Author

frewsxcv commented Feb 1, 2017

I can imagine a case where you want to use a struct but you don't like the implementation of a trait provided by the crate. If that implementation was inherent, how would you opt out of using it in favor of an implementation with the same signature of your choosing?

I'm not sure how this idea makes your concern any worse.

e.g. It's already possible for PointType to implement Display, and if you don't like the implementation of Display provided by the crate, you can't do much about it.

@Stebalien
Copy link
Contributor

Inherit impls actually came up when discussing "thin" pointers to traits. One idea (that I liked) was to allow structs to declare which traits they must implement:

struct MyStruct: Trait1 + Trait2 + Trait3; // or struct MyStruct impl Trait1 ...

This would have two side effects (not including thin pointer stuff).

  1. The struct must implement these traits (checked by the compiler).
  2. The user would be able to call MyStruct.trait1_method() without importing Trait1.

The nice thing about this alternative is that it keeps everything in one place.

@SimonSapin
Copy link
Contributor

@Stebalien This is an OK alternative, but it seems a bit redundant with the impl blocks which already name traits and types together. And don’t see how "keeping everything in one place" is valuable when impl MyStruct { fn inherent() } can already be "far" from struct MyStruct

@burdges
Copy link

burdges commented Feb 1, 2017

I'd think the syntax for your exposed impl .. should be adding a pub reference to the trait to the inherent impl for the type, like

trait PointTrait { .. }

impl PointTrait for PointType { .. }

impl PointType {
    pub use PointTrait;
}

or maybe pub trait PointTrait.

Now use module::PointType brings PointTrait into scope as PointType::PointTrait, so it does not conflict with another item named PointTrait, but method notation still works on PointType.

@Wopple
Copy link

Wopple commented Feb 1, 2017

@frewsxcv In the following example, it prints 0 or 1 depending on which trait you import:

struct MyStruct {
    val: u32
}

trait Trait1 {
    fn get_val(&self) -> u32;
}

trait Trait2 {
    fn get_val(&self) -> u32;
}

impl Trait1 for MyStruct {
    fn get_val(&self) -> u32 { self.val }
}

impl Trait2 for MyStruct {
    fn get_val(&self) -> u32 { self.val + 1 }
}

mod test {
    use ::MyStruct;
//    use ::Trait1;
    use ::Trait2;

    fn run() {
        let s = MyStruct { val: 0 };
        println!("val: {:?}", s.get_val());
    }
}

it can't compile if you import both because the get_val function becomes ambiguous.

@frewsxcv
Copy link
Member Author

frewsxcv commented Feb 1, 2017

@Wopple You can differentiate via:

Trait1::get_val(&s)

EDIT: I originally wrote this, but it's wrong:

(s as Trait1).get_val()

@Wopple
Copy link

Wopple commented Feb 2, 2017

@frewsxcv I see, thanks!

@mark-i-m
Copy link
Member

mark-i-m commented Feb 2, 2017

Perhaps something like pub impl would be useful? A nice side effect of this is that you can have some methods be inherent and not others, even within the Trait...

@mark-i-m
Copy link
Member

mark-i-m commented Feb 2, 2017

Also, pub is already a reserved word, whereas inherent and exposed are not, TMK...

@ideasman42
Copy link

ideasman42 commented Feb 8, 2017

One of the down-sides to the current situation (needing to explicitly use the traits), that it makes using traits less attractive - even when they make sense logically. In some code I just don't use traits because of the extra hassles of ensuring they're in the name-space and just impl SomeStruct {...}, the down-side to this is the functions on multiple structs can get out of sync without any warning, see stackexchange question on this topic.

@mark-i-m
Copy link
Member

mark-i-m commented Feb 8, 2017

In some code I just don't use traits because of the extra hassles of ensuring they're in the name-space

Hmm... I can see why it would be annoying having to find the correct trait, but I have never found this to be an excessive "hassle" before... at least, not to the degree that I would rather not use traits. Could you give an example of where this was a huge burden?

@frewsxcv frewsxcv changed the title Idea: Exposed trait implementations Idea: Inherent trait implementations Sep 4, 2017
@frewsxcv frewsxcv changed the title Idea: Inherent trait implementations Inherent trait implementations Sep 4, 2017
@newpavlov
Copy link
Contributor

newpavlov commented Oct 13, 2017

I would love to see this feature implemented, as for example in the RustCrypto I have to re-export traits in every crate to make it more convenient for users, or otherwise they'll have to list additional crate (containing traits) as explicit dependency. Also it makes harder to write documentation, as I have to explain how traits are organized to users who do not want to write generic code and just want to call several methods and have stuff done.

Overall it's a very annoying papercut for crate authors and users.

@burdges
Copy link

burdges commented Oct 13, 2017

It's never been clear to me why RustCrypto uses so many crates instead of features. Yes, crates provide an easier to understand separation, but if the crates all reside in the same repository anyways then most advantages seem moot.

There is no shortage of applications for this language feature though of course, so I'm not objecting to anything. :)

We rand into issues when discussing use in items like impls a few months ago, so the impl MyStruct { pub use Trait; .. } syntax might run into similar difficulties, or maybe it would fix them. I donno if pub impl Trait for MyStruct { .. makes sense, but maybe. And the struct declaring traits it must implement sounds workable.

@Diggsey
Copy link
Contributor

Diggsey commented Jan 21, 2018

I would like to see this feature too: I'm currently looking at generating traits and types from javascript IDL files, and I have to choose between generating inherent methods vs trait methods for interfaces, and neither corresponds cleanly to what the javascript API actually looks like.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jan 21, 2018
@newpavlov newpavlov linked a pull request Mar 27, 2018 that will close this issue
@stepancheg
Copy link

I want to point to (now closed) "custom prelude" RFC #890 Should that RFC implemented, the described problem could be solved with a custom prelude: the client could import PointTrait into their crate prelude.

@idanarye
Copy link

If I could add my suggestion, I would like to see a syntax like this:

impl PointTrait for PointType {
    fn x(&self) -> f32 { self.x }
    pub fn y(&self) -> f32 { self.y }
}

You'd have to use PointTrait in order to access point.x, but point.y will be available in other modules even if they don't use PointTrait. Of course, you can only use pub if the impl block is defined in the same crate as PointType.

The advantages of marking the specific function rather then the impl statement are:

  • Just like with non-trait impl blocks, pub fn will be visible outside the module and just fn without pub will only be visible inside the module. (Though in this case you can use the trait to get access to these methods)
  • Finer picking of which functions are inherent and which are trait-only.
  • Conflicts are easier to resolve - just pick which of the methods are the inherent one.
  • Speaking of conflicts - the error messages will be nicer. Instead of something like "conflicting inherent implementation of trait A and trait B on type T: method foo is defined in both traits" the error message will be more like "inherent method foo` is defined multiple times".

@idanarye
Copy link

I've implemented my syntax with a proc_macro: https://github.com/idanarye/rust-inherent-pub

@linclelinkpart5
Copy link

Throwing in my +1 for this, I have a type that implements a custom trait containing a single trait method, but the method really makes sense to be inherent to the type. However, I also need to be able to generalize over multiple types that impl that custom trait.

@idanarye Your crate looks very close to what I'd need, but I'd want to preserve the arg names and not hide the inherent method from docs. I'm not familiar with proc macros to be able to understand if the former is possible, though?

@idanarye
Copy link

idanarye commented Jun 3, 2021

@linclelinkpart5 This is probably a discussion for a ticket in my repository, but IIRC I used arg0, arg1, etc. instead of using the original argument names because I was under the impression that you can't use variables with single leading underscrore. I just checked and this assumption is not true (maybe it was true back then? Not going to dig in the past of Rust history...) but under it had I preserved the argument names then this

#[inherent_pub]
impl Foo for Bar {
    pub fn foo(&self, _baz: Baz) {
        // ...
    }
}

Would generate this:

impl Bar {
    pub fn foo(&self, _baz: Baz) {
        <Self as Foo>::foo(self, _baz)
    }
}

Which would be illegal because it uses _baz. And if I tried to detect it and emit this:

impl Bar {
    pub fn foo(&self, baz: Baz) {
        <Self as Foo>::foo(self, baz)
    }
}

I would have a problem with:

#[inherent_pub]
impl Foo for Bar {
    pub fn foo(&self, _baz: Baz, baz: Baz) {
        // ...
    }
}

Which would become:

impl Bar {
    pub fn foo(&self, baz: Baz, baz: Baz) {
        <Self as Foo>::foo(self, baz, baz)
    }
}

So rather than dealing with all the edge cases it was easier to just rename all the arguments.

@branpk
Copy link

branpk commented Jul 22, 2022

It would be nice if it could support this case as well:

pub struct PointType { ... }

// Want magnitude to be accessible as an inherent method on PointType
trait Magnitude {
  fn magnitude(&self) -> f32;
}

// Helper trait for implementing Magnitude, may or may not want to expose to user
trait Coordinates {
  fn coords(&self) -> Vec<f32>;
}
impl<T: Coordinates> Magnitude for T {
  ...
}

// PointType implements the helper type explicitly, but not Magnitude
impl Coordinates for PointType { ... }

I think this is a +1 for pub use Magnitude within impl PointType and a -1 for the above #[inherent_pub] syntax.

@KatsuoRyuu
Copy link

My problem with this is that if this got implemented i would now have to sit at each case deciding should i make a trait or and inherited implementation.
I have a lot of code that depend on different functionalities from different traits, adding this would put a large amount of extra complexity.

It would also make rust and many pacakages as a language less flexible because you would probably see an overuse of inheritance causing more packages that does the same to show up.

My personal opinion is that this kind of inheritance is not a good direction.

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

Successfully merging a pull request may close this issue.