-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: inherent trait implementation #2375
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
- Feature Name: inherent-trait-impl | ||
- Start Date: 2018-03-27 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Provides a mechanism to declare "inherent traits" for a type defined in the same crate. Methods on these traits are callable on instances of the specified type without needing to import the trait. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
There are two similar cases where this is valuable: | ||
|
||
- Frequently used traits. | ||
|
||
Sometimes getting the right abstractions require breaking up a type's implementation into many traits, with only a few methods per | ||
trait. Every use of such a type results in a large number of imports to ensure the correct traits are in scope. If such a type is used | ||
frequently, then this burden quickly becomes a pain point for users of the API, | ||
especially if users do not care about writing generic code over traits. | ||
|
||
- Mapping object-oriented APIs. | ||
|
||
When mapping these APIs to rust, base classes are usually mapped to traits: methods on those base classes will need to be callable on any | ||
derived type. This is sub-optimal because to use a class method a user must now know which class in the hierachy defined that | ||
method, so that they can import and use the corresponding trait. This knowledge is not required when using the same API from an | ||
object-oriented language. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
The feature is implemented using a new attribute which can be applied to trait | ||
`impl` blocks: | ||
|
||
```rust | ||
pub trait Bar { | ||
fn bar(&self); | ||
} | ||
|
||
pub struct Foo; | ||
|
||
#[inherent] | ||
impl Bar for Foo { | ||
fn bar(&self) { println!("foo::bar"); } | ||
} | ||
|
||
impl Foo { | ||
fn foo(&self) { println!("foo::foo"); } | ||
} | ||
``` | ||
|
||
The method `bar` is now callable on any instance of `Foo`, | ||
regardless of whether the `Bar` trait is currently in scope, | ||
or even whether the `Bar` trait is publically visible. In other words if `Bar` | ||
is defined in one crate and `Foo` in another, the user of `Foo` will be | ||
able to explicitly depend only on the crate which defines `Foo` and still use | ||
the inherent trait's methods. | ||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
The `#[inherent]` attribute in the above example desugars equivalently to an inherent forwarding method (in addition to the trait impl): | ||
|
||
```rust | ||
impl Foo { | ||
#[inline] | ||
pub fn bar(&self) { <Self as Bar>::bar(self); } | ||
} | ||
``` | ||
|
||
Any questions regarding coherence, visibility or syntax can be resolved by | ||
comparing against this expansion, although the feature need not be implemented | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be explicitly specced out, especially since this expansion actually introduces an ambiguity in method calls. |
||
as an expansion within the compiler. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
- Increased complexity of the language. | ||
- Hides use of traits from users. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this works the way I think it does, this actually changes our trait stability story, and that's a major drawback. Currently, adding a defaulted method to a trait is not a major breaking change, at worst it will cause ambiguity errors, so it's considered minor. Now, a dependency can add a method to a trait you This should be explored and addressed in this RFC, and as a bare minimum should be called out in this section. (One "fix" is to only allow local traits) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another fix is to list out the method names that are inherent, although that becomes quite a burden for e.g. iterator methods. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure if the existing situation should be classified as so minor because you interact with many types almost exclusively through their traits. As an example, there are definitely traits with a As a fix, I'd suggest
And There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, but this breakage is easily fixed by using UFCS. This is not true for breakage caused in the world of The "minor" terminology comes from the API evolution RFC, such changes may cause crates to stop compiling, however:
Without this notion of "minor" being allowed, crates wouldn't be able to add anything new (types, traits, functions, or methods) without it being considered a breaking change. Furthermore, the API evolution RFC mentions in the trait method case that you should check to ensure the fallout isn't too great; which is where your In other words, when I use the term "minor" here, I'm using a precisely defined term from another RFC. Whether or not it is actually "minor" is irrelevant, I'm talking about what we do and don't consider breaking, which we have specced in terms of this major/minor categorization. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will it be possible to implement shadowing of inherent trait methods by true inherent methods? Compiler will warn on clashing inherent names while building crate which uses Though I think in practice such collisions should be extremely rare. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that would be the ideal fix here: an inherent trait's methods get shadowed by the type's own methods. They could produce a warning lint when compiling the crate itself, so that people notice, and then cap-lints will suppress that when compiling a dependency. |
||
|
||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Rationale and alternatives | ||
[alternatives]: #alternatives | ||
|
||
- Define inherent traits either on a type `T` or on an `impl T { .. }` block. | ||
- Implement as part of the delegation RFC. | ||
- Do nothing: users may choose to workaround the issue by manually performing the expansion if required. | ||
|
||
The most viable alternative is delegation proposal, although arguably inherent | ||
traits and delegation solve different problems with the similar end result. | ||
The former allows to use trait methods without importing traits and the latter | ||
to delegate methods to the selected field. Nethertheless delegation RFC and | ||
this RFC can be composable with each other: | ||
|
||
```Rust | ||
struct Foo1; | ||
struct Foo2; | ||
|
||
#[inherent] | ||
impl T1 for Foo1 { | ||
fn a() {} | ||
} | ||
|
||
#[inherent] | ||
impl T2 for Foo2 { | ||
fn b() {} | ||
} | ||
|
||
struct Bar { | ||
f1: Foo1, | ||
f2: Foo2, | ||
} | ||
|
||
impl Bar { | ||
// all methods from `T1` will be delegated as well | ||
// though `T1` will not be implemented for `Bar` | ||
delegate * to f1; | ||
} | ||
|
||
// method `b` will be accessable on `Bar` without importing `T2` | ||
#[inherent] | ||
impl T2 for Bar { | ||
delegate * to f2; | ||
} | ||
``` | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
- https://github.com/rust-lang/rfcs/pull/2309 (previous closed PR) | ||
- https://github.com/rust-lang/rfcs/issues/1880 | ||
- https://github.com/rust-lang/rfcs/issues/1971 | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
joshtriplett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Do we want to introduce a new keword instead of using `#[inherent]`? In other | ||
words do we want to write `inherent impl A for B { .. }`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fleshed out, including:
#[fundamental]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd think it should error if you have an inherent method with the same name, like if you had two inherent methods with the same name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can. One of the main use-cases for inherent trait implementations is implementation of crates defined in external crates.
I am not sure why it should not, though I can't say I fully understand
#[fundamental]
semantics.Initially I though it should error, but as I wrote below probably it may be better to issue a warning and shadow inherent trait methods by true inherent methods.
I will try to update the text based on your review! Thanks!