Pre-RFC: Supertrait item shadowing

Hello,

I've been thinking about a change to the language to avoid a case of potential downstream breakage with supertraits. This is an especially unexpected case for the user, because the breakage is introduced indirectly by a transitive dependency.

I drafted this as an RFC, not a pre-RFC, so it's a bit long and might read a bit formal at times, but I hope that's not an issue. I'd love to hear your feedback on it.


Summary

Change item resolution for generics and trait objects so that a trait bound does not bring its supertraits' items into scope if the subtrait defines an item with this name itself.

Motivation

Consider the following situation:

mod traits {
    trait Super {
        fn foo(&self);
    }

    trait Sub: Super {
        fn foo(&self);
    }
}

A trait Sub with a supertrait Super defines a method with the same name as one in Super.

If Sub is used as a generic bound, or as a trait object, trying to use the foo method raises an error:

Generics:

use traits::Sub;

fn generic_fn<S: Sub>(x: S) {
    x.foo();
}

Trait objects:

use traits::Sub;

fn use_trait_obj(x: Box<dyn Sub>) {
    x.foo();
}

Both of these currently raise the following error:

error[E0034]: multiple applicable items in scope
  --> src\main.rs:10:4
   |
10 |     x.foo();
   |       ^^^ multiple `foo` found
   |
note: candidate #1 is defined in the trait `traits::Super`
  --> src\main.rs:2:2
   |
2  |     fn foo(&self);
   |     ^^^^^^^^^^^^^^
   = help: to disambiguate the method call, write `traits::Super::foo(x)` instead
note: candidate #2 is defined in the trait `traits::Sub`

Note that the trait bound is always Sub, Super is not mentioned in the user code that errors. The items of Super are only in scope because the bound on Sub brought them into scope.

As the diagnostic mentions, Universal function call syntax (UFCS) will work to resolve the ambiguity, but this is unergonomic. More pressingly, this ambiguity can in fact create a Fragile base class problem that can break library users' code. Consider the following scenario:

Initial situation:

There are three crates in this scenario: A low-level library, a high-level library that depends on the low-level one, and user code that uses the high-level library. The high-level library uses the trait from the low-level library as a supertrait, and the user code then uses the high-level library's trait as a generic bound:

Low-level library:

mod low {
    pub trait Super {

    }
}

High-level library:

use low::Super;

mod high {
    pub trait Sub: Super {
        fn foo(&self);
    }
}

User code:

use high::Sub;

fn generic_fn<S: Sub>(x: S) {
    // ok
    x.foo();
}

Library change:

At some point in time, the low-level library's supertrait gets refactored to have a method that also happens to be called foo:

mod low {
    pub trait Super {
        fn foo(&self);
    }
}

The user code is unchanged, but breaks:

use high::Sub;

fn generic_fn<S: Sub>(x: S) {
    // error: both Super::foo and Sub::foo are in scope
    x.foo();
}

A change to add a supertrait to a public trait or to add a method to an existing supertrait can therefore cause downstream breakage. Notably, the user code was never aware of the supertrait, and the low-level library could never have known the signatures of the high-level library. Taking care not to introduce name conflicts is therefore not possible, since any name that is safe at present could cause a conflict in the future, both from the perspective of the low- and the high-level library:

  • The low-level library can't know all the crates that depend on it, and how they use the trait. It's not possible for it to go through dependent crates and check if the name already exists.
  • The high-level library was "first" in defining the name, at the time of naming the issue didn't exist. When the low-level library changes, it's now stuck. Either it can't update its dependency, or it is forced to rename its method, which is a breaking change.

This kind of change is acknowledged as breaking but minor by the API Evolution RFC. However, it does not specifically consider the case of sub-/supertrait interaction. It turns out that in this specific situation there is a potential solution for the problem that avoids the breakage in the first place.

To resolve this issue, this RFC proposes the following: If the user does not explicitly bring the supertrait into scope themselves, the subtrait should "shadow" the supertrait, resolving the current ambiguity in favor of the subtrait.

Guide-level explanation

When using a trait as a bound in generics, or using a trait object, a trait with a supertrait will only bring the supertrait's items into scope if it does not define an item with the same name itself. Items on the object in question that were previously ambiguous now resolve to the subtrait's implementation. While it is still possible to refer to the conflicting supertrait's items, it requires UFCS. Supertrait items that were not previously ambiguous continue to be in scope and are usable without UFCS.

In the context of the trait examples above, this means:

fn generic_fn<S: Sub>(x: S) {
    // This:
    x.foo();
    // is the same as:
    Sub::foo(x);
    // also still possible:
    Super::foo(x);
}
fn use_trait_obj(x: Box<dyn Sub>) {
    // This:
    x.foo();
    // is the same as:
    Sub::foo(x);
    // also still possible:
    Super::foo(x);
}

However, when both subtrait and supertrait are brought into scope, the ambiguity remains:

fn generic_fn<S: Sub+Super>(x: S) {
    // Error: both Sub::foo and Super::foo are in scope
    x.foo();
}

This solution makes intuitive sense: If the user requested Sub and not Super, they should get Sub's items, not Super's. In fact, the user might never have known about the existence of Super, in which case the error message would be confusing to them.

Choosing not to resolve ambiguities when both traits are explicitly requested similarly makes sense: Both traits seem to be wanted by the user, so it's not immediately clear which trait should take precedence.

The feature is backwards-compatible: Anything changed by this proposal was previously rejected by the compiler. As seen in the motivation section, it also improves forwards-compatibility for libraries.

Reference-level explanation

Currently, when a trait is brought into scope through generics or trait objects, all of its supertrait's items are brought into scope as well. Under this proposal, a supertrait's items would only be brought into scope if an item with that name is not already present in the subtrait. This extends to the case of multiple supertraits without special provisions, the rule is simply applied for each supertrait.

Specifically, if two supertraits of a subtrait conflict with each other, but not with the subtrait, it is still an error to refer to the item without UFCS, just as it is today:

trait Super1 {
    fn foo(&self);
}

trait Super2 {
    fn foo(&self);
}

trait Sub: Super1+Super2 {}

fn generic_fn<S: Sub>(x: S) {
    // Is and will continue to be an error
    x.foo();
}

The resolution rule applies recursively to super-supertraits as well:

trait SuperSuper {
    fn foo(&self);
    fn bar(&self);
}

trait Super: SuperSuper {
    fn foo(&self);
    fn bar(&self);
}

trait Sub: Super {
    fn foo(&self);
}

fn generic_fn<S: Sub>(x: S) {
    // Resolves to Sub::foo
    x.foo();
    // Resolves to Super::bar
    x.bar();
}

fn generic_fn_2<S: Sub+SuperSuper>(x: S) {
    // Error: both Sub::foo and SuperSuper::foo are in scope
    x.foo();
}

A case previously not presented, but also technically affected by this RFC, is the definition of a trait itself. Supertraits are brought into scope here as well, through the act of defining them as supertraits in the first place. Therefore, a situation like the following might also be of interest:

trait Super {
    fn foo(&self);
}

trait Sub: Super {
    fn foo(&self);

    fn bar(&self) {
        // Is and will continue to be an error
        self.foo();
    }
}

Using self.foo() is an error today, and it is reasonable to expect it to be, since it is not clear which trait it refers to. Under the rule laid out above, this will continue to raise an error, since Super is explicitly mentioned and brought into scope.

Drawbacks

This change makes the implementation of bringing supertrait items into scope a bit more complex.

While it is not the intent of this RFC, the resolution strategy it introduces is somewhat similar to how inheritance in object-oriented languages works. Users coming from those languages may be confused when they realize that Rust actually works differently. Specifically, items in Rust aren't inherited, and methods with the same name will only shadow and not override. (See also Prior art for this distinction.)

Rationale and alternatives

The simplest alternative is to continue with the status quo, and require the user to explicitly disambiguate items with the same name using UFCS.

The affected area of this RFC is fairly minor, and items with the same name don't seem to come up often enough that it would be urgently needed. (The issue was reported in 2014, but has so far attracted little attention.)

However, the current behavior seems to go against the spirit of why supertrait items are brought into scope: to make traits more ergonomic by avoiding additional trait bounds or explicit supertrait naming in UFCS. Ironically, right now this introduces a requirement for UFCS that wouldn't exist without the automatic scoping of supertraits.

Moreover, as demonstrated in the Motivation section, it is currently possible to inadvertently introduce downstream breakage by changing a supertrait. As outlined in the example, this can cause breakage to bubble down multiple layers of dependencies and cause errors far from the origin of the change. The breakage is especially unexpected by the user because they didn't mention the supertrait or its library themselves.

Alternatives

Resolving in favor of the supertrait instead

While it is theoretically possible to resolve in favor of the supertrait, this is very counterintuitive and there is no reason to do so. There can't be a converse "fragile derived class problem", because the subtrait knows all its supertraits. Therefore, before adding a method in the subtrait, all the supertraits can be checked for a method of the same name. This is not possible in the "fragile base class" case because the supertrait can't know all its subtraits.

Always resolving in favor of the subtrait

This RFC's resolution strategy explicitly rejects resolving items when both the sub- and the supertrait have been brought into scope by the user explicitly. An alternative would be to always resolve in favor of the subtrait. It can be argued that this is in line with an intuitive notion of Sub specializing Super. However, there are a few drawbacks to this strategy:

  • This would be inconsistent with how use declarations work, although they mainly work that way because they don't bring supertrait items into scope.

  • This RFC already resolves in favor of the subtrait when the supertrait is not explicitly brought into scope. Explicitly specifying the supertrait is likely done for a reason, and it would appear counterintuitive when the mention of the supertrait does not influence resolution at all.

  • If the user wants resolution to be in favor of the subtrait, all they have to do is remove the explicit mention of the supertrait. The non-conflicting supertrait items will continue to work anyway, since they are implied by the subtrait.

Order-dependent resolution

Another possibility in the face of ambiguity would be to resolve in favor of the last specified trait, so that a bound on Sub + Super resolves in favor of Super, while Super + Sub resolves in favor of Sub. However this adds semantic meaning to the ordering of traits in bounds, which right now is order-agnostic. It's also not very clear and may lead to user confusion when bounds are reordered, which could change program behavior in subtle ways.

All in all, rejecting to resolve ambiguity seems like the right way to go.

Prior art

Typeclasses in Haskell

Haskell's typeclasses are closely related to Rust's traits, so it's worth seeing what the situation looks like in Haskell.

In Haskell, defining a typeclass constraint on a function does not automatically bring into scope methods defined in superclasses, no matter what they're called. To use them, they have to be explicitly imported. In fact, since Haskell binds method names to the module namespace, and there is no separate namespace for typeclasses, not even methods from the explicitly specified subclass are imported. They are only available if they are imported themselves, either by importing the entire module or by importing the methods by name explicitly.

As a result, this RFC's issue doesn't arise in Haskell in the first place, similarly to how it doesn't arise when use notation is used in Rust.

Inheritance in object-oriented languages

The general idea of a method call on an object resolving to the most derived class's implementation is typically the way inheritance works in object-oriented languages. A notable distinction is that in object-oriented languages, defining a method with the same name (actually, signature) typically overrides the base class's method, meaning that the derived implementation will be selected even in a context that only references the base class. This is not how traits work in Rust, supertraits have no way of calling subtrait implementations, and this will not be changed by this RFC. Instead, the subtrait will only shadow the supertrait's items, and Super::foo and Sub::foo will still be two distinct functions.

Similar distinctions exist in

  • Visual Basic,
  • C#, (called "hiding"),
  • Java, (called "hiding", however the concept is more limited, with instance methods always overriding and static methods always hiding).

Unresolved questions

Terminology

It's not immediately clear which terminology to use for this behavior. Both "shadowing" and "hiding" are used for immediately related behaviors in object-oriented languages, with "variable shadowing" also being used more generally for variable scopes, and "name masking" used for the same concept as "variable shadowing" but from a different perspective. The concept of variable shadowing also already exists in Rust today, and the similar name could be a source of confusion.

There's also some clarification needed on how the term should be used: Should shadowing only be used for the items ("Sub::foo shadows Super::foo") or also for the traits themselves ("subtraits shadow supertraits")?

Note: using "hiding" may lead to a different interpretation here: "subtraits hide supertraits" sounds like the entirety of the supertrait is hidden.

Further fragile base class situations

The situation laid out above is actually not the only fragile base class situation in Rust. Consider the following:

trait Super1 {
    fn foo(&self);
}

trait Super2 {
    // fn foo(&self);
}

trait Sub: Super1+Super2 {}

fn generic_fn<S: Sub>(x: S) {
    x.foo();
}

The above will compile, however adding a method foo to Super2 will result in an error due to the introduced ambiguity. This RFC's resolution strategy won't immediately help here either, since the error does not result from a sub-/supertrait interaction. In fact, this is a fundamental problem of multiple inheritance.

However, using this RFC's rule, it would be possible to at least manually change Sub to prevent the breakage from flowing further downstream:

trait Super1 {
    fn foo(&self);
}

trait Super2 {
    fn foo(&self);
}

trait Sub: Super1+Super2 {
    fn foo(&self) {
        Super1::foo(self);
    }
}

fn generic_fn<S: Sub>(x: S) {
    x.foo();
}

By manually resolving the ambiguity the error can be avoided. This RFC's part here is to make it possible to resolve the ambiguity close to where it originates, instead of having to do it at the level of generic_fn, where the supertraits were never explicitly mentioned. However, this RFC is not able to resolve the fundamental issue, and it is considered out of scope for this RFC, which is intended to deal only with single "inheritance".

Future possibilities

use declarations

As mentioned before, use declarations work differently from trait bounds in generics and trait objects. There may be some benefit in unifying their behavior, so that use declarations bring supertrait items into scope as well. However this would be a breaking change, since it has the potential to introduce ambiguity. It could however still be considered for an edition.

Further resolution of ambiguity

As demonstrated above, there are other fragile base class problems unaddressed by this RFC. There are some possibilities of addressing this, for example by considering the order of supertraits. However, it may be reasonable to explicitly reject resolving the ambiguity, as a resolution could mean a subtle change in behavior when a supertrait changes. It may be preferable to keep this an error instead, and ask the user to explicitly specify.


Thank you for reading!

I'd be especially interested if anyone here has been affected by this situation before, either as a user or a library author. Of course, any other feedback is welcome as well. :slight_smile:

10 Likes

From a users perspective this would indeed be a good addition in my opinion.

I think that another alternative is to always resolve based on the type itself. (Be it the sub or the super trait).

That way you can have (in the same module) a function that uses the super trait and one that uses the sub trait. I also think that this would be the least surprising.

trait Sub: Super is sugar for trait Sub where Self: Super, so A: Sub means the same thing as A: Super + Sub.

Right, currently it's just sugar for Super + Sub. However, this means it's not possible to write Sub and mean "import Sub's items only, and not Super's". For most cases this doesn't matter, but it does in the case where both traits have an item with the same name. Maybe this was just overlooked when the sugar was introduced. That's why I'd like to change the behavior slightly so that the need for UFCS in this case is avoided.


I'm not sure what you mean here, could you elaborate? The pre-RFC is meant for generics/trait objects, where the specific type can't be known. The traits to be used are also specified through trait bounds per function / trait object, so it's already possible to use different bounds for different functions in the same module.

Your proposal could be implemented by adding a non-commutative trait combination operator (e.g. >) and desugaring to (Sub > Super).

Edit: for precedence, I’d punt by making parens mandatory if both operators are used in a single expression, making A + B > C an error.

The "less than" operator is probably not optimal, but another asymmetrical lexeme could do the trick.

Oh no, not any more operators and even more parentheses in types, please. You already started discussing about the syntax, that's a whole other rabbit hole we don't need.

But why would you want that? That sounds super confusing. Since trait "inheritance" is close conceptually (and in usage) to proper subtyping, not knowing based on the name whether a Sub (that is also simultaneously a Super) refers to Sub::Foo or Super::Foo is a huge readability burden.

4 Likes

Without delving into syntax, my point was: let’s create a mechanism to let users define per context method resolution priority and desugar sub-traits to that.

However that is probably a bad idea. Take the case where you have code bounded to x: Foo + Bar, where Bar implements a method and Foo doesn’t, and the code uses x.method. If Foo also gets a method, you get a compile-time error. For code bounded by Foo > Bar, you’d get new, possibly unexpected runtime behavior.

I'm not sure why this would be a concern compared to the situation that already exists today. Whenever a trait function is used in method syntax you already can't tell from the method invocation alone which trait it refers to, you have to check the generic bounds and find a trait that has a matching function. It's also already possible that the traits in the bounds don't define a method themselves but bring a supertrait's method into scope.

So, currently, the user's 'algorithm' for finding which trait a method call refers to is to go through each trait in the trait bound, and for each of these traits check whether it has a matching function, and if not, recursively consult supertraits until one does.

My proposal actually doesn't change this 'algorithm' at all, it just makes more situations compile than before. Since subtrait methods would shadow supertrait methods, you still only need to go up the hierarchy until you find a matching name.

I'd say the resolution is quite clear for the user: If you have Sub as a bound, you're going to get Sub's items in method syntax, if you have Super as a bound, you're going to get Super's.

1 Like

Right, I briefly touched on this in the pre-RFC text. For arbitrary traits, resolving the ambiguity has the potential to introduce unexpected subtle changes in behavior when one of the traits adds a method which then takes precedence. This is an issue because, as arbitrary unrelated traits, neither is aware of the other's items, and can't plan to avoid naming conflicts.

However, this is fine in the sub-/supertrait case, because the subtrait can check the supertrait for an already existing method, and avoid the name if it already exists. However the supertrait can't do the converse, and because of that, my proposal shadows the supertrait so there can't be a subtle change if the supertrait adds a method.

1 Like

There hasn't been much activity on this thread in a while. This is my first Pre-RFC, so I'm unfamiliar with the process. Is this generally seen as a sign that there are no outstanding concerns, or that there isn't enough interest, or that more time is needed so people can voice their concerns if they arise? I'm wondering if/when a move to file a formal RFC would be appropriate.

BTW, @Nokel81, I'm still interested in your comment about "resolving based on type", I'm not sure what you meant by that. It'd be great if you could go into more detail about this. :slightly_smiling_face:

Given that this is true, and that I didn't know that this is how it works. My suggestion is sort of moot.

However, what I was going for was fn foo<T> (arg: T) where T: Sub would only use Sub's methods but fn foo<T> (arg: T) where T: Super would prefer Super's methods.

That's actually the way it would work under this pre-RFC's proposal. (With Sub also bringing Super's items into scope, but in cases of name conflicts, this is how it would work.)

Right now Sub may be the same as Sub + Super, but the compiler currently rejects name conflicts, so the rules for resolving them are still up for design without breaking backwards compatibility.

This RFC could probably start its life as a clippy lint. How often does it happen where you use methods from Super when only Sub is in scope? If we had that lint and did a crater run we could roughly estimate how much code would be affected by these sorts of naming conflicts.

Starting off with a lint is also considerably easier than trying to convince people to accept a breaking change to Rust's method resolution. Plus doing experiments and providing the data changes the RFC from "we'd like to avoid this theoretical problem" to "here are a list of places where the current behaviour is a source of footguns".

2 Likes

Quite often, and you can still do that in this proposal. If you've ever bound on Eq but not PartialEq, you've used a supertrait's methods. If you've ever done comparison on a type bound by Ord but not PartialOrd, you've used a supertrait's methods.

There is no breaking change here.

The only change is that when both a subtrait and a supertrait introduce some name, where it currently does not compile, we prefer the "closer" implementation. This is exactly how we already treat inherent implementations against trait methods, even.

4 Likes

Other than the points that CAD97 has already addressed, I'd like to add that because currently the compiler rejects ambiguous code, a crater run also wouldn't be an option. Any crate that has actually run into the outlined situation either doesn't compile and wouldn't be uploaded to crates.io, or has worked around the situation by renaming a method or removed a supertrait (accepting a breaking change), which can't be detected because this also could have happened for other reasons. It also doesn't consider the possibility that some crates could have intentionally avoided introducing a supertrait to avoid running into the situation.

It's also not an option to scan for crates that are "at risk" of being affected by the outlined situation, because it could happen anywhere a public trait uses a supertrait from another crate (even including the standard library). Using a supertrait from std or a different crate is probably quite common (and would be completely fine unless/until the supertrait adds a conflicting item), so a scan wouldn't really help much.

Since no unresolved concerns have arisen in the thread here, I've gone ahead and filed an RFC for this.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.