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

Vec<Vec<(String, usize)>> panics with unsatisfied trait bounds #105

Closed
Vagab opened this issue Mar 30, 2024 · 3 comments · Fixed by #106
Closed

Vec<Vec<(String, usize)>> panics with unsatisfied trait bounds #105

Vagab opened this issue Mar 30, 2024 · 3 comments · Fixed by #106

Comments

@Vagab
Copy link
Contributor

Vagab commented Mar 30, 2024

Hi everyone. Probably a stupid question but I'm getting this error when trying to return Vec<Vec<(String, usize)>> from a function and then define it as a singleton method:

error[E0599]: the method `call_handle_error` exists for fn item `fn(String, String) -> Vec<Vec<(String, usize)>> {reg}`, but its trait bounds were not satisfied
  --> ext/xre2/src/lib.rs:98:43
   |
98 |     module.define_singleton_method("reg", function!(reg, 2))?;
   |                                           ^^^^^^^^^^^^^^^^^ method cannot be called due to unsatisfied trait bounds
   |
   = note: the following trait bounds were not satisfied:
           `<fn(String, String) -> Vec<Vec<(String, usize)>> {reg} as FnOnce<(&Ruby, _, _)>>::Output = _`
           which is required by `fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
           `fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: Fn<(&Ruby, _, _)>`
           which is required by `fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
           `<&fn(String, String) -> Vec<Vec<(String, usize)>> {reg} as FnOnce<(&Ruby, _, _)>>::Output = _`
           which is required by `&fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
           `&fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: Fn<(&Ruby, _, _)>`
           which is required by `&fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
           `&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: Fn<(_, _)>`
           which is required by `&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: Function2<_, _, _>`
           `<&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg} as FnOnce<(&Ruby, _, _)>>::Output = _`
           which is required by `&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
           `&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: Fn<(&Ruby, _, _)>`
           which is required by `&mut fn(String, String) -> Vec<Vec<(String, usize)>> {reg}: RubyFunction2<_, _, _>`
   = note: this error originates in the macro `function` (in Nightly builds, run with -Z macro-backtrace for more info)

which in my very limited experience meant that there's no way to implicitly transform this rust type into ruby type(might be wrong, please correct me). So I was wondering why does Vec<(String, usize)> is ok but this one is not?

@matsadler
Copy link
Owner

This is a bug in Magnus, looks like I forgot to implement IntoValueFromNative for Vec.

IntoValue is the trait that handles turning Rust types into Ruby types. Ruby types also implement it, just by returning themselves. Vec implements it like:

impl<T> IntoValue for Vec<T>
where
    T: IntoValueFromNative,
{
    ...
}

IntoValueFromNative is a special version of IntoValue that isn't implemented for Ruby types. This is done so you can't return an Vec containing Ruby types, because you shouldn't ever put Ruby types in a Vec, because the GC will lose track of them.

Vec shouldn't blanket implement IntoValueFromNative (because it could contain Ruby types), but it should impl<T> IntoValueFromNative for Vec<T> where T: IntoValueFromNative. I missed adding this impl.

I don't have time to do it right now, but I'll try and get a patch release out this evening with a fix.

@Vagab
Copy link
Contributor Author

Vagab commented Mar 30, 2024

I see, makes sense. Thank you for the explanation and for such a swift response. I have some time right now so I will try to do it myself, if I do I'll ping you on a pr. Otherwise feel free to fix 👍

upd: here's the pr, let me know if I should add any tests for this or anything, but I built it on my machine and tried it out with my use case and all was fine(with more nested vecs too, makes sense 😅)

@matsadler
Copy link
Owner

I've put out a 0.6.3 release with this fix.

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

Successfully merging a pull request may close this issue.

2 participants