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][FRONTEND] refactoring builder to use a derive #2304

Closed
ehsanmok opened this issue Dec 18, 2018 · 2 comments
Closed

[RUST][FRONTEND] refactoring builder to use a derive #2304

ehsanmok opened this issue Dec 18, 2018 · 2 comments

Comments

@ehsanmok
Copy link
Contributor

Use derive_builder for tvm_frontend::Function for less boilerplate codes. Proper config is required to make the return buffer accepts at most one value. TODO #2292.

@ehsanmok
Copy link
Contributor Author

ehsanmok commented Dec 19, 2018

@nhynes don't think #[derive(Builder)]] can add the proper fields in builder struct needed for Function. It is merely for very common type of builders. Currently, derive_builder only uses the fields of the underlying struct, but the rather minimal signature of the current function::Builder is

pub struct Builder<'a> {
    pub func: Option<Function>,
    pub arg_buf: Option<Box<[TVMArgValue<'a>]>>,
    pub ret_buf: Option<Box<[TVMRetValue]>>,
}

with completely different fields, in particular arg_buf and ret_buf. So my conclusion is that the derive_builder is not mature enough to handle our case.

Side note: In my code, I've pretty much followed std::process::Command construct.

@ehsanmok
Copy link
Contributor Author

Discussed here and will plan for runtime-frontend compatibility.

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

1 participant