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 API for qirlib uses String to represent OS paths #44

Closed
cgranade opened this issue Feb 3, 2022 · 2 comments
Closed

Rust API for qirlib uses String to represent OS paths #44

cgranade opened this issue Feb 3, 2022 · 2 comments

Comments

@cgranade
Copy link
Contributor

cgranade commented Feb 3, 2022

The Rust API for qirlib uses the &str type to represent OS-specific paths, such as the emit_ir method, but these slices are immediately converted into Path values.

pub fn emit_ir(&self, file_path: &str) -> Result<(), String> {
    let ir_path = Path::new(file_path);
    if let Err(llvmstr) = self.module.print_to_file(ir_path) {
        return Err(llvmstr.to_string());
    }
    Ok(())
}

These methods should be generalized to allow any type that supports Into<Path>, so that APIs which return Path values can be used with the trivial Into<T> for T impl but without breaking existing code that relies on passing &str slices.

@idavis
Copy link
Collaborator

idavis commented Feb 3, 2022

@cgranade - @SamarSha has started updating these to use impl AsRef<Path> which allows for &str to be passed. /home/iadavis/dev/pyqir/pyqir-jit/src/jit.rs:run_module_file for example.

@idavis
Copy link
Collaborator

idavis commented Feb 17, 2022

@cgranade Closing this as all examples have been fixed in various PRs.

@idavis idavis closed this as completed Feb 17, 2022
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

No branches or pull requests

2 participants