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

Remove Collect + Sized on Rootable::Root type. #102

Merged
merged 2 commits into from
Jun 30, 2024
Merged

Conversation

kyren
Copy link
Owner

@kyren kyren commented Jun 27, 2024

Since we are using Gc pointers for DynamicRootSet again, we need to relax the way the Rootable macro works so that it can work for Gc<dyn Trait> and Gc pointers which have been cast to other types.

This is a breaking change (ofc), but hopefully it should not cause too much churn?

We could add a trait alias for Sized + Collect Root types to make things much easier, but I think the only way to have an actually helpful trait alias is to use the very recently stable associated type bounds.

I think it's probably just not a big deal unless you do deep tricks with Rootable (like piccolo's Any).

This also removes the 'static bound on Rootable because absolutely nothing was using it (piccolo does, but it can add it itself).

Since we are using `Gc` pointers for `DynamicRootSet` again, we need to
relax the way the `Rootable` macro works so that it can work for `Gc<dyn
Trait>` and `Gc` pointers which have been cast to other types.
@kyren kyren marked this pull request as ready for review June 27, 2024 06:58
Comment on lines -459 to -461
let arena1: Arena<Rootable![DynamicRootSet<'_>]> = Arena::new(|mc| DynamicRootSet::new(mc));

let arena2: Arena<Rootable![DynamicRootSet<'_>]> = Arena::new(|mc| DynamicRootSet::new(mc));
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed here is that let arena: Arena<Rootable![MyRoot<'_>]> = Arena::new(|mc| ...); no longer works, but let arena = Arena::<Rootable![MyRoot<'_>]>::new(|mc| ...); does, and I have no idea why.

It's a very minor paper cut, but it'd be nice not to break it.

@kyren
Copy link
Owner Author

kyren commented Jun 27, 2024

Fixing piccolo after this was not a big deal in the slightest, I don't know where my head was when I was testing #53.

@kyren kyren merged commit 2f67d9f into master Jun 30, 2024
1 check passed
@kyren kyren deleted the relaxed_fit_rootable branch July 8, 2024 20:45
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.

1 participant