fix: allow multiple trait impls for the same trait as long as one is in scope

[asterite]

Description

Problem

This code used to give an error saying multiple traits (Convert and Convert) were in scope:

struct Foo {
    inner: Field,
}

trait Converter<N> {
    fn convert(self) -> N;
}

impl Converter<Field> for Foo {
    fn convert(self) -> Field {
        self.inner
    }
}

impl Converter<u32> for Foo {
    fn convert(self) -> u32 {
        self.inner as u32
    }
}

fn main() {
    let foo = Foo { inner: 42 };
    let _: u32 = foo.convert();
}

Summary

Now the error is gone. We search if there's only one trait, not one trait method, in scope. In the snippet above there are two but they are for the same trait so it's fine. Later on the type-checker checks that it's only bound to the one for u32.

If we remove the u32 type annotation, we get this error:

error: Multiple trait impls match the object type `Foo`
   ┌─ src/main.nr:25:13
   │
25 │     let x = foo.convert();
   │             ----------- Ambiguous impl
   │
   = Candidate 1: `Foo: Converter<Field>`
   = Candidate 2: `Foo: Converter<u32>`

Additional Context

To be honest, I'm not sure where's the code that eventually checks what's the impl for foo.convert() that matches against u32, but seeing it work, and seeing the error we get when there are multiple matching impls, I'm confident it's working fine (it seems to be in verify_trait_constraint right before we end analyzing a function)

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.

[github-actions]

Execution Report

Program	Execution Time
sha256_regression	0.054s
regression_4709	0.001s
ram_blowup_regression	0.575s
rollup-root	0.105s
rollup-block-merge	0.106s
rollup-base-public	1.330s
rollup-base-private	0.521s

[github-actions]

Compilation Report

Program	Compilation Time
sha256_regression	1.040s
regression_4709	0.771s
ram_blowup_regression	14.800s
rollup-root	3.720s
rollup-block-merge	3.820s
rollup-base-public	30.400s
rollup-base-private	12.300s

[github-actions]

Compilation Memory Report

Program	Peak Memory	%
keccak256	78.55M	0%
workspace	123.47M	0%
regression_4709	422.96M	0%
ram_blowup_regression	1.58G	0%
rollup-base-public	2.55G	0%
rollup-base-private	1.24G	0%
private-kernel-tail	168.56M	-16%
private-kernel-reset	168.59M	-77%
private-kernel-inner	168.56M	-43%

[github-actions]

Execution Memory Report

Program	Peak Memory	%
keccak256	74.65M	0%
workspace	123.28M	0%
regression_4709	315.98M	0%
ram_blowup_regression	512.41M	0%
rollup-base-public	492.17M	0%
rollup-base-private	332.89M	0%
private-kernel-tail	168.56M	-8%
private-kernel-reset	168.59M	-34%
private-kernel-inner	168.56M	-21%

[jfecher]

LGTM

lookup_struct_method has gotten way too large, we should break it up into pieces sometime

[asterite]

lookup_struct_method has gotten way too large, we should break it up into pieces sometime

Makes sense. There are a couple more trait PRs. After those I'll try to simplify things.

[TomAFrench]

@asterite This is causing errors in aztec-packages, is there an issue tracking this?

[asterite]

Oh, I didn't notice. All tests passed. Was it because what you found in the other PR?

[asterite]

Where can I see the errors?

[TomAFrench]

Nope, we're failing to compile the private_kernel_reset package as part of the benchmarking workflow here.

[TomAFrench]

https://github.com/noir-lang/noir/actions/runs/12675812872/job/35327773532

[asterite]

Strange, I don't understand why it errors. That said, I have a workaround, I'll push that for now.