Skip to content

Conversation

@antiguru
Copy link
Member

Remove the requirement that all Container implementations are Clone and 'static. This makes implementing types simpler that depend on Container, and requires us to explicitly mark various places as Data such that they comply with Timely's type requirements.

@frankmcsherry
Copy link
Member

I started to look at this, and one of the first complaints from rust about removing Clone is the number of RefOrMut::swap calls we have. These are potentially an antipattern with containers, in that we'll likely be expecting to read a bunch of references out rather than owned contents, and cloning the container (and contents) while close to mandatory for Ref variants normally, probably isn't what we want with containers.

Probably not a big deal at the moment in that I think most Ref variants are e.g. abomonation related, and that's a different antipattern. But we might want to have a think about the right way to encourage good behavior here, and whether we are even doing the right thing ourselves.

Remove the requirement that all Container implementations are Clone and
'static. This makes implementing types simpler that depend on Container,
and requires us to explicitly mark various places as `Data` such that they
comply with Timely's type requirements.

Signed-off-by: Moritz Hoffmann <[email protected]>
@frankmcsherry frankmcsherry merged commit b411957 into TimelyDataflow:master Nov 25, 2024
7 checks passed
@github-actions github-actions bot mentioned this pull request Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants