noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
821 stars 177 forks source link

fix(frontend): Error for when impl is stricter than trait #5343

Open vezenovm opened 4 days ago

vezenovm commented 4 days ago

Description

Problem*

Resolves #5210

Summary*

For context it is good to start with the linked issue and the discussion.

In the linked issue discussion, a non-existent trait implementation would pass type checking and fail during monomorphization. It turns out the linked issue's error was not that the compiler should error with No impl found ... but rather that the impl has stricter requirements than the trait. When we move the where clause to the trait impl we get the expected no matching impl error as expected.

But now the following code from the issue will have this error:

Screenshot 2024-06-26 at 4 30 39 PM

I have also attached a test showing how we get the expected no impl found error when the where clause is accurately placed on the trait impl rather than the trait impl method.

Additional Context

The way we attach trait impl where clauses onto methods did complicate things a little. We should consider maintaining a clearer separation for where clauses on impls and where clauses on functions.

Documentation*

Check one:

PR Checklist*

TomAFrench commented 4 days ago

Looking at the issue in more detail now but doesn't this just boil down to prohibiting any trait bounds on the impl? Would this not be simpler + clearer than allowing it in the situation where it matches up with the trait?

jfecher commented 4 days ago

@TomAFrench we'd still want to allow trait bounds on an impl's method if the corresponding trait method also requires that impl. It's not the most common feature but it does match Rust's behavior.