fix: let a trait impl that relies on another trait work

[asterite]

Description

Problem

Resolves #5195

Summary

If I'm understanding the code correctly, when you have code like this:

trait Trait {
  fn trait_func(self);
}

fn foo<T>(t: T) {
  Trait::trait_func(t);
}

the compiler will search in all impls of Trait for a matching one. Then:

• if it only finds one, it succeeds
• otherwise it's an error (no matching impl found, or too many found)

If an impl has a where clause, the compiler checks if that where clause matches. For example:

trait One {
}

impl Trait for T where T: One {
  // ...
}

So here there's an impl for Trait, it has a where clause, so the compiler checks if that T is One. And here's the thing: if there's no match, it would immediately error. I think that was the bug.

In this PR, we capture that error but continue seeing if we can find a matching impl. If we can't find any impl, we use the error of the first non-matching where clause to help users.

But... let me know if I got this wrong. I think I understood the code, but I'm not sure.

Additional Context

When trying out different codes with traits and impls I noticed that we don't error if two trait impls are conflicting. For example Rust gives an error in this case:

trait One {}

impl One for i32 {}

trait Three {}

impl Three for i32 {}

trait Two {}

impl<T> Two for T where T: One {}

impl<T> Two for T where T: Three {}

The error:

error[E0119]: conflicting implementations of trait `Two`
  --> src/main.rs:13:1
   |
11 | impl<T> Two for T where T: One {}
   | ------------------------------ first implementation here
12 |
13 | impl<T> Two for T where T: Three {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation

I don't know if it's a requirement to do this before merging this PR. Even if there are conflicting impls in Noir, the compiler does the right thing. For example this code:

trait One {
}

impl One for i32 {
}

trait Two {

}

impl Two for i32 {

}

trait Three {
    fn three(self);
}

impl<T> Three for T where T: One {
    fn three(self) {
        println("One");
    }
}

impl<T> Three for T where T: Two {
    fn three(self) {
        println("Two");
    }
}

fn use_one<T>(t: T) where T: One {
    Three::three(t);
}

fn use_two<T>(t: T) where T: Two {
    Three::three(t);
}

fn main() {
    let x: i32 = 0;
    use_one(x);
    use_two(x);
}

prints:

One
Two

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[jfecher]

I don't know if it's a requirement to do this before merging this PR. Even if there are conflicting impls in Noir, the compiler does the right thing. For example this code:

The issue rust is pointing out is what if the same type implemented both traits One and Two? The traits themselves aren't overlapping so we can't error when defining them. So what should we do when calling Three::three(3) without a where clause which forces one impl?

[asterite]

@jfecher Oooh, I see. That makes sense. In this case in Noir we say "Expression type is ambigous". I guess Rust errors earlier but we still error on usage so that's good enough for now.

[jfecher]

I'm not 100% sure we should allow this when Rust doesn't but the implementation is simple enough I'm willing to try it out until we run into problems later. For now it is a useful feature to have.

[asterite]

I think Rust does allow it. That is, it allows impl Trait for T where T: SomeOtherTrait, it's just that it also errors if two of those exists for any T. So maybe all we'd need is for Noir to also detect that case... but this is just a guess, I don't know Rust that much yet.