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

Lifetime support? #21

Closed
mre opened this issue Aug 15, 2016 · 11 comments
Closed

Lifetime support? #21

mre opened this issue Aug 15, 2016 · 11 comments
Labels

Comments

@mre
Copy link
Contributor

mre commented Aug 15, 2016

Just tried using rust-derive-builder for a struct with lifetimes:

#[macro_use] extern crate custom_derive;
#[macro_use] extern crate derive_builder;

custom_derive! {
    #[derive(Default, Builder)]
    struct Channel<'a> {
        token: &'a str,
        special_info: i32,
        // .. a whole bunch of other fields ..
    }
}

impl Channel {
    // All that's left to do for you is writing a method,
    // which actually *does* something. :-)
    pub fn build(&self) -> String {
        format!("The meaning of life is {}.", self.special_info)
    }
}

fn main() {
    // builder pattern, go, go, go!...
    let x = Channel::default().special_info(42u8).build();
    println!("{:?}", x);
}

I get the following error:

6:22 error: expected ident, found 'a
src/main.rs:6     struct Channel<'a> {
                                 ^~
error: Could not compile `derive-test`.

Can lifetimes be easily added? If not, are there any workarounds?

@colin-kiegel
Copy link
Owner

Thanks for your feedback. I'll look into it when I find some spare time.
As a quick workaround you might use String and just copy the data for now.

@killercup
Copy link
Collaborator

Colin, I remember there were some issues with matching lifetimes in macros
(I think there is an RFC to add lifetime token types), but in this case
can we just match on <$generics:tt> and copy the whole thing?

Colin Kiegel notifications@github.com schrieb am Mo. 15. Aug. 2016 um
09:39:

Thanks for your feedback. I'll look into it when I find some spare time.
As a quick workaround you might use String and just copy the data for now.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#21 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABOX6zyY6Af_zd6wO_cU2ljxIvXLNFDks5qgBeXgaJpZM4JkKzT
.

@colin-kiegel
Copy link
Owner

Yeah, I remember the same thing, i.e. we can't match specifically on
lifetimes right now. Your suggestion is probably the only way to do it
right now. I am confident this will work, because when we match on fields
types later on, the lifetime there should just be part of the matched type.

On Mon, Aug 15, 2016 at 9:45 AM Pascal Hertleif notifications@github.com
wrote:

Colin, I remember there were some issues with matching lifetimes in macros
(I think there is an RFC to add lifetime token types), but in this case
can we just match on <$generics:tt> and copy the whole thing?

Colin Kiegel notifications@github.com schrieb am Mo. 15. Aug. 2016 um
09:39:

Thanks for your feedback. I'll look into it when I find some spare time.
As a quick workaround you might use String and just copy the data for
now.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<
#21 (comment)
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AABOX6zyY6Af_zd6wO_cU2ljxIvXLNFDks5qgBeXgaJpZM4JkKzT

.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#21 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHVbsgbhga3yqt2QmTHMgU9nM25M6kUtks5qgBkAgaJpZM4JkKzT
.

@colin-kiegel
Copy link
Owner

Hm, looks like I was to quick on this. I was sure I tested this on stable and beta, too - but travis proves me wrong... So this is working on nightly, but not on beta + stable yet.

https://travis-ci.org/colin-kiegel/rust-derive-builder/jobs/152447585#L253

Rust expects an identifier when expanding a generic struct, and we can't coerce a lifetime into an identifier. Too bad.

This looks like a bug which has been fixed in rustc 1.12, i.e. we have to wait for 2 releases. I'll revert the change and rebase my pull request (again) later.

@killercup: SORRY about the little mess ^^

@colin-kiegel colin-kiegel reopened this Aug 15, 2016
@colin-kiegel
Copy link
Owner

@mre: You are lucky - there is a workaround with derive_builder v0.1.0 and stable rust and it is not too bad actually. Just use a generic struct struct Channel<T> and substitute &'a YourType as T when you instantiate it. It's a bit longer than 'a but that is the only disadvantage I can see. :-)

A full example looks like this

#[macro_use] extern crate custom_derive;
#[macro_use] extern crate derive_builder;

custom_derive! {
    #[derive(Debug, Builder)]
    struct Channel<T> {
        token: T,
        special_info: i32,
        // .. a whole bunch of other fields ..
    }
}

impl<T> Channel<T> {
    pub fn new<X: Into<T>>(token: X) -> Self {
        Channel {
            token: token.into(),
            special_info: 0,
        }
    }
}

impl<T: ::std::fmt::Display> Channel<T> {
    // All that's left to do for you is writing a method,
    // which actually *does* something. :-)
    pub fn build(&self) -> String {
        format!("{}. The meaning of life is {}.", self.token, self.special_info)
    }
}

fn main() {
    let ch: Channel<&'static str> = Channel::new("Hello world!");
    let x: String  = ch.special_info(42u8).build();
    println!("{:?}", x);
}

@mre
Copy link
Contributor Author

mre commented Aug 16, 2016

@colin-kiegel thanks for that! I will try it out as soon as I get a chance.
Just out of curiosity: Does it support multiple lifetimes? I don't have a use-case for that right now, just want to learn a bit more.

@colin-kiegel
Copy link
Owner

@mre: Yes, the workaround supports multiple lifetimes, because the lifetime is hidden in a type parameter: You can both

  • use as many different type parameters as you want,
  • hide as many lifetimes within each type parameter as you want.

However, you can only access methods of your generic type, if they are defined in a trait and if you add the appropriate trait boundary (e.g. the boundary T: ::std::fmt::Display is neccessary to call Display::fmt() on T - implicit in the format! macro).

@mre
Copy link
Contributor Author

mre commented Aug 16, 2016

Nice to know. Thanks for your work. ☺️

@colin-kiegel
Copy link
Owner

reopening .. github didn't get the meaning of 'revert fix #21' ^^

Btw. the fix currently works on nightly and beta (=rustc 1.12). I.e. with the next stable release we should be able to roll it out. :-)

@nbigaouette
Copy link

What's the status on this? Can fda318a be used with rust 1.13?

nbigaouette added a commit to nbigaouette/rust-derive-builder that referenced this issue Dec 1, 2016
The patch on `src/derive_builder.rs` is quite straightforward, but the
test had to be adapted a little. Mainly, the `Lorem` struct had to derive
`Clone` and the created variable must call `clone()`, like it is done
in `tests/generic_structs.rs`.

See issue colin-kiegel#21.

Original commit: fda318a
Reverting commit: 5b44190.
@colin-kiegel
Copy link
Owner

v0.3.0 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants