Converging a payload from two different projections

[zbraniecki]

For #519 I'm using the following manual projections to allow for patterns to pattern projection:

    /// Helper struct used to allow for projection of `DataPayload<DatePatternsV1>` to
    /// `DataPayload<PatternV1>`.
    pub struct PatternFromPatternsV1Marker;

    impl<'data> DataMarker<'data> for PatternFromPatternsV1Marker {
        type Yokeable = PatternV1<'static>;
        type Cart = DatePatternsV1<'data>;
    }

Now I have a case where I need to project from a different payload to the same result payload and store that result on DateTimeFormat.

This means I need PatternFromPatternsV1Marker and PatternFromSkeletonsV1Marker and I'd like to end up with a single payload type that wraps PatternV1.

How to do that?

[zbraniecki]

@Manishearth @sffc

[Manishearth]

Honestly I'm not very happy with the concept of RcStruct (which is why we need Cart here); I suspect that's the core issue here. It's worth checking if we can avoid needing it.

[sffc]

I replied here:

#1150 (comment)

What @Manishearth said is correct. We can't coalesce the two projections to the same type because we carry a cart around. This is because we support 3 different ownership models in DataPayload:

1. Fully-owned
2. Borrowed from structured data
3. Borrowed from unstructured data

The "cart" type parameter feeds case 2; it allows the client to supply structured data and borrow from it.

To resolve the issue, we could:

1. Fix Zibi's case as a one-off by, e.g., specifying a custom cart type to be used by both PatternsV1 and SkeletonsV1 such that they can have a projection into the same type and everyone is happy
2. Clone the pattern from the skeletons as 'static since the skeletons are a slower code path anyway and may soon be obsolete
3. Remove the structured data variant from DataPayload (and replace it with what? just drop it?)

As stated in the other thread, I suggest we do option 2 for now, and keep this issue to discuss when we have more time and aren't in a crunch.

[zbraniecki]

I'm not sure how (2) is solving it. How can I get a static pattern into DataPayload<'data, PatternFromPatternsV1Marker>?

[Manishearth]

• Fix Zibi's case as a one-off by, e.g., specifying a custom cart type to be used by both PatternsV1 and SkeletonsV1 such that they can have a projection into the same type and everyone is happy

Yeah, additionally a thing to point out is that the Cart type is basically irrelevant except for its destructor; it doesn't do anything otherwise. So using an enum here just means you have a slightly more complex destructor.

I'm not sure how (2) is solving it. How can I get a static pattern into DataPayload<'data, PatternFromPatternsV1Marker>?

DataPayload::from_owned()

[sffc]

So using an enum here just means you have a slightly more complex destructor.

Can we project from Yoke<P1,C1> to Yoke<P2,C2> where C1 can be transformed into C2 (e.g. an enum variant)?

[Manishearth]

I don't think there's a way to do that safely because yoke won't know what part of the cart is borrowed and if it was preserved. That said we might be able to introduce an unsafe CartFrom trait that you implement between C1 and C2.

I'm still not convinced that RcStruct is worth it, or that it makes sense given the design of Yoke. It introduces another associated type, and all we use that type for is its destructor, while it constrains us. If we really want something like RcStruct I'd prefer we used RcSelf(M::Yokeable, Rc<dyn Any + 'data>).

[sffc]

Rc<dyn Any + 'data>

How would you make such a type? The dyn Any trait object requires 'static, and then you might as well use the Owned variant of Yoke.

[Manishearth]

Oh, actually it can be any object safe trait, I picked Any because dyn Drop doesn't work, but we could literally impl DynCart for T {} with zero methods and be fine because vtables contain destructors. There are no restrictions on dyn DynCart.

Yoke would need a constructor that handles this but it would be a simple cast.

[Manishearth]

Discussion of this solution (and a bigger one) in #1262