Rust: Inherent/trait method priority rules exist, but are unclear and seem to be undocumented

[niconii]

Consider this code:

struct Foo;

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

trait Bar {
    fn foo(&self);
}

impl Bar for Foo {
    fn foo(&self) { 
        println!("Bar"); 
    }
}

fn main() {
    let mut f = Foo;
    f.foo();
}

Unlike a method name conflict between two traits, Rust will compile this code, which will produce:

One might conclude that the inherent method (i.e. belonging to the type itself) Foo::foo() has priority over the trait method Bar::foo(). Yet, if we change the signature of Foo::foo() to fn foo(&mut self) in this code, suddenly the program outputs:

So it would seem the type of arguments also plays some role in which method is chosen. To try and understand the rules behind it, I created the following table which shows the output for each combination of self's type in Foo::foo() and Bar::foo():

| Bar↓ Foo→ | &Self | &mut Self | Self |
| --- | --- | --- | --- |
| &Self | Foo | Bar | Foo |
| &mut Self | Foo | Foo | Foo |
| Self | Bar | Bar | Foo |

Looking at this, I think I can deduce that:

1. Inherent methods indeed have priority over trait methods when types are equal.
2. The priority of types is &mut Self < &Self < Self.

That said, these rules are not terribly intuitive, and I'm unsure how they stack up against more complex method signatures. It would be good to have documentation about this.

Another concern of mine, however, is the inconsistency between this and a situation involving multiple trait methods conflicting. In that situation, rustc gives an error and asks the programmer to specify which trait's method should be used via UFCS. I'd prefer something like that over the current situation, though I'm not sure it's possible to do backwards-compatibly.

[steveklabnik]

@nikomatsakis is this some kind of bug?

[steveklabnik]

/cc @rust-lang/lang , I repeat my question from last time: is this a bug, or are there rules? We should doc the rules if this is expected.

[eddyb]

IMO what's being described here is a subtle but correct combination of autoderef and priority rules.

[withoutboats]

Ugh I would prefer it if this were not the case but don't know what would break! In particular, I would prefer if & vs &mut didn't influence dispatch precedence.

[softprops]

I believe that what ever the rules for this were they recently changed in beta ( and earlier on in nightly )

I have a travis build that just started failing on beta do the incorrect selection of dispatch

https://travis-ci.org/softprops/afterparty/jobs/188728497#L378

I think I have precisely the case listed in this issue. I have an inherent impl method called handle which takes a &mut self and impl a trait ( hyper's Handler ) method, also called called handle which takes &self

on stable the prioritization was my inherent impl and on beta/nighlightly the priority has become the trait impl. I worked around this with UFCS in https://github.com/softprops/afterparty/commit/2b1548331873b76e8a5b3f5b3e07e2e5fdbb5b52 but I think this change may come as a surprise to some. It may very well be the case that my usage has worked by accident but I think the lack of docs on expectation for this may hurt others as well.
this works on stable but

[softprops]

what's wierd is that if I modify the playpen example above to try and reproduce the error I see in my build it fails on stable as well in the playpen https://play.rust-lang.org/?gist=ac3bb4233c2c32876ff68f0fcd9c222d&version=stable&backtrace=1

[niconii]

The priority table in my original post seems to still be the same on rustc 1.14.0 (e8a012324 2016-12-16) and rustc 1.16.0-nightly (4ecc85beb 2016-12-28), though I don't know if or how other arguments play into all this.

[softprops]

Mysterious

[softprops]

I found what I think my by a work around for my case with scoping. As long as the trait with a conflicting method is not in scope things work ask expected. Another case for avoiding glob-style imports!

Here's an example.
https://play.rust-lang.org/?gist=5affb565d4b3876e006ef43a82a12405&version=nightly&backtrace=1

[nikomatsakis]

I've never commented on this bug it seems. These rules are indeed intentional and yes they are the result of an interplay between autoderef rules and prioritization. The algorithm is that we first arrange a series of steps which correspond to increasing amounts of deref operations. We will try these steps in order and stop at the first step for which we find methods. Within a given step, we will give inherent methods priority over trait methods.

This ordering is the result of a lot of experimentation and making changes to it will definitely cause massive breakage. Before 1.0, I did spend some time experimenting with different "cleaner" strategies, but abandoned it because they broke a lot of code and by and large made things work less well (i.e., more explicit * and &).

The exact algorithm:

• Assemble list of types found by repeatedly deref'ing, with the addition of a single "unsize" step.
  
  
  
  • Example: if receiver is Box<Vec<T>>, we would produce:
    
    
    
    
    
    • Box>
    
    
    
    • Vec (deref)
    
    
    
    • [T] (unsize)
    
    
    
  
  
• Iterate down that list, and for each type X:
  
  
  
  • Try with receiver of X (no adjustments), then &X (one autoref), then &mut X (one autoref-mut):
    
    
    
    
    
    • Search for inherent methods defined on the given type (X, &X, or &mut X)
    
    
    
    • If none found, search for extension methods
    
    
    
  
  

I am certainly not saying that this procedure is ideal, but we definitely can't cavalierly change it, and it has some nice properties. For example, if you have an &self method in an impl defined on T, that is equivalent in resolution order to a fn(self) method defined in an impl for &T.

An example of a change I tried to make which did not work was to search for impls defined directly on the type X and then apply the autoref by looking at how the method was defined. This seems cleaner, but it interacted quite poorly with Clone, since with the new rules, if you invoke clone() on an &T type, it would invoke the (no-op) impl of Clone for &T instead of invoking the Clone impl for T (which is what you want, and what we do today). That is, today, if the receiver has type &T, we will search for methods whose receivers are &T first -- and this matches the impl of Clone for T, since clone() is an &self method (whereas the impl of Clone for &T would match when we search for &&T).

[nikomatsakis]

So basically I think we have to document these rules but I wouldn't want to change them without careful consideration.

[QuietMisdreavus]

In a docs team meeting we decided this would be best served in the reference, so I'm moving the issue over there: rust-lang-nursery/reference#62

[dwrensha]

I just ran into a surprising (to me at least) consequence of the current rules: https://github.com/capnproto/capnproto-rust/issues/90

I have a struct Foo<'a> with an inherent method borrow<'b>(&'b self) -> Foo<'b>, and it apparently collides the the stdlib Borrow trait, which has impl<'a, T: ?Sized> Borrow<T> for &'a T. When I do foo.borrow(), I expect the inherent method to be called, but the trait method gets chosen because, as I understand it, it doesn't need to do any deref conversion.

[nikomatsakis]

@dwrensha while it can happen that trait methods get preferred, I'm a bit confused by the wya you described the situation. For example, in this self-contained example, the inherent method is chosen.

[dwrensha]

@nikomatsakis oops! I forgot about a mut qualifer. My inherent method looks like:

impl <'a> Foo<'a> {
   borrow<'b>(&'b mut self) -> Foo<'b> { ... }
}

If I add a mut to your self-contained example, then the trait method is chosen.

[nikomatsakis]

I see. That does match my expectations, for better or worse -- in particular, the method selection algorithm first tries no-ref, then shared-ref (&), then mut ref (&mut). I think that in retrospect I wish that & and &mut were equivalent, but it'd be a breaking change to alter that order now. This seems related to https://github.com/rust-lang/rust/issues/39232 as well.

It seems to me that it's sort of late to make any changes here for the current epoch -- but I feel like an epochal transition is exactly where we might try to make improvements.