Trait method shadowing can make adding a method to a sealed trait a breaking change and silently change behaviour in downstream crates

[maia-s]

(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:

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:

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

% 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

[compiler-errors]

I'd like to gauge the (general) impact of this kind of trait shadowing on blanket impls. I've started an experiment here: #128560