Remove KeyOwned
#498
Remove KeyOwned
#498
Conversation
frankmcsherry
force-pushed
the
remove_keyowned
branch
from
e81362a to
2fbb39b
Compare
| @@ -68,27 +69,28 @@ use differential_dataflow::consolidation::{consolidate, consolidate_updates}; | |||
| /// Notice that the time is hoisted up into data. The expectation is that | |||
| /// once out of the "delta flow region", the updates will be `delay`d to the | |||
| /// times specified in the payloads. | |||
| pub fn half_join<G, V, R, Tr, FF, CF, DOut, S>( | |||
| stream: &Collection<G, (Tr::KeyOwned, V, G::Timestamp), R>, | |||
| pub fn half_join<G, K, V, R, Tr, FF, CF, DOut, S>( | |||
There was a problem hiding this comment.
Is it correct that the K is also implied by the IntoOwned implementation? If yes, would it be possible to remove the type parameter? Just asking because it still shows up in many places, but better than what we had before, but I can imagine that Rust has a hard time to figure out the associated types.
There was a problem hiding this comment.
I think generally the issue is that we need to assert that there is one type that all lifetimes can be taken to. Each implements IntoOwned, but nothing ensures that there is a common target. If we could say for example for<'a, 'b> Key<'a>::Owned = Key<'b>::Owned or something like, I think you could avoid the K. So, it's less that it is a degree of freedom, and more that it is the rallying point for the common type.
|
Thanks! We can discuss the arg further, but yes I think better than we have before, and less ambiguous for values (for which |
This PR removes the
KeyOwnedassociated type, replacing it with generic parameters that GATs must implementIntoOwnedinto. These requirements are meant to be relaxed to those moments where something owned is needed, and they can potentially be avoided entirely for logic that does not required owned types.The PR also tidies the
Val<'_>GATs to follow the same idiom, where they previously required a function to map toV.