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

Support Vec, HashMap and more types #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sw1sh
Copy link

@sw1sh sw1sh commented Oct 24, 2023

I'm a Rust noob, so some things could probably be improved. I'm trying to build a paclet for this small library: https://github.com/jcmgray/cotengrust/blob/main/src/lib.rs. With these additional types and traits, I've been able to just use #[wll::export] attribute on a function I need:

#[wll::export]
fn optimize_optimal(
    inputs: Vec<Vec<char>>,
    output: Vec<char>,
    size_dict: Dict<char, f32>,
    minimize: Option<String>,
    cost_cap: Option<Score>,
    search_outer: Option<bool>,
    simplify: Option<bool>,
    use_ssa: Option<bool>,
) -> Vec<Vec<Node>>

And it works without fiddling with wstp. Pretty cool stuff! :)

impl FromArg<'_> for char {
unsafe fn from_arg(arg: &MArgument) -> Self {
String::from_arg(arg).chars().next().unwrap()
}
Copy link
Collaborator

@ConnorGray ConnorGray Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sw1sh, as discussed in our meeting, the basic implementation here is exactly right, but we may want to improve the error reporting to help users out in cases where they accidentally pass in strings that are not a single char in length.

My proposal would be something like:

unsafe fn from_arg(arg: &MArgument) -> Self {
    let string = String::from_arg(arg);

    let mut chars = string.chars();

    let Some(char) = chars.next() else {
        panic!("char argument string was empty")
    };

    if chars.next().is_some() {
        panic!("char argument string contains multiple chars: {string}")
    }

    char
}

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 this pull request may close these issues.

2 participants