Skip to content

Macros with lifetimes #241

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

Closed
ivanovaleksey opened this issue Mar 7, 2018 · 5 comments
Closed

Macros with lifetimes #241

ivanovaleksey opened this issue Mar 7, 2018 · 5 comments

Comments

@ivanovaleksey
Copy link
Contributor

ivanovaleksey commented Mar 7, 2018

Hello,

I would like to create JSON RPC method with a parameter's type of a struct with a borrowed data.
Something like

#[derive(Deserialize)]
pub struct NewRoom<'a> {
    #[serde(borrow)]
    pub label: Option<Cow<'a, str>>,
    // Or it could be just &str
    // pub label: Option<&'a str>
}

build_rpc_trait! {
    pub trait Rpc {
        type Metadata;

        #[rpc(meta, name = "room.create")]
        fn create<'a>(&self, NewRoom<'a>) -> Result<CreateResponse<'a>>;
    }
}

The problem is that current macro doesn't support functions with lifetimes.
I am not sure if it's possible to create macro with lifetimes at all but I found PR which seems to implemented it rust-lang/rust#46895
https://doc.rust-lang.org/unstable-book/language-features/macro-lifetime-matcher.html
Thought I don't know if it's already available to use and on which Rust version.

My idea is to use borrowed string instead of owned one to avoid extra-allocations.
As far as I understand Serde kindly allows us to do exactly that.

A possible alternative is not to use macro and define RPC server manually. Although we know that it is much less convenient (one should parse parameters and handle errors become very non ergonomic).

What do you think?

@ivanovaleksey
Copy link
Contributor Author

ivanovaleksey commented Mar 7, 2018

Oh, looks like there would be a problem.

I defined a method

fn create<'a>(params: Params, meta: Meta) -> Result<Value> {
    let conn = establish_connection!(meta.db_pool.unwrap());

    let req = params.parse::<CreateRequest>().unwrap();

    let room: models::Room = diesel::insert_into(rooms::table)
        .values(&req.data)
        .get_result(conn).unwrap();

    let resp = CreateResponse::new(&room);

    Ok(serde_json::to_value(resp).unwrap())
}

But I got an error

error[E0279]: the requirement `for<'de> 'de : ` is not satisfied (`expected bound lifetime parameter 'de, found concrete lifetime`)
  --> src/rpc/mod.rs:62:22
   |
62 |     let req = params.parse::<CreateRequest>().unwrap();
   |                      ^^^^^
   |
   = note: required because of the requirements on the impl of `for<'de> serde::Deserialize<'de>` for `messages::room::CreateRequest<'_>`
   = note: required because of the requirements on the impl of `serde::de::DeserializeOwned` for `messages::room::CreateRequest<'_>`

I guess it is because of how Params::parse is defined.

So, it is impossible to parse Params into a type that borrows?

@tomusdrw
Copy link
Contributor

tomusdrw commented Mar 8, 2018

Yeah, unfortunately the whole library assumes that we own the objects being serialzed/deserialized. It would require changes on all layers to support that.

@ivanovaleksey
Copy link
Contributor Author

Thanks for the reply @tomusdrw.
I guess I can close the issue.
Just for my understanding - is it really worth making those changes?

@tomusdrw
Copy link
Contributor

I think it makes sense on the core/servers level to support borrowed data, not so much for macros unless we implement #66

@tomusdrw
Copy link
Contributor

Closing since it probably would be pretty substantial.

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