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

references #194

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

references #194

wants to merge 12 commits into from

Conversation

Emilgardis
Copy link
Member

No description provided.

@Emilgardis Emilgardis added the enhancement New feature or request label Aug 5, 2021
@Emilgardis
Copy link
Member Author

GAT? 👀

@Emilgardis
Copy link
Member Author

Emilgardis commented Sep 4, 2022

this mostly works, only getting one error at call site for req_get (which is hopefully not hiding more errors)

error[E0308]: mismatched types
   --> examples/get_user.rs:38:20
    |
38  |     let response = client.req_get(req, &token).await.unwrap();
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
    |
    = note: expected struct `Vec<twitch_api::helix::users::User<'y>>`
               found struct `Vec<twitch_api::helix::users::User<'_>>`
note: the lifetime requirement is introduced here
   --> /Users/emil/workspace/twitch_api/src/helix/client.rs:113:28
    |
113 |         for<'y> R: Request<Response<'y> = D> + RequestGet,
    |                            ^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0308`.
error: could not compile `twitch_api` due to previous error

the library works though, you just cant get the response

i'm not very familiar with this type of error, I think I need to make different responses but it's unclear.

Maybe it's a bug even

bors bot added a commit that referenced this pull request Oct 30, 2022
280: refactor: Use (mostly) References in Requests r=Emilgardis a=Nerixyz

Since #194 only implemented a few references and a lot of changes happened in the meantime, I went ahead and tried to refactor (most) request fields to use references. "_most_" in this case means that (1) `after`/`cursor`s remain owned (since I couldn't get pagination to work) and (2) eventsub requests remain owned.

I think having (mostly) non-owned requests is already a big step.

* Some fields are `&str` or `&types::TypeRef` but should be `Cow<'_, str>` for the `Deserialize` impl (since they might be encoded as escaped characters in JSON). These are mainly user-inputs. user-ids, user-logins, UUIDs and similar can remain, since they aren't escaped in JSON. I think it's fair to make sure `Deserialize` works for JSON, but it shouldn't need to work for _all_ encodings.
* The use of `Cow<'_, [&types::TypeRef]>` makes passing (static) slices a bit ugly, because `&["foo"]` is `&[&str; 1]` and `&["foo"][..]` is `&[&str]` and `Cow`'s `Into` only accepts the latter.

Co-authored-by: Nerixyz <nerixdev@outlook.de>
Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
@Emilgardis
Copy link
Member Author

❯ cargo check --no-default-features -F helix,reqwest
    Blocking waiting for file lock on build directory
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error: implementation of `helix::_::_serde::Deserialize` is not general enough
  --> src/helix/client/client_ext.rs:37:9
   |
37 |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough
   |

error[E0277]: the trait bound `<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output: helix::_::_serde::Deserialize<'_>` is not satisfied
   --> src/helix/request.rs:431:42
    |
431 |         let response: InnerResponse<_> = parse_json(response, true).map_err(|e| {
    |                                          ^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output`
    |
note: required for `InnerResponse<<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output>` to implement `for<'d> helix::_::_serde::Deserialize<'d>`
   --> src/helix/mod.rs:66:21
    |
66  | #[derive(PartialEq, Deserialize, Debug)]
    |                     ^^^^^^^^^^^
67  | struct InnerResponse<D> {
    |        ^^^^^^^^^^^^^^^^
note: required by a bound in `parse_json`
   --> src/lib.rs:276:26
    |
276 | pub fn parse_json<'a, T: for<'d> serde::Deserialize<'d>>(
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `parse_json`
    = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider further restricting the associated type
    |
429 |             serde::Deserialize<'r>, <<Self as helix::request::Request>::Response as Yokeable<'r>>::Output: helix::_::_serde::Deserialize<'_>
    |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0277`.
error: could not compile `twitch_api` due to 2 previous errors

Now getting this, and not sure how to proceed...

```rust
❯ cargo +stable check --no-default-features -F helix,reqwest
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error[E0277]: the trait bound `<D as Yokeable<'_>>::Output: helix::_::_serde::Deserialize<'_>` is not satisfied
   --> src/helix/client.rs:156:41
    |
156 |                 } = <R>::parse_response(Some(request), &uri, &response)?;
    |                     ------------------- ^^^^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<D as Yokeable<'_>>::Output`
    |                     |
    |                     required by a bound introduced by this call
    |
note: required by a bound in `helix::request::RequestGet::parse_response`
   --> src/helix/request.rs:396:57
    |
389 |     fn parse_response<'r>(
    |        -------------- required by a bound in this
...
396 |         <Self::Response as yoke::Yokeable<'r>>::Output: serde::Deserialize<'r>,
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `helix::request::RequestGet::parse_response`
help: consider further restricting the associated type
    |
130 |         C: Send, <D as Yokeable<'_>>::Output: helix::_::_serde::Deserialize<'_>
    |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0277`.
error: could not compile `twitch_api` due to previous error
```
```rust
❯ cargo +stable check --no-default-features -F helix,reqwest
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error: implementation of `helix::_::_serde::Deserialize` is not general enough
  --> src/helix/client/client_ext.rs:37:9
   |
37 |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough
   |
   = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `helix::_::_serde::Deserialize<'0>`, for any lifetime `'0`...
   = note: ...but it actually implements `helix::_::_serde::Deserialize<'1>`, for some specific lifetime `'1`
   --> src/helix/request.rs:429:42
    |
429 |         let response: InnerResponse<_> = parse_json(response, true).map_err(|e| {
    |                                          ^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output`
    |
note: required for `InnerResponse<<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output>` to implement `helix::_::_serde::Deserialize<'_>`
   --> src/helix/mod.rs:66:21
    |
66  | #[derive(PartialEq, Deserialize, Debug)]
    |                     ^^^^^^^^^^^
67  | struct InnerResponse<D> {
    |        ^^^^^^^^^^^^^^^^
note: required by a bound in `parse_json`
   --> src/lib.rs:276:26
    |
276 | pub fn parse_json<'a, T: serde::Deserialize<'a>>(
    |                          ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `parse_json`
    = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider further restricting the associated type
    |
427 |         for<'d> yoke::trait_hack::YokeTraitHack<<Self::Response as yoke::Yokeable<'d>>::Output>: serde::Deserialize<'d>, <<Self as helix::request::Request>::Response as Yokeable<'r>>::Output: helix::_::_serde::Deserialize<'_>
    |                                                                                                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0277`.
error: could not compile `twitch_api` due to 2 previous errors
```
@Emilgardis
Copy link
Member Author

and now

❯ cargo +stable check --no-default-features -F helix,reqwest
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error: implementation of `helix::_::_serde::Deserialize` is not general enough
  --> src/helix/client/client_ext.rs:37:9
   |
37 |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough
   |
   = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `helix::_::_serde::Deserialize<'0>`, for any lifetime `'0`...
   = note: ...but it actually implements `helix::_::_serde::Deserialize<'1>`, for some specific lifetime `'1`
   --> src/helix/request.rs:429:42
    |
429 |         let response: InnerResponse<_> = parse_json(response, true).map_err(|e| {
    |                                          ^^^^^^^^^^ the trait `helix::_::_serde::Deserialize<'_>` is not implemented for `<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output`
    |
note: required for `InnerResponse<<<Self as helix::request::Request>::Response as Yokeable<'r>>::Output>` to implement `helix::_::_serde::Deserialize<'_>`
   --> src/helix/mod.rs:66:21
    |
66  | #[derive(PartialEq, Deserialize, Debug)]
    |                     ^^^^^^^^^^^
67  | struct InnerResponse<D> {
    |        ^^^^^^^^^^^^^^^^
note: required by a bound in `parse_json`
   --> src/lib.rs:276:26
    |
276 | pub fn parse_json<'a, T: serde::Deserialize<'a>>(
    |                          ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `parse_json`
    = note: this error originates in the derive macro `Deserialize` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider further restricting the associated type
    |
427 |         for<'d> yoke::trait_hack::YokeTraitHack<<Self::Response as yoke::Yokeable<'d>>::Output>: serde::Deserialize<'d>, <<Self as helix::request::Request>::Response as Yokeable<'r>>::Output: helix::_::_serde::Deserialize<'_>
    |                                                                                                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0277`.
error: could not compile `twitch_api` due to 2 previous errors

@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 16, 2022

and almost there!

❯ cargo +stable check --no-default-features -F helix,reqwest,yoke/serde
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error[E0521]: borrowed data escapes outside of closure
   --> src/helix/client.rs:154:37
    |
146 |             |body| -> Result<_, ClientRequestError<<C as crate::HttpClient<'a>>::Error>> {
    |              ----
    |              |
    |              `body` is a reference that is only valid in the closure body
    |              has type `&'1 [u8]`
...
154 |                 }: Response<_, _> = request.parse(&uri, &response).unwrap();
    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                                     |
    |                                     `body` escapes the closure body here
    |                                     argument requires that `'1` must outlive `'static`

For more information about this error, try `rustc --explain E0521`.
error: could not compile `twitch_api` due to previous error

not sure how to solve this, or what the issue actually is.

I think this is why, I might be wrong:

    pub async fn req_get<'req, R, T>(
        &'a self,
        // 1. R is Foo<'foo>
        request: R, 
        token: &T,
    ) -> Result<
        Response<R, yoke::Yoke<R::Response<'static>, Vec<u8>>>,
        ClientRequestError<<C as crate::HttpClient<'a>>::Error>,
    >
    where
        R: Request + RequestGet, // Here, due to something
        // 2. Because R::Response is Yokable only when R::Response<'static>
        //    R::Response<'de> in the later call becomes 'static  
        for<'y> R::Response<'static>: yoke::Yokeable<'y>, 
        T: TwitchToken + ?Sized,
        C: Send,
    { ... }

or I'm doing something totally wrong

```
error: implementation of `helix::_::_serde::Deserialize` is not general enough
  --> src/helix/client/client_ext.rs:37:9
   |
37 |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `helix::_::_serde::Deserialize` is not general enough
   |
   = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `helix::_::_serde::Deserialize<'0>`, for any lifetime `'0`...
   = note: ...but it actually implements `helix::_::_serde::Deserialize<'1>`, for some specific lifetime `'1`
```
@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 16, 2022

So, I feel like the current code is the most correct.

❯ cargo +stable check --no-default-features -F helix,reqwest,yoke/serde
    Checking twitch_api v0.7.0-rc.2 (/Users/emil/workspace/twitch_api)
error: implementation of `serde::Deserialize` is not general enough
  --> src/helix/client/client_ext.rs:37:9
   |
37 |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `serde::Deserialize` is not general enough
   |
   = note: `YokeTraitHack<Vec<get_users::User<'static>>>` must implement `serde::Deserialize<'0>`, for any lifetime `'0`...
   = note: ...but it actually implements `serde::Deserialize<'1>`, for some specific lifetime `'1`

error: could not compile `twitch_api` due to previous error

I think this is because

fn req_get()
where:
    for<'y> R::Response: yoke::Yokeable<'y>,
    for<'y> yoke::trait_hack::YokeTraitHack<<R::Response as yoke::Yokeable<'y>>::Output>:
            serde::Deserialize<'y>,

doesn't say

for specific lifetime 'y, where 'y is tied to yoke::Yokeable<'y>, yoke::Yokeable<'y>::Output: Deserialize<'y>

but instead says (for some reason)

for any lifetime 'y, where yoke::Yokeable<'_>::Output: Deserialize<'y>

is this a bug?

might be rust-lang/rust#70263

I might minimize this a bit and then (unless I solve it myself) see if the icu4x people have any ideas

@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 16, 2022

This is basically the same issue as we got for #284, but this case should be solveable as we control all surfaces

@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 16, 2022

Side note:

Making the bound

fn req_get()
where:
    for<'y> R::Response: yoke::Yokeable<'y>,
    for<'y> <R::Response as yoke::Yokeable<'y>>::Output:
        serde::Deserialize<'y>,

gives the following error instead, (interestingly, it's also malformed, no error: line)

it does not seem to be the same as rust-lang/rust#89196 which is what YokeTraitHack is used for

❯ cargo +stable check --no-default-features -F helix,reqwest,yoke/serde
   --> src/helix/client/client_ext.rs:37:22
    |
37  |         self.req_get(helix::users::GetUsersRequest::ids(&[id.into()][..]), token)
    |              ------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `for<'y> serde::Deserialize<'y>` is not implemented for `<_ as Yokeable<'y>>::Output`
    |              |
    |              required by a bound introduced by this call
    |
    = help: the following other types implement trait `serde::Deserialize<'de>`:
              &'a AccessTokenRef
              &'a BadgeSetIdRef
              &'a BlockedTermIdRef
              &'a CategoryIdRef
              &'a CharityCampaignIdRef
              &'a ChatBadgeIdRef
              &'a ClientIdRef
              &'a ClientSecretRef
            and 266 others
note: required by a bound in `HelixClient::<'a, C>::req_get`
   --> src/helix/client.rs:128:13
    |
116 |     pub async fn req_get<'req, R, T>(
    |                  ------- required by a bound in this
...
128 |             serde::Deserialize<'y>,
    |             ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `HelixClient::<'a, C>::req_get`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `twitch_api` due to previous error

@Emilgardis
Copy link
Member Author

Ehh, turns out it actually does work as is, it's just the Vec<T> that is problematic... Nice! But now I need to figure out how to solve that. (and I'm also not sure why it's not working)

@Emilgardis
Copy link
Member Author

Emilgardis commented Dec 18, 2022

this works, but it sucks... imo it's very cluttered...

#[zerovec::make_varule(ExampleResponseULE)]
#[zerovec::derive(Deserialize)]
#[derive(serde::Deserialize, yoke::Yokeable, PartialEq, Eq, PartialOrd, Ord)]
pub struct ExampleResponse<'a> {
    #[serde(borrow)]
    pub a_thing: Cow<'a, str>,
}

impl Request for ExampleRequest {
    type Response = zerovec::VarZeroVec<'static, ExampleResponseULE>;
}

impl RequestGet for Example

and the make_varule macro can not automatically get the ULE of our custom braids, so I'd either have to write my own implementation or do it manually for every response.
I also don't like how cluttered this feels, there has to be a better way, or I might do this in another way.

Maybe an arena on the client? feels like overkill.

Might be best to just make it possible to do this, but in a different way, either self-referential or something else.

example

impl Response<R,D> {
    fn data(self) -> Result<R::Response, _>  {
        <R>::parse_response_no_fail(data)
    }
	
	// `Output` is yoke::Yokable::Output
    fn data_ref<'a>(&'a self) -> Result<R::Response::Output<'a>, _>  {
        <R>::parse_response_ref_no_fail(data, uri, self.data)
    }
}
// Owned
let user: User<'static> = client.get_user_by_login("twitchdev").await?.data()?;
// Borrowed
let resp = client.get_user_by_login("twitchdev").await?;
let user: User<'_> = resp.data_ref()?;

we might hit the same issues though with the Vec Deserialize impl.

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

Successfully merging this pull request may close these issues.

References on all the things
1 participant