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

Queriable trait codegen breaks on explicit lifetimes #35

Closed
mcasper opened this Issue Nov 30, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@mcasper
Collaborator

mcasper commented Nov 30, 2015

Deriving Queriable for structs with explicit lifetimes blows up with the error message:

   Compiling diesel_test v0.1.0 (file:///Users/mattcasper/code/diesel_test)
src/main.rs:28:18: 28:20 error: use of undeclared lifetime name `'a` [E0261]
src/main.rs:28     first_name: &'a str,
                                ^~
src/main.rs:25:10: 25:19 note: in this expansion of #[derive_Queriable] (defined in src/main.rs)
src/main.rs:28:18: 28:20 help: run `rustc --explain E0261` to see a detailed explanation
src/main.rs:28:18: 28:20 error: use of undeclared lifetime name `'a` [E0261]
src/main.rs:28     first_name: &'a str,
                                ^~
src/main.rs:25:10: 25:19 note: in this expansion of #[derive_Queriable] (defined in src/main.rs)
src/main.rs:28:18: 28:20 help: run `rustc --explain E0261` to see a detailed explanation
error: aborting due to 2 previous errors

for the struct

#[derive(Queriable)]
pub struct User<'a> {
    id: i32,
    first_name: &'a str,
    last_name: String,
    email: String
}

And trying to implement the trait by hand got tricky, I got stuck on what $row_type evaluates to.

@sgrif

This comment has been minimized.

Member

sgrif commented Nov 30, 2015

We wouldn't be able to query this out, as there would be no lifetime for us to deserialize &'a str to. You must own the data.

@sgrif

This comment has been minimized.

Member

sgrif commented Nov 30, 2015

We should improve the error message here though

@reem

This comment has been minimized.

reem commented Nov 30, 2015

@sgrif on the other hand supporting lifetimes is important since there are types like Cow<'a, str> which could be used to allow borrowed data on the encoding end and would deserialize into a owned string of type Cow<'static, str>.

@sgrif

This comment has been minimized.

Member

sgrif commented Nov 30, 2015

You're right, that's a valid use case. (Just for the record, this was the code which I wrote to get my head straight)

use std::borrow::{Cow, ToOwned};
impl<'a, T: ?Sized, ST> ToSql<ST> for Cow<'a, T> where
    ST: NativeSqlType,
    T: 'a + ToOwned + ToSql<ST>,
    T::Owned: ToSql<ST>,
{
    fn to_sql<W: Write>(&self, out: &mut W) -> Result<IsNull, Box<Error>> {
        match self {
            &Cow::Borrowed(ref t) => t.to_sql(out),
            &Cow::Owned(ref t) => t.to_sql(out),
        }
    }
}

impl<'a, T: ?Sized, ST> FromSql<ST> for Cow<'a, T> where
    ST: NativeSqlType,
    T: 'a + ToOwned,
    T::Owned: FromSql<ST>,
{
    fn from_sql(bytes: Option<&[u8]>) -> Result<Self, Box<Error>> {
        T::Owned::from_sql(bytes).map(Cow::Owned)
    }
}
@sgrif

This comment has been minimized.

Member

sgrif commented Nov 30, 2015

I also need to properly support generic lifetimes for the other codegen as well, at the moment they just take advantage of the fact that you can have an unused lifetime name in an impl, and just put impl<'a> on them universally.

@reem

This comment has been minimized.

reem commented Nov 30, 2015

Does that actually compile? I would guess that in the FromSql implementation you'd need Cow<'static, T>.

It's very tricky to get bounds correct in codegen, even the built-in derives are overly restrictive in basically all non-straightforward cases.

@sgrif

This comment has been minimized.

Member

sgrif commented Nov 30, 2015

Yes, that code compiles. Because for any lifetime 'a, returning a 'static will satisfy it.

@reem

This comment has been minimized.

reem commented Nov 30, 2015

Ah, I see, that's a nice generalization.

@sgrif sgrif closed this in eef7af9 Nov 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment