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

Implement working Clone and PartialEq on DataPayload and Yoke #753

Closed
sffc opened this issue Jun 3, 2021 · 2 comments · Fixed by #895
Closed

Implement working Clone and PartialEq on DataPayload and Yoke #753

sffc opened this issue Jun 3, 2021 · 2 comments · Fixed by #895
Assignees
Labels
C-data-infra Component: provider, datagen, fallback, adapters question Unresolved questions; type unclear T-core Type: Required functionality
Milestone

Comments

@sffc
Copy link
Member

sffc commented Jun 3, 2021

Currently, the Clone impl is present for Yoke and DataPayload, but it does not work when one tries to use it. The error looks like this:

note: the following trait bounds were not satisfied:
      `<hello_world::HelloWorldV1<'static> as Yokeable<'a>>::Output: Clone`

This seems to be caused by rust-lang/rust#85636. However, the workaround mentioned there does not work for Clone, since the Clone impl for a reference copies the reference instead of cloning the struct that the reference points to.

PartialEq is a related but separate issue. I have the following impl, but it doesn't compile:

impl<'d, 's, M> PartialEq for DataPayload<'d, 's, M>
where
    M: DataMarker<'s>,
    for<'a> &'a <M::Yokeable as Yokeable<'a>>::Output: PartialEq,
{
    fn eq(&self, other: &Self) -> bool {
        self.get().eq(other)
    }
}

Error message:

error[E0308]: mismatched types
   --> provider/core/src/data_provider.rs:165:23
    |
165 |         self.get().eq(other)
    |                       ^^^^^ expected reference, found struct `data_provider::DataPayload`
    |
    = note: expected reference `&&<<M as marker::DataMarker<'s>>::Yokeable as Yokeable<'_>>::Output`
               found reference `&data_provider::DataPayload<'d, 's, M>`

I played around with different variations of & and * on the arguments to get them to match up, to no success. My guess is that the compiler is just confused about the lifetimes, since the lifetime of .get() is different than the lifetime of other.

CC @Manishearth

@sffc sffc added question Unresolved questions; type unclear T-core Type: Required functionality C-data-infra Component: provider, datagen, fallback, adapters discuss Discuss at a future ICU4X-SC meeting labels Jun 3, 2021
@sffc sffc removed the discuss Discuss at a future ICU4X-SC meeting label Jun 4, 2021
@sffc sffc added this to the 2021 Q3-m1 milestone Jun 4, 2021
@Manishearth
Copy link
Member

I think we fixed this with yoke_trait_hack, yes? Can this issue be closed?

@sffc
Copy link
Member Author

sffc commented Jul 14, 2021

It's implemented for Yoke, but we still need to it for DataPayload I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-data-infra Component: provider, datagen, fallback, adapters question Unresolved questions; type unclear T-core Type: Required functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants