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

Should Box<dyn ToSql> be an instance of ToSql? #500

Closed
DevJac opened this issue Mar 27, 2019 · 2 comments
Closed

Should Box<dyn ToSql> be an instance of ToSql? #500

DevJac opened this issue Mar 27, 2019 · 2 comments

Comments

@DevJac
Copy link

DevJac commented Mar 27, 2019

I had a function like fn my_value_to_sql(v: &MyValue) -> Box<dyn ToSql> which would parse a MyValue into one of several different types, but all the types implemented ToSql. I had to returned this Boxed since otherwise the size of the return value wouldn't have been known.

It would have been nice if Box<dyn ToSql> was an instance of ToSql.

(I'm fairly new to Rust, so it may be that my Box approach is misguided, but it's the best I could come up with.)

@thomcc
Copy link
Member

thomcc commented Mar 27, 2019

Returning a &dyn ToSql definitely should work (and will be more efficient than using a Box, although will impose more lifetime restrictions). I'd expect Box to work too, but since i know Box<dyn Error> doesn't implement std::error::Error, I could believe that I'm wrong.

@DevJac
Copy link
Author

DevJac commented Mar 27, 2019

You sure? ToSql is a trait; &dyn ToSql can be any of an infinite number of types. How can its size be known? Or am I mistaken that the size of a returned value must be known?

EDIT: Nevermind, of course a reference has a known size. I'm new at this. 😉
Still, I returned a Box<dyn ToSql> because I wanted to pass ownership back. I ended up turning those Boxes into &dyn ToSqls, but it would be nice if Box<dyn ToSql> was itself a ToSql instance.

thomcc pushed a commit to thomcc/rusqlite that referenced this issue Mar 27, 2019
@gwenn gwenn closed this as completed in d8074b1 Mar 28, 2019
gwenn added a commit that referenced this issue Mar 28, 2019
impl ToSql for Box<dyn ToSql>. Fixes #500
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