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

Rust's orphan rule doesn't seem to hold #5475

Closed
nventuro opened this issue Jul 10, 2024 · 4 comments
Closed

Rust's orphan rule doesn't seem to hold #5475

nventuro opened this issue Jul 10, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@nventuro
Copy link
Contributor

nventuro commented Jul 10, 2024

Aim

I'm able to define impls for traits in modules in which neither the trait nor the type are defined. My understanding is that this is illegal (the so-called 'the orphan rule').

Bug

mod serde {
    trait Serialize<let N: u32> {
        fn serialize(self) -> [Field; N];
    }
}

use crate::serde::Serialize;

// Even though Field is primitive, this should be illegal - the definition should go inside the serde
// module (where the trait is defined).
impl Serialize<1> for Field {
    fn serialize(self) -> [Field; 1] {
        [self]
    }
}

mod many_field {
    struct ManyFieldStruct {
        a: Field,
        b: Field,
    }
}

use crate::many_field::ManyFieldStruct;

// In this case neither the struct nor the trait have been defined in this module, yet I can still provide
// an impl and break the orphan rule.
impl Serialize<2> for ManyFieldStruct {
    fn serialize(self) -> [Field; 2] {
        [self.a.serialize()[0], self.b.serialize()[0]]
    }
}

#[test]
fn test() {
    let bar = ManyFieldStruct { a: 13, b: 42 };

    let bar_ser = bar.serialize();
    assert_eq(bar_ser[0], 13);
    assert_eq(bar_ser[1], 42);
}

I tried wrapping trait with an inner module (i.e. serde::foo::Serialize), and then place a second impl for Field there, and got a duplicate impl error, so at least that seems to work as expected.

@nventuro nventuro added the bug Something isn't working label Jul 10, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 10, 2024
@asterite
Copy link
Collaborator

I think this is working as expected. The orphan rule seems to be about not allowing these across crates, but here everything is defined in the same crate.

@asterite
Copy link
Collaborator

(put another way: I couldn't reproduce this issue if mod serde is in a different crate)

@asterite
Copy link
Collaborator

Closing as this is how it works in Rust too, but feel free to reopen if what I'm saying is wrong.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 22, 2024
@nventuro
Copy link
Contributor Author

Ahh you seem to be right, sorry! For some reason I was sure modules had to contain both definition and impl. This makes much more sense.

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
Archived in project
Development

No branches or pull requests

2 participants