Refined trait implementations

[tmandry]

trafficstars

This RFC generalizes the safe_unsafe_trait_methods RFC, allowing implementations of traits to add type information about the API of their methods and constants which then become part of the API for that type. Specifically, lifetimes and where clauses are allowed to extend beyond what the trait provides.

Rendered RFC

This RFC was authored as part of the async fundamentals initiative.

[nikomatsakis]

I'm nominating this for @rust-lang/lang attention -- this is a principle that we have frequently enunciated internally, so I encouraged @tmandry to make an RFC for it. We don't necessarily need to rush to approve it, but I wanted folks to see it and give it a read.

[clarfonthey]

So, two big questions for this:

1. How does this impact dynamic dispatching? Assuming unsized types become eventually allowed in return position, does this mean that returning impl Trait is fine because the dynamic version will instead return dyn Trait as its impl Trait value?
2. If we can reconcile the above, would it make sense to modify the std Iterator trait to allow return value refinement? This could allow lots of cool things like fuse being a genuine no-op for certain types, but could allow people to genuinely break the behaviour of some adapters.

[tmandry]

I'm going to answer the above question assuming we have return-position impl Trait in traits and support dynamic dispatch on those traits, but keep in mind that neither of those features have been through the RFC process yet.

So, two big questions for this:

1. How does this impact dynamic dispatching? Assuming unsized types become eventually allowed in return position, does this mean that returning impl Trait is fine because the dynamic version will instead return dyn Trait as its impl Trait value?

Dynamic dispatch of return-position impl Trait wouldn't be affected by this proposal. It's like you said. More concretely, if you have something like this:

impl Foo {
  fn stuff(&self) -> impl Bar;
}

then the compiler can generate an impl that looks something like this:

impl Foo for dyn Foo {
  fn stuff(&self) -> impl Bar;
}

and that impl is free to choose whatever concrete type it wants for the return type of stuff; in this case, it would be something with dyn Bar in it.

2. If we can reconcile the above, would it make sense to modify the std Iterator trait to allow return value refinement? This could allow lots of cool things like fuse being a genuine no-op for certain types, but could allow people to genuinely break the behaviour of some adapters.

~It should be possible to modify the fuse method to return just impl Iterator instead.~ EDIT: This would break existing code, also see https://github.com/rust-lang/rfcs/pull/3245#issuecomment-1088142178.

This doesn't currently interact with dynamic dispatch because of the where Self: Sized clause on fuse. In the future we may want to relax that bound, but that is out of the scope of this RFC.

[clarfonthey]

This doesn't currently interact with dynamic dispatch because of the where Self: Sized clause on fuse. In the future we may want to relax that bound, but that is out of the scope of this RFC.

Oh right, I completely forgot about the where Self: Sized bound. 🤦🏻

In that case, I would be very interested in seeing impl Trait return types for Iterator, but another issue came to mind: there are two main reasons for impl Trait here, and only one of them can really be accounted for in the case of iterator adapters.

The first case would be to allow trait implementations to provide their own types. For example, it would be nice to let iterators return Self from fuse when they know that fusing is a no-op.

However, the second case is to explicitly remove types from the implementation so that they aren't exposed to users of the API. And with the current proposal, the second case is always true for the default implementations; you can't choose to expose the type of an impl Trait return type and let end users provide an implementation they want; it will always be so. I'm not sure how to solve this problem, but I think it's a worthwhile one to think about for this proposal, as I can't immediately tell how to fix this problem.

Note that the same is true for all forms of refinement, not just the defaults -- for example, if you want to not require const for end users but to provide a const implementation for the default, you can't do that, at least from what I understand.

[tmandry]

I don't think having provided methods with a refined API is a good idea. That would mean you could break existing code that uses your impl by overriding a provided method to return a different type. But that's antithetical to the idea of this RFC, which is to let impls add to existing guarantees, not take them away. So the provided method API should always match the trait, in my opinion.

We could allow this but say it's a breaking change to refine a method on an existing impl in a way that doesn't agree with the provided method API. This would mean that new impls are allowed to override it, as well as impls that don't care about semver guarantees. But this violates the principle of least surprise – users expect the API of a method provided by a trait to match the API of the trait. It would complicate the mental model of the feature in addition to introducing a new semver hazard for library authors.

So, on second thought I'll have to revise my earlier statement. I don't think we can backward-compatibly change fuse to return impl Iterator. There might be another way to solve for this use case, like "struct specialization" (which actually seems quite plausible to me), but I don't think the feature in this RFC is what you need. This RFC is about making it possible to add API guarantees to impls, not to relax them in traits or to optimize special cases.

[clarfonthey]

I completely didn't consider the effect my proposal had on breaking changes and now understand completely why it would not work that way. It really does seem like the type-alias-impl-trait feature would be required in these sorts of situations, but that will probably be finished before this one anyway.

[nikomatsakis]

We discussed this in the @rust-lang/lang meeting today -- we were thinking, @tmandry, that it would be better to make the #[refine] attribute mandatory for the refined signature to be "visible" outside of the impl.

I would personally be in favor of having a lint warning if the signature in the impl diverges from the trait and there is no #[refine] attribute, because it seems like something one would want to resolve one way or the other (or mark as #[allow] with a comment).

This rule would also amend the existing unsafe RFC to require #[refine].

[tmandry]

I think using #[refine] is reasonable to prevent accidental stabilization of API surface. However, we should consider that not all Rust code cares about this. This may be a private API, and I can imagine it getting quite noisy to write #[refine] everywhere for an API that will never be published.

So if we go this direction, I think we should consider tweaking the formulation slightly. In the current edition it would work just as you said. In future editions the refined signature should always be available, but there is a deny-by-default or warn-by-default lint in cases where there is no #[refine] attribute and the signature does not exactly match. That way code that doesn't care about semver breakage can do a blanket #![allow] for their crate or module.

[nikomatsakis]

@rfcbot fcp merge

I think we've got consensus on most of this here -- the main question is whether, in the next edition, #[refine] ought to be required in order to see the effects of the change or optional (perhaps with a lint). I'm going to separate propose a concern to move this to an unresolved question, I dont' see why it should block this RFC from going forward now.

[nikomatsakis]

@rfcbot concern unresolved-question-for-next-edition

Let's add an official "unresovled question" about the behavior in the next edition. I think the options to choose between are:

• no edition dependent behavior: #[refine] is required in Rust 2024 to have an impl commit to a refined interface
• refine recommend in Rust 2024: in Rust 2024, refined impls are always visible to clients, but we warn if the interface diverges from the trait and there is no #[refine] attribute to document that fact
• refine not required in Rust 2024: #[refine] is not required and not recommended in Rust 2024, we just present a refined interface always

[djkoloski]

I think this RFC may exacerbate existing problems with name resolution.

If a refined impl is semantically equivalent to an inherent impl with the refined signature plus a trait impl that calls into the refined impl. With those semantics, every refined impl runs into the same unintuitive name resolution that inherent functions have but with the added problem that there's no way to explicitly call the trait method with the original signature. It all depends on the context in which the method is called, which means that the "correct" form can change just by moving code from place to place. I think that refined impls are a good idea, but that we should tackle this name resolution problem head-on before introducing language features that provide more opportunities to cause this kind of confusion.

For example: in the case where a user calls an unsafe trait method that has been refined to be safe, would that trigger an unnecessary unsafe block lint? Here's an example of this situation right now using a name collision between an inherent impl and a trait method:

trait Print {
    unsafe fn print(&self);
}

struct Foo;

impl Foo {
    fn print(&self) {
        println!("Inherent");
    }
}

impl Print for Foo {
    unsafe fn print(&self) {
        println!("Trait");
    }
}

fn main() {
    let x = Foo;
    unsafe {
        x.print();
    }

    fn foo<T: Print>(x: T) {
        unsafe {
            x.print();
        }
    }

    foo(x);
}

This triggers a lint on the first x.print() saying that the unsafe block is unnecessary (because it's calling the inherent method). However, moving that block into foo calls the trait impl instead, and so does not trigger the lint. The way to disambiguate this right now is to use fully qualified syntax:

Foo::print(&x);
// or
<Foo as Print>::print(&x);

And to be clear, I think that x.print() should be a lint by default. However, with refined impls it becomes impossible to disambiguate between calling the refined impl and the unrefined impl because the fully qualified syntax of the refined impl is the same as the fully qualified syntax as the original method. I would argue that this violates the expectation that many rust programmers have that moving code from a concrete context (where we know the exact types of variables) to a generic context (where these types are parameters) will not require changing the code. This is one of the properties that makes it easy to refactor rust code.

It's worth noting that the community generally avoids naming collisions between inherent impls and trait methods for this exact reason. If the intention is to encourage users to use impl refinement when possible, then I think we should at least give users a way to refer to the unrefined method in concrete contexts. However I think that defaulting to the refined signature in these contexts makes concrete rust code difficult to refactor by default.

[tmandry]

If a refined impl is semantically equivalent to an inherent impl with the refined signature plus a trait impl that calls into the refined impl.

I wouldn't say it's semantically equivalent. It's really even stronger than that: The trait method itself has the refined signature when you know the type of the receiver. That's why the RFC says that you will get the refined signature even when using UFCS.

For example: in the case where a user calls an unsafe trait method that has been refined to be safe, would that trigger an unnecessary unsafe block lint?

Good question. I think either behavior could be reasonable: the lint could fire, or the lint could have an exception for trait methods that are marked unsafe. To some extent it's an empirical question what's more useful vs noise in the lint.

We could also have a "sub-lint" for this exact situation so people could customize the behavior themselves, but I'm not sure it's worth it.

I would argue that this violates the expectation that many rust programmers have that moving code from a concrete context (where we know the exact types of variables) to a generic context (where these types are parameters) will not require changing the code. This is one of the properties that makes it easy to refactor rust code.

I don't have that expectation: when moving from a non-generic to a generic context you are always losing some information (indeed that is the point), which can require modifying your code a bit. For example, we already have this problem with associated types today. Concrete code gets to assume associated type values, while generic code only gets the bounds of the type on the trait itself (plus bounds from any where clauses on your function).

I'd also like to make it possible to bound method return values in the same way you can bound associated types, which would recover some of the ability to "just add a where clause" and make the problem go away. There is even a version of that idea that allows you to bound the argument types that are accepted. But I think all this is out of scope for this particular RFC.

Maybe it's worth digging in more to some example scenarios that this proposal would make harder. My sense is that the benefits of the feature are overall worth it. But it sounds like you might have experience with situations that give you a different impression.

It's worth noting that the community generally avoids naming collisions between inherent impls and trait methods for this exact reason.

I can think of another strong reason to avoid this, which is that shadowing makes it unclear which implementation is actually being called. That's not the case in this proposal.

If the intention is to encourage users to use impl refinement when possible, then I think we should at least give users a way to refer to the unrefined method in concrete contexts.

Perhaps we should, but I can't think of a scenario where people would actually use this. UFCS is generally only there to reach for when you need it. Would users proactively write their code in a less convenient fashion just so they can avoid changing it if they were to make it generic later? I think not. It seems more reasonable to write your code using impl Trait from the get-go. I can understand if there are ergonomic or cognitive overhead costs to doing so that would steer a user away from that, but that's a tradeoff the user can make.

Also, we were discussing this some yesterday and realized you can implement this "ultra-UFCS" in a macro, so if people actually want it they could do that.

Finally, I wouldn't say the intention is to encourage users to use impl refinement whenever possible. It's a feature that is there if you need it. Refining an impl creates a stronger contract than the one in the trait, which entails a tradeoff that implementers can choose to make or not.

However I think that defaulting to the refined signature in these contexts makes concrete rust code difficult to refactor by default.

What alternative would you propose? I agree it has this tradeoff, but that seems lower cost than requiring that users use a special syntax to invoke a refined API.

I don't anticipate that this feature will get used everywhere. In fact with the version that @nikomatsakis mentioned, implementers would always have to opt in by adding the #[refine] attribute on their method. So I think the danger of tripping over this each time you convert concrete code to generic is not that high.

[djkoloski]

Here's one example where I think refined return types cause confusion. Starting from a relatively egregious example that has nothing to do with trait refinement:

Click to expand

trait Iterator {
    fn next(&mut self) -> bool;

    fn len(&mut self) -> usize {
        println!("Slow");

        let mut len = 0;
        while self.next() {
            len += 1;
        }
        len
    }
}

trait LenIterator: Iterator {
    fn len(&self) -> usize;
}

#[derive(Clone)]
struct BasicIterator(usize);

impl Iterator for BasicIterator {
    fn next(&mut self) -> bool {
        if self.0 == 0 {
            false
        } else {
            self.0 -= 1;
            true
        }
    }
}

impl LenIterator for BasicIterator {
    fn len(&self) -> usize {
        println!("Fast");

        self.0
    }
}

trait IntoIterator {
    type Iterator: Iterator;
    fn into_iter(self) -> Self::Iterator;
}

impl IntoIterator for usize {
    type Iterator = BasicIterator;
    fn into_iter(self) -> Self::Iterator {
        BasicIterator(self)
    }
}

These five calls to len in main will print, respectively:

1. Fast
2. Slow
3. Fast
4. Slow
5. Fast

fn call_len<T: Iterator>(mut iter: T) {
    iter.next();
    iter.len();
}

fn call_len_2<T: LenIterator>(mut iter: T) {
    iter.next();
    iter.len();
}

fn main() {
    10.into_iter().len();
    Iterator::len(&mut 10.into_iter());
    LenIterator::len(&mut 10.into_iter());
    call_len(10.into_iter());
    call_len_2(10.into_iter());
}

This is very similar to the existing count and size_hint functions on Iterator, so I don't think it's unreasonable to expect.

This demonstrates how unintuitive name resolution can already be. In this case, I'd argue that it's unintuitive but manageable. However, with the introduction of refined return types and RPITIT we can cause additional problems by changing IntoIterator:

trait IntoIterator {
    fn into_iter(self) -> impl Iterator;
}

impl IntoIterator for usize {
    fn into_iter(self) -> impl Iterator;
}

impl IntoIterator for isize {
    fn into_iter(self) -> impl Iterator + LenIterator;
}

We can end up in situations where the IntoIterator trait looks like it will always return an impl Iterator and so always call Iterator::len. But in reality, calling into_iter on some types will return an iterator that calls LenIterator::len! This is similar to the problem we already have with associated types, but with associated types we are aware that the iterator has a concrete type and so name resolution can be odd. impl Trait muddies these waters significantly because the trait promises that the return type will be opaque and only implement Iterator, yet impls can elect to implement additional traits and cause name resolution issues.

Obviously RPITIT is not part of this RFC, but it's explicitly mentioned in the text so I feel that it's worth addressing.

---

Refined trait implementations may also lead to somewhat perverse incentives when implementors write trait impls. For example:

trait Invert {
    /// # Safety
    ///
    /// All of the items in the container must sum to 1.
    unsafe fn invert(&mut self);
}

impl Invert for [f32] {
    fn invert(&mut self) {
        // We want to be able to call invert safely, so we don't rely on the precondition
        let total = self.iter().sum::<f32>();
        for i in self.iter_mut() {
            *i = total / *i;
        }
    }
}

Let's assume that the user is implementing some foreign trait and can't change its signature. That trait may have some unsafe methods with some preconditions. Taking advantage of those preconditions would result in a faster implementation at the cost of being unsafe. However, the user can also write a totally safe implementation that doesn't take advantage of any preconditions. In exchange for performance, they can call their code safely and avoid writing any unsafe code. I think it's fair to say that many users would choose those tradeoffs when writing their own code.

However, this subverts the entire purpose of the trait - there's not much point in guaranteeing preconditions if implementors stand to benefit from ignoring them. I would argue that not allowing users to call unsafe methods as safe prevents this kind of perverse incentive. In this particular case, it's typically best to implement the trait unsafely and provide an inherent impl that checks preconditions and delegates to the trait.

I also don't see any situations in the motivation that aren't arguably better solved by adding an inherent impl with a more permissive signature. In fact, a proc macro could probably achieve most of these goals without any new language features since inherent impls are still accessible and prioritized when the type is known concretely. Are there any cases where an inherent impl with the more permissive signature is less useful?

[nikomatsakis]

@rfcbot fcp merge

I think we've got consensus on most of this here -- the main question is whether, in the next edition, #[refine] ought to be required in order to see the effects of the change or optional (perhaps with a lint). I'm going to separate propose a concern to move this to an unresolved question, I dont' see why it should block this RFC from going forward now.

[rfcbot]

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

• [x] @cramertj
• [ ] @joshtriplett
• [x] @nikomatsakis
• [x] @pnkfelix
• [ ] @scottmcm

Concerns:

• ~~unresolved-question-for-next-edition~~ resolved by https://github.com/rust-lang/rfcs/pull/3245#issuecomment-1155485530

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!

See this document for info about what commands tagged team members can give me.

[nikomatsakis]

@rfcbot concern unresolved-question-for-next-edition

Let's add an official "unresovled question" about the behavior in the next edition. I think the options to choose between are:

• no edition dependent behavior: #[refine] is required in Rust 2024 to have an impl commit to a refined interface
• refine recommend in Rust 2024: in Rust 2024, refined impls are always visible to clients, but we warn if the interface diverges from the trait and there is no #[refine] attribute to document that fact
• refine not required in Rust 2024: #[refine] is not required and not recommended in Rust 2024, we just present a refined interface always

[nikomatsakis]

(those commands were posted earlier, but since there was no T-lang label, rfcbot ignored me)

[QuineDot]

I wanted to note a scenario in Rust today where a programmer must write a refined implementation without the expectation it can be called, or perhaps even the expectation it cannot be called. That is, a scenario wherein programmers must write an implementation they do not want exposed in their APIs.

The scenario is that mentioned in #2829: trait items must be implemented even if their bounds cannot be met. Say we have

    pub trait Example {
        type Item;
        fn f(&self) -> Self::Item where Self::Item: Copy;
    }

And we attempt:

    impl Example for String {
        type Item = Self;
    }

It errors; we must supply the method. So we add:

        fn f(&self) -> Self::Item where Self::Item: Copy {
            String::new()
        }

But this does not compile either. However if we remove the bound:

        fn f(&self) -> Self::Item {
            String::new()
        }

Then everything compiles, but you can rest assured the method can't be called. (Maybe your dummy method does something undesirable, or maybe those bounds were crucial for some other reason.)

---

Allowing unsatisfiably bounded items to be omitted is not required to improve the situation (although I would personally love to see that). If the trivial bounds RFC was stabilized, you wouldn't need the refined method in order to compile. (Probably you would get a warning, which is fine; I'm not sure a trivial bounds lint exists yet.)

This does imply that unsatisfiable bounds on implementations must be respected (i.e. the implementation in the last link should continue to error as the "looks refined but isn't" version on stable does today).

[tmandry]

I've updated the RFC with options for how to handle #[refine] in the "Unresolved Questions" section.

[tmandry]

From https://github.com/rust-lang/rfcs/pull/3245#issuecomment-1105959958:

This demonstrates how unintuitive name resolution can already be. In this case, I'd argue that it's unintuitive but manageable. However, with the introduction of refined return types and RPITIT we can cause additional problems by changing IntoIterator:

Yes, it is surprising. I found the behavior documented in the reference, which is an improvement from when this was only documented in compiler source code :). The example you gave specifically depends on the fact that we have a &mut self method in one trait and a &self method elsewhere (which is given priority). Otherwise we would have an ambiguity error and need UFCS to disambiguate. To me the current behavior seems arbitrary and suboptimal; it could be improved in a future edition by requiring UFCS in more cases like this one.

The need for a trait like LenIterator would be removed if we extended this proposal to allow &mut self methods to be refined as &self methods. There would be no ambiguity in that case, since you would be overriding the provided method and there would only be one method to call.

Even without this feature I think it's a bad idea to define a method with the same name as another method on another common trait (especially on a supertrait). That would be confusing for readers in any case. So I think you want to avoid situations like this anyway where possible.

To the extent that this feature interacts negatively with the already-confusing method dispatch rules, I think we should focus on improving those rules.

This is similar to the problem we already have with associated types, but with associated types we are aware that the iterator has a concrete type and so name resolution can be odd. impl Trait muddies these waters significantly because the trait promises that the return type will be opaque and only implement Iterator, yet impls can elect to implement additional traits and cause name resolution issues.

Under this proposal the statement "the trait promises that the return type will be opaque and only implement Iterator" (emphasis mine) is not true. Arguably it was never true. Intuitively, traits promise a subset of what a concrete type can do. Even opaque impl Trait types are only a subset since we leak auto traits. The idea that we are promising a negative bound on everything else doesn't doesn't ring true to me.

You cite associated types – this proposal makes associated functions and consts work more like associated types. In all cases you get more information if you know the concrete type. If all you know is the trait, then you get only what the trait says. I don't really see how this is inherently confusing or problematic.

[tmandry]

From https://github.com/rust-lang/rfcs/pull/3245#issuecomment-1105959958:

I think it's fair to say that many users would choose those tradeoffs when writing their own code.

However, this subverts the entire purpose of the trait - there's not much point in guaranteeing preconditions if implementors stand to benefit from ignoring them. I would argue that not allowing users to call unsafe methods as safe prevents this kind of perverse incentive. In this particular case, it's typically best to implement the trait unsafely and provide an inherent impl that checks preconditions and delegates to the trait.

How is allowing a user to make this tradeoff a perverse incentive? Just because a method can be implemented safely does not necessarily make it slower. In those cases where it's not you always want to call the same implementation, and making it safe means you have fewer unsafe blocks to audit in your code. That alone makes the feature worth it in my book.

In the cases where you have a safe but slower implementation, yes, using an inherent method with a different name might be best. But if a user knows Rust well enough to consider writing unsafe then they should also know it well enough to understand the implications on generic code and weigh this tradeoff for themselves.

...but anyway, now we are arguing about RFC #2316 which was already accepted!

[tmandry]

Thanks to @djkoloski and @QuineDot for your feedback. I've updated the RFC with new drawbacks section, Refactoring, and a corresponding future possibilities section, Bounding refined items. I think these are worth considering when evaluating the RFC. I also added interactions with Method dispatch and Unsatisfiable trait members.

Since I already added the unresolved question on #[refine], I think the current concern can be resolved. But we may want a new concern added so that lang team members have time to read the new sections.

[nikomatsakis]

@rfcbot resolve unresolved-question-for-next-edition

[rfcbot]

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

[rfcbot]

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.

This will be merged soon.

[joshtriplett]

@tmandry Want to file a tracking issue, and add that to the RFC?

[steffahn]

The RFC says it “generalizes the safe_unsafe_trait_methods RFC”, then describes the status quo as “This allows code that knows it is calling a safe implementation of an unsafe trait method to do so without using an unsafe block. In other words, this works today” with a code example without a #[refine] attribute. Yet, later, it describes (reference level explanation) that “Refinements of trait items that do not match the API of the trait exactly must be accompanied by a #[refine] attribute on the item in Rust 2021 and older editions”. And @nikomatsakis suggested above that “This rule would also amend the existing unsafe RFC to require #[refine].”

My point being: The RFC as written right now is awefully ambiguous about whether or not #[refine] is required for safe implementations of unsafe trait-methods. I would assume that they are required, but this should be more explicit, especially also being explicit about the point that the safe_unsafe_trait_methods RFC is changed in this regard. Also, the comment “In other words, this works today” is inaccurate, since the safe_unsafe_trait_methods RFC is not stably implemented yet (and that’s important so it can be changed in the first place).

[clarfonthey]

Would it make more sense to simply retcon rust-lang/rust#87919 as being the tracking issue for this RFC? Since this is technically just a broader version of RFC #2316.

[tmandry]

@joshtriplett Tracking issue has been opened as https://github.com/rust-lang/rust/issues/100706.

@steffahn Thanks, I clarified in the text that #[refine] will be required for safe impls of unsafe methods.

I also removed the unresolved question around whether we need a soft transition from the current behavior. With it being decided to use #[refine] in the current edition, the question isn't relevant. It also doesn't seem realistic to expect that we can get away without some kind of edition migration.

@clarfonthey Sorry, race condition!