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

Deriving Decode with nested Cows #431

Closed
andrenth opened this issue Nov 5, 2021 · 5 comments
Closed

Deriving Decode with nested Cows #431

andrenth opened this issue Nov 5, 2021 · 5 comments

Comments

@andrenth
Copy link
Contributor

andrenth commented Nov 5, 2021

Hello

Apologies if this isn't the proper medium to ask this. I'm trying something similar to the code below:

use bincode::{de::Decode, enc::Encode, Decode, Encode};
use std::borrow::Cow;
  
#[derive(Encode, Decode)]
struct T<'a, A: Clone + Encode + Decode> { 
    t: Cow<'a, U<'a, A>>,
}
  
#[derive(Clone, Encode, Decode)]
struct U<'a, A: Clone + Encode + Decode> { 
    u: Cow<'a, A>,
}

which gives me the following error:

error[E0277]: the trait bound `U<'a, A>: Decode` is not satisfied
  --> src/main.rs:4:18
   |
4  | #[derive(Encode, Decode)]
   |                  ^^^^^^ the trait `Decode` is not implemented for `U<'a, A>`
   |
   = note: required because of the requirements on the impl of `Decode` for `Cow<'_, U<'a, A>>`
   = note: required because of the requirements on the impl of `BorrowDecode<'_>` for `Cow<'_, U<'a, A>>`
note: required by a bound in `borrow_decode`
  --> /home/andre/.cargo/registry/src/github.ghproxy.top-1ecc6299db9ec823/bincode-2.0.0-alpha.0/src/de/mod.rs:34:25
   |
34 |     fn borrow_decode<D: BorrowDecoder<'de>>(decoder: D) -> Result<Self, DecodeError>;
   |                         ^^^^^^^^^^^^^^^^^^ required by this bound in `borrow_decode`
   = note: this error originates in the derive macro `Decode` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.

The code works if one of the Cows is removed. Is it possible to derive(Decode) in the nested case?

@VictorKoenders
Copy link
Contributor

Ah, because U has a lifetime 'a, bincode_derive tries to be smart and implement BorrowDecode instead of Decode.

This is the code that is generated:

impl < '__de : 'a, 'a, A : Clone + Encode + Decode > bincode :: de ::
BorrowDecode < '__de > for U < 'a, A >
{
    fn borrow_decode < D : bincode :: de :: BorrowDecoder < '__de > >
    (mut decoder : D) -> core :: result :: Result < Self, bincode :: error ::
    DecodeError >
    {
        Ok(Self
           {
               u : bincode :: de :: BorrowDecode ::
               borrow_decode(& mut decoder) ?,
           })
    }
}

However Cow<U> is only implemented for U: Decode, but U implements BorrowDecode.

We've been considering making #[derive(BorrowDecode)] separate from #[derive(Decode)]. I think this issue might give us the reason to actually implement this.

For now you can solve this by making a manual implementation of Decode for U:

impl<'a, A: Clone + Encode + Decode> bincode::de::Decode for U<'a, A> {
    fn decode<D: bincode::de::Decoder>(
        mut decoder: D,
    ) -> core::result::Result<Self, bincode::error::DecodeError> {
        Ok(Self {
            u: bincode::de::Decode::decode(&mut decoder)?,
        })
    }
}

@VictorKoenders
Copy link
Contributor

Also Decode is currently implemented for Cow<T> where T: Decode, I think it should be implemented for Cow<T> where <T as ToOwned>::Owned: Decode.

In an ideal world we'd

impl BorrowDecode for Cow<T> where T: BorrowDecode {
  // return Cow::Borrowed
}

and if T doesn't implement BorrowDecode we'd fall back to

impl Decode for Cow<T> where <T as ToOwned>::Decode {
    // return Cow::Owned
}

But implementation specialization seems to be a long way from being stabilized

VictorKoenders added a commit that referenced this issue Nov 7, 2021
@VictorKoenders
Copy link
Contributor

Can you try #432 and see if that fixes your issue? I added tests/issues/issue_431.rs which contains your situation

VictorKoenders added a commit that referenced this issue Nov 7, 2021
* split off BorrowDecode from Decode in bincode_derive

* Added test case for issue #431

* Fixed Cow implementation having the wrong constraint, added BlockedTODO for cow implementation specialization

* Re-exported the Decode and Encode traits in the crate root

* Removed outdated comments

* Removed some ::de::Decode that were introduced by the merge
@andrenth
Copy link
Contributor Author

andrenth commented Nov 7, 2021

I will give it a try tomorrow.

@andrenth
Copy link
Contributor Author

andrenth commented Nov 8, 2021

It worked for me. Thanks!

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

No branches or pull requests

2 participants