We've past proposals for inherent trait methods I think, mostly proposing a second inherent looking impl block, not sure that syntax matters, but this winds up functionally equivelent, yes?

There is however a question of default speed vs size optimizations in vtables, aka do these methods appear in the vtable, or do they have a generic implementation for every type? Also, how does one flag that these method should appear in a vtable, or should use a generic implementation for every type?

It might be better to have this as an attribute rather than yet another potential keyword position for parsing to deal with. Otherwise, great.

@Lokathor wrote:

It might be better to have this as an attribute rather than yet another potential keyword position for parsing to deal with. Otherwise, great.

I get that, but on the flip side, that'd be inconsistent with RFC 3323, and it seems awkward to use a keyword in one place and an attribute in another.

@burdges wrote:

We've past proposals for inherent trait methods I think, mostly proposing a second inherent looking impl block, not sure that syntax matters, but this winds up functionally equivelent, yes?

That might be equivalent to a subset of this, depending on the details. I haven't seen those proposals.

There is however a question of default speed vs size optimizations in vtables, aka do these methods appear in the vtable, or do they have a generic implementation for every type? Also, how does one flag that these method should appear in a vtable, or should use a generic implementation for every type?

In theory, if you have a method that can't be overridden outside of the crate/module, and nothing overrides it, you could optimize by omitting it from the vtable. I don't think that optimization should be mandatory, or required for initial implementation, though.

I'm more asking what's the best default. If the best default were to be in the vtable, then you can just do

trait Trait {
    #[inline(always)]
    impl(crate) fn f(&self) { f_inner(self) }
}
fn f_inner<T: Trait>(s: &T) { .. }

I'd expect f_inner gets only one copy for the trait obejct here.

If otoh the best default were not to be in the vtable, then rustc should do something more complex.

Anyways yeah maybe not relevant here.

Another future possibility could be to add impl(unsafe) trait methods, which can only be (re)implemented by unsafe impls.

@bluebear94 Interesting! So, rather than requiring unsafe impl Trait for Type, you could implement the trait safely, as long as you don't override the method? That's a novel idea.

this would be quite useful for stabilizing std::error::Error::type_id, which currently has several workarounds to prevent it from being overridden:
https://doc.rust-lang.org/1.80.1/src/core/error.rs.html#88-100
cc rust-lang/rust#60784

@programmerjake That's a great use case, thank you!

This seems like a reasonable and desirable extension to RFC 3323. So I propose...

@rfcbot fcp merge

Thanks @joshtriplett for writing this up.

I note that syntax was left as an open item on RFC 3323, and so we're implicitly carrying that open item here also.

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• visiblity-style-vs-final-style (Trait method impl restrictions (final methods) #3678 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

Big +1 to having something like this.

@rfcbot concern visiblity-style-vs-final-style

I think these are far more than a syntax difference, so I want to talk about it more.

As a couple of examples:

• With #[final] we can allow it in #[marker] traits, but with impl(in self) we can't, because it could still be overridden in the module.
• With #[final] we can have the MIR inliner inline the provided method into other provided methods or into generic code, but with impl(in self) we can't because it might be overridden somewhere.
• With #[final] unsafe code can trust the implementation of the method, but arguably if it's overridable at all by anyone then to trust the implementation of the method it ought to be an unsafe trait.

So is the visibility phrasing here actually important? Do we actually need it? The cases I know of don't need it

• the RFC mentions Error::type_id, which doesn't need it.
• I'd like to add a Copy::copy method, which doesn't need it (and would really benefit from being trivially inlinable and such)
• things like https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/visit/trait.MutVisitor.html don't need overriding the super_* methods to be impossible, but I think would be perfectly happy to just mark them #[final], and as far as I know has no need for impl(crate) or similar.

The provided implementation of a #[final] can always call another trait if needed, so I think that anyone who really needs such a thing could do it that way. And having the stronger rule gives us a bunch of advantages. And if we're going to give it semantic capabilities beyond just a visibility-like error, I don't think it should look like visibility -- my code should never fail to compile because I changed something from impl(self) to impl(crate) for example! (Going to pub might make things unsound, but doesn't make it stop compiling -- other than name resolution glob conflicts or something.)

Thus my inclination is that we should do the #[final] version of it instead of the impl (in …) version.

I think #[final] is easier to explain too. As usages of the impl and use keywords multiply, we quickly lose our ability to explain them.

If it is to be an attribute then I would prefer something like #[no_override] that says what it does, rather than taking a C++ keyword that imo isn't very descriptive. But +1 to this suggestion - I think it is immediately obvious what the effects are, and either being on or off is easier to follow than giving fine grained control. Agreeing with Scott, being able to e.g. override something in the module but not the rest of the crate doesn't seem like a common enough need to justify the complexity.

I assume the syntax was chosen for parity with RFC3323, but I think it is okay to deviate from this if it comes with a simplification.

• With #[final] we can allow it in #[marker] traits, but with impl(in self) we can't, because it could still be overridden in the module.
• With #[final] we can have the MIR inliner inline the provided method into other provided methods or into generic code, but with impl(in self) we can't because it might be overridden somewhere.
• With #[final] unsafe code can trust the implementation of the method, but arguably if it's overridable at all by anyone then to trust the implementation of the method it ought to be an unsafe trait.

Note that the RFC allows for impl(), which would express the same thing as #[final]. From the RFC:

In addition to impl(visibility), a trait method or associated function can also use impl(), which does not permit overriding the method anywhere, even in the current module. In this case, all implementations will always use the default body.

This is something I specifically checked to ensure was there before proposing FCP merge, in part for the reasons you mention.

Note that the RFC allows for impl(), which would express the same thing as #[final].

I think that that just pushes me even more to skip the impl(in blah) part of the feature, because if what I want is already an unusual case that doesn't work with the 3323-style syntax, that says to me we should just skip that style syntax entirely.

If we need a special syntax, let's make it final or #[final]. That way we can keep the "this is only about visibility, not runtime semantics" property of pub(…) and impl(…). If we're not adding pub() fn foo() {} and struct Foo { mut() a: i32 }, then I think any superficial similarity here is more harmful than good. We can keep impl(…) on provided methods if people really want it -- though it would be good to have at least a single concrete example scenario in the RFC -- but I'm opposed to mixing the visibility restrictions with non-visibility ones.

(And, aesthetically, impl() looks weird to me.)

rust-lang/rust#150101

Updated based on @Amanieu's comments.

@rfcbot resolve amanieu-dyn-compatibility

@rfcbot resolve amanieu-dyn-compatibility

🔔 This is now entering its final comment period, as per the review above. 🔔

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

Huzzah! The @rust-lang/lang team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/rust#131179

The next step is an implementation. There was a previous draft at rust-lang/rust#130802 , which may or may not be useful as a starting point for someone looking to work on an implementation. (It does not straightforwardly rebase; the compiler has changed since that PR.) I'd be happy to work with someone looking to implement this to provide any assistance needed.