rfcs
rfcs copied to clipboard
rust-lang
RFC: inherent trait implementation
Continuation of #2309.
Fixes: #1880,#1971
This RFC allows us to write the following code:
#[inherent]
impl Bar for Foo {
// code
}
Which allows methods from Bar trait to be used on Foo instances without having Bar in the scope.
cc @nikomatsakis, @aturon
This RFC assumes that rust-lang/rust#48444 will be treated as a feature and not as a bug.
To clarify: #2309 was postponed until we figure out the story for delegation. While you talk about it a bit in this new RFC, it's not clear what in this proposal changes the situation compared to when the lang team previous took up this question. Can you expand on your thinking here?
I was under impression that the previous RFC was closed due to the lack of reaction from @Diggsey to add requested changes, and not postponed. (ctrl+f "postpone" yields zero results)
While I agree that delegation RFC and inherent traits should be discussed together, in my opinion RFCs have orthogonal scopes and similar only in the end effect. As was shown in the text, delegation can be nicely composed with #[inhrent] attribute and there is no need to overload delegation with an additional functionality.
Yes, the new delegation RFC draft mentions inherent trait impls as a possible future extension, but I don't think that using delegate is a correct approach here. Also in my opinion inherent trait implementations can have a much bigger impact on the ecosystem (including stdlib and core) than the delegation RFC, thus it should consider as one of the main topics and not as a vague future extension.
@newpavlov I don't think that's an accurate assessment. The RFC was closed because:
We discussed this in the lang team meeting today, and everyone agreed it would be nice to provide "inherent trait methods" of some sort. However, we also felt that this feature would probably fall out naturally from a solution to the more general problem of trait and method delegation. We'd like to see another RFC for delegation before we accept a feature like this.
ie. the lang team wanted a more general solution
@Diggsey Yeah, my mistake. Though by reading discussion of the new version of delegation RFC it looks like this use case is not going to "fall out naturally" any time soon.
@aturon So should I close this RFC until lang-team decides how inherent traits and delegation will interact with each other or should I leave it to be for now?
@newpavlov Sorry for the delay, was out on vacation.
Yes, I think it should probably be closed for the time being, especially since there's now a delegation RFC. Would be good to revisit after Rust 2018 ships!
How are polymorphic traits handled? I'd presume
#[inherent]
impl<'de> Deserialize<'de> for MyType { .. }
works roughly like
impl MyType {
fn deserialize<'de,D>(deserializer: D) -> Result<Self, D::Error>
where D: Deserializer<'de> { ... }
}
Yet, how should separate impl blocks be handled?
#[inherent]
impl Borrow<str> for MyType { }
#[inherent]
impl Borrow<[u8]> for MyType { }
We'd likely want roughly
impl MyType {
fn borrow<R>(&self) -> R
where Self: Borrow<R>
{ self.borrow() }
}
@burdges I think that initial implementation should simply forbid inherent implementations for polymorphic traits. Later we could do an extension, though I don't see how it can be done nicely right now.
is there a reason (that I'm not seeing) that this cannot be done with a macro that just implements the wrapping functions in a generated impl block?
It can be done, but will result in code and documentation bloat. Also use of procedural macros will increase compilation times.
fn aliases would provide a nice solution here too. I suppose fn aliases should wait until delegation gets sorted out, although argument for waiting is less compelling than for this RFC's proposal.
Does this need to be an RFC? As far as I'm aware, it could be implemented using a normal procedural macro and uploaded to crates.io. That also gives us a place to iterate on the implementation before anything is merged.
Kinda like how we used the work from failure to improve std::error::Error's API.
Does this need to be an RFC? As far as I'm aware, it could be implemented using a normal procedural macro and uploaded to crates.io. That also gives us a place to iterate on the implementation before anything is merged.
This already exists as a procedural macro: inherent
is there a reason (that I'm not seeing) that this cannot be done with a macro that just implements the wrapping functions in a generated impl block?
Does this need to be an RFC? As far as I'm aware, it could be implemented using a normal procedural macro and uploaded to crates.io.
This cannot be implemented with a procedural macro alone. While the simple case provided by the inherent crate (linked by @jplatte) does work when only a single item with a unique name exists, it fails when multiple exist. For example, if a struct's impl has a const foo, and an #[inherent] trait has fn foo, compilation fails due to duplicate definitions.
The main issue to me with a macro is it duplicates documentation. I'd rather rustdoc handle these by inlining documentation with a clear back-reference to the trait. A attribute would allow rustdoc to do that.
I'd rather rustdoc handle these by inlining documentation with a clear back-reference to the trait.
Couldn't the attribute macro also do that? It just moves the #[doc] attributes to the inherent impl then appends /// See also [MyTrait::foo()] at the end to do the back-link.
The things that would need compiler support are when you have information outside the impl Trait for Foo { ... } block (like conflicting methods) because macros can only know about the token trees it has been passed.
This RFC as-written needs some amendments to clarify the intended behavior in the case of overlap (trait methods with the same names as impl methods) in order to resolve the concern @Manishearth raised above: how can we ensure that adding methods to a trait remains a backwards-compatible change?
This RFC came up in the libs team meeting yesterday when we were talking about https://github.com/rust-lang/rust/pull/82786, which we want to solve via inherent traits to avoid producing many "unused import" warnings, which presumably won't be output for imports on pre-existing trait impls that are made inherent. We additionally decided it would be nice to have an allow by default lint to still warn about these unnecessary imports, and that this lint should probably also warn about unnecessarily included prelude items. I found an existing clippy issue about adding a lint for warning of unnecessary prelude item imports and added additional context about also warning about unneeded inherent trait imports, https://github.com/rust-lang/rust-clippy/issues/1124.
We discussed this in the most recent @rust-lang/lang triage meeting, and there was broad agreement that we'd like to merge this RFC or something similar to it. However, this RFC needs changes to answer the unresolved questions, most notably how to resolve the ambiguity that occurs when trait methods are added that overlap with existing inherent methods (the team felt generally in favor of resolving the ambiguity in favor of inherent impl methods, and allowing overlap).
@newpavlov Are you able to amend the RFC to address these questions? If not, the language team would be happy to mentor someone to send a patch or open a new, amended RFC.
@cramertj I should be able to work on this RFC next week, although I likely will need some mentorship to resolve the open questions. Whom should I contact about it?
@newpavlov feel free to open a thread on the lang team's zulip channel (https://rust-lang.zulipchat.com/login/) and cc myself and other members of the language design team!
How would the #[inherent] attribute handle argument names to the "inherent-ized" method on the target type? Looking at the approaches used with proc macros (including the inherent crate), the argument names get converted to generic arg1, arg2, etc names. Hoping that this could preserve the original names!
One thing I'd like to see here is more about alternatives for different places it could be other than on the impl. That's certainly the place it has to go when it's done in a proc macro, but I think there's more options when there's direct language support for it.
Particularly if this is adding another layer of name resolution -- as I think is implied by the overlap conversation -- then "attaching" it to the definition of the type somehow might make sense.
Another thought: I'd like to see something in the Guide section about when I should, and importantly when I shouldn't, mark a particular trait impl as #[inherent]. Certainly if it's for an external type I can't, but if it's for my own type, why wouldn't I want to let people call things without needing a trait import?
An concept that occurred to me: Can this be used with a private trait?
This could offer an interesting alternative to macros and sealed traits, by defining methods on a private trait marked inherent to allow them to be called by others.
Though that adds another question: the desugar in the reference section says that this is always pub. Is there a way to have #[inherent(in self)] to allow this inside a crate only?
The interaction with privacy, @scottmcm, is a good callout. I would expect, I think, that the inherent methods get the privacy defined on the trait, though that's certainly not the only option.
(Not for the first time, I rather wish trait methods had privacy of their own.)
Following up on this: I went through previous feedback, resolved a couple of threads, and commented in others. I think this is now ready to update to incorporate the remaining feedback.
