Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a trait for types implementing some other trait #5195

Closed
fcarreiro opened this issue Jun 6, 2024 · 0 comments · Fixed by #5646
Closed

Implement a trait for types implementing some other trait #5195

fcarreiro opened this issue Jun 6, 2024 · 0 comments · Fixed by #5646
Assignees
Labels
bug Something isn't working

Comments

@fcarreiro
Copy link

Aim

Implement a trait for types implementing some other trait.

E.g., SerializeToSlice based on Serialize.

trait SerializeToSlice {
    fn serialize_to_slice(self) -> [Field];
}

impl SerializeToSlice for [Field] {
    fn serialize_to_slice(self) -> [Field] {
        self
    }
}

impl<T,N> SerializeToSlice for T where T: Serialize<N> {
    fn serialize_to_slice(self) -> [Field] {
        Serialize::serialize(self).as_slice()
    }
}

This seems to compile but then

fn use_it<T>(t: T) -> [Field] where T: SerializeToSlice {
    SerializeToSlice::serialize_to_slice(t)
}

triggers

error: No matching impl found for `T: Serialize<_>`
    │
390 │     SerializeToSlice::serialize_to_slice(t)
    │     ------------------------------------ No impl for `T: Serialize<_>`
    │
    = Required by `T: SerializeToSlice`

Expected Behavior

Works

Bug

Not sure exactly what's going on.

To Reproduce

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

discussed with @TomAFrench

cc: @nventuro @Thunkar

Installation Method

None

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@fcarreiro fcarreiro added the bug Something isn't working label Jun 6, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jun 6, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 2, 2024
# Description

## Problem

Resolves #5195

## Summary

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

```rust
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:

```rust
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:

```rust
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:

```rust
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:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants