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

Invalid BorrowDecode derive for a struct with a Cow<'a, [Foo]> field #631

Open
MoSal opened this issue Apr 9, 2023 · 5 comments
Open

Invalid BorrowDecode derive for a struct with a Cow<'a, [Foo]> field #631

MoSal opened this issue Apr 9, 2023 · 5 comments

Comments

@MoSal
Copy link

MoSal commented Apr 9, 2023

use std::borrow::Cow;
use bincode::{Encode, Decode};

#[derive(Debug, Clone, Encode, Decode)]
pub enum Foo {
    A,
    B,
}

#[derive(Debug, Decode, Encode)]
pub(crate) struct FooList<'a>(Cow<'a, [Foo]>);

fn main() {
}

This fails due to the generated:

impl<'__de, 'a> ::bincode::BorrowDecode<'__de> for FooList<'a>
where
    '__de: 'a,
{
    fn borrow_decode<__D: ::bincode::de::BorrowDecoder<'__de>>(
        decoder: &mut __D,
    ) -> core::result::Result<Self, ::bincode::error::DecodeError> {
        Ok(Self {
            0: ::bincode::BorrowDecode::borrow_decode(decoder)?,
        })
    }
}

The error is obviously:

error[E0277]: the trait bound `&[Foo]: BorrowDecode<'_>` is not satisfied

This is due to the assumption that all Cow<'cow, T> uses will have a T where &'cow T: BorrowDecode<'cow> holds?!

@VictorKoenders
Copy link
Contributor

Can you try the lifetimes that are used in this test? https://github.com/bincode-org/bincode/blob/trunk/tests/issues/issue_431.rs

We should improve the ergonomics if this is the case

@MoSal
Copy link
Author

MoSal commented Apr 10, 2023

Yes. Adding this line above FooList's definition fixes the issue:

#[bincode(borrow_decode_bounds = "&'__de [Foo]: ::bincode::de::BorrowDecode<'__de> + '__de, '__de: 'a")]

But, it happens to be that the real FooList in my code looks like this:

#[derive(Debug, Decode, Encode)]
pub(crate) struct FooList(Cow<'static, [Foo]>);

So a way to pass custom bounds in this case (or a way to skip the generation of BorrowDecode) is still needed. But that's maybe a separate issue.

@VictorKoenders
Copy link
Contributor

It might be easier for you to manually implement decode. You can check target/generated/bincode/FooList_Decode.rs for what the derive generates.

Cow is a bit of an annoying structure for how bincode is currently set up and I'm not sure how to best deal with this in the derive logic.

At the least I think bincode should properly document that anything involving Cow is weird

@MoSal
Copy link
Author

MoSal commented Apr 11, 2023

It might be easier for you to manually implement decode. You can check target/generated/bincode/FooList_Decode.rs for what the derive generates.

Thanks for the suggestion. I actually already did that using good old cargo expand ;)

I removed the BorrowDecode impls first before reporting the issue, and kept the Decode ones.

Then I added the BorrowDecode ones back with the suggested working bounds.

@MoSal
Copy link
Author

MoSal commented Apr 11, 2023

The situation of borrow_decode_bounds being ignored if a type has no lifetime param, when a BorrowDecode is going to be generated anyway, is still worth a separate consideration IMHO.

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