Repository: rust
Issue: Trait method shadowing can make adding a method to a sealed trait a breaking change and silently change behaviour in downstream crates
Labels: T-lang,T-compiler,C-bug,T-types

(I already reported this as a diagnostic bug at #127703, but I was advised to re-submit it as a "real" bug as this has more serious consequences than I initially thought, and updates to that issue would be unlikely to be noticed at this point. I'll say everything here so you don't have to read the previous issue.)

In short, the issue is that in a trait impl for a type, if a method exists in both a dependent trait for that type and for the trait being implemented, the method in the dependent trait is used with no warning. If the method only exists in the trait being implemented, the method from the trait itself is used. The switch from the latter to the first can be triggered with no changes to the crate itself by adding a matching method to the dependent trait in an upstream crate, or the upstream crate can cause a compile error in the downstream crate by adding a method with the same name as in the downstream crate's trait but with an incompatible signature. This does not use supertraits.

As an example, make a crate `a` with this code in its `lib.rs`:
```rust
use sealed::Sealed;

mod sealed {
    pub trait Sealed {}
}

pub trait TraitA: Sealed {
    fn do_things(&self);
    //fn do_more_things(&self);
}

impl Sealed for i32 {}

impl TraitA for i32 {
    fn do_things(&self) {
        println!("TraitA::do_things()");
    }

    //fn do_more_things(&self) {
    //    println!("TraitA::do_more_things()");
    //}
}
```
(`TraitA` is sealed for illustrative purposes; sealing isn't required for this issue to happen)

Now make a crate `b` that depends on crate `a` and has this code in its `main.rs`:
```rust
use a::TraitA;

pub trait TraitB {
    fn do_more_things(&self) {
        println!("TraitB::do_more_things()");
    }

    fn do_all_the_things(&self);
}

impl<T: TraitA> TraitB for T {
    fn do_all_the_things(&self) {
        self.do_things();
        self.do_more_things();
    }
}

fn main() {
    let i = 0_i32;
    i.do_all_the_things();
}
```

This is the output when you `cargo run` crate `b`:
```
TraitA::do_things()
TraitB::do_more_things()
```

Now uncomment the commented lines in crate `a`. After this change, `cargo run` on crate `b` outputs this:
```
TraitA::do_things()
TraitA::do_more_things()
```

Crate `b` changed from calling `TraitB::do_more_things` to calling `TraitA::do_more_things`. Unlike the example in the previous bug report, there are no warnings for unused functions here.

`TraitA` is a sealed trait, and the common wisdom is that just adding a new method to a sealed trait can't break downstream crates, but in this case it silently changed the logic of crate `b`. If `do_more_things` was `unsafe` in both traits, this could even silently introduce undefined behavior if the preconditions for the two methods were different.

If the signature of `TraitA::do_more_things` is changed to be incompatible with `TraitB::do_more_things`, crate `b` will fail to compile.

This happens with associated functions used through `Self` as well as for methods. `TraitA` doesn't have to be sealed.

[Playground link with another example from the previous bug report](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9169b4e94344b15f380c0dbb50a445d7)

```
% rustc --version --verbose
rustc 1.80.0 (051478957 2024-07-21)
binary: rustc
commit-hash: 051478957371ee0084a7c0913941d2a8c4757bb9
commit-date: 2024-07-21
host: aarch64-apple-darwin
release: 1.80.0
LLVM version: 18.1.7

% rustc +nightly --version --verbose
rustc 1.82.0-nightly (c1a6199e9 2024-07-24)
binary: rustc
commit-hash: c1a6199e9d92bb785c17a6d7ffd8b8b552f79c10
commit-date: 2024-07-24
host: aarch64-apple-darwin
release: 1.82.0-nightly
LLVM version: 18.1.7
```