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

[Regression] Installing a module with a crate name panics. #560

Closed
DarkRTA opened this issue Jun 11, 2023 · 3 comments
Closed

[Regression] Installing a module with a crate name panics. #560

DarkRTA opened this issue Jun 11, 2023 · 3 comments
Labels

Comments

@DarkRTA
Copy link
Contributor

DarkRTA commented Jun 11, 2023

When using rune from the main branch, you can not install a module with a different crate name due to a type hash mismatch. This works fine in 0.12.4.

use rune::Module;
use rune::Context;
use rune::Any;

#[derive(Any)]
struct X {
    #[rune(get, set)]
    x: i32,
}

impl X {
    fn get_x_squared(&self) -> i32 { 
        self.x * self.x
    }
}

fn module() -> Module {
    let mut m = Module::with_crate("testcrate");
    m.ty::<X>().unwrap();
    m.associated_function("x_squared", X::get_x_squared).unwrap();
    m
}

fn main() {
    let m = module();
    let mut ctx = Context::new();
    ctx.install(m).unwrap(); // panics
}

Console Ouput:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: TypeHashMismatch { type_info: Any(AnyTypeInfo { name: "tmp::X" }), item: ::testcrate::X, hash: Hash(0x55e7ff128005ac8f), item_hash: Hash(0x7c57d5ebc9226f50) }', src/main.rs:27:20
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@udoprog
Copy link
Collaborator

udoprog commented Jun 11, 2023

Hey!

It's not a regression! It's a breaking change introduced in #509.

If you print the error it gives you more guidance on what needs to change. Basically you now need to specify the item a type belongs to as it's being installed, for you that means:

#[derive(Any)]
#[rune(item = ::testcrate)]
struct X {
    #[rune(get, set)]
    x: i32,
}

This change will be included in the release notes for 0.15.x. It's necessary because we want to stop relying on TypeId's (which is technically unsound). Good timing too, see #550 where the size of a TypeId is about to change.

@DarkRTA
Copy link
Contributor Author

DarkRTA commented Jun 11, 2023

i'm going to close this then. thanks.
should probably document this somewhere in the docs as its kind of unclear you need to do this if you read the documentation for Module::with_crate

@DarkRTA DarkRTA closed this as not planned Won't fix, can't repro, duplicate, stale Jun 11, 2023
@udoprog
Copy link
Collaborator

udoprog commented Jun 11, 2023

Will do before release. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants