Clean up pointer use in BundleSpawner/BundleInserter#12269
Clean up pointer use in BundleSpawner/BundleInserter#12269james7132 merged 9 commits intobevyengine:mainfrom
Conversation
james7132
added
A-ECS
|
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file. |
james7132
force-pushed
the
cleanup-bundles
branch
from
550eb5b to
2162a77
Compare
james-j-obrien
left a comment
james-j-obrien
left a comment
There was a problem hiding this comment.
Looks reasonable to me.
joseph-gio
left a comment
joseph-gio
left a comment
There was a problem hiding this comment.
Definitely seems like an improvement
I wonder if it would make more sense for bevy_ptr's non-null API to look more like a typed version of bevy_ptr::Ptr
pub struct NonNull<'a, T> {
ptr: NonNull<T>,
_ref: PhantomData<&'a T>,
}
pub struct NonNullMut<'a, T> {
ptr: NonNull<T>,
_ref: PhantomData<&'a mut T>,
}This should allow us to enforce at compile time that the data is initialized and aligned and valid for reads/writes, so to dereference it you'd just need to guarantee there's no aliasing
Isn't that just |
james7132
added
the
S-Ready-For-Final-Review
I see it as an &'a mut T with the aliasing requirement lifted -- but I guess that's just |
|
CI is failing still <3 |
# Objective Following bevyengine#10756, we're now using raw pointers in BundleInserter and BundleSpawner. This is primarily to get around the need to split the borrow on the World, but it leaves a lot to be desired in terms of safety guarantees. There's no type level guarantee the code can't dereference a null pointer, and it's restoring them to borrows fairly liberally within the associated functions. ## Solution * Replace the pointers with `NonNull` and a new `bevy_ptr::ConstNonNull` that only allows conversion back to read-only borrows * Remove the closure to avoid potentially aliasing through the closure by restructuring the match expression. * Move all conversions back into borrows as far up as possible to ensure that the borrow checker is at least locally followed.
5 participants
Objective
Following #10756, we're now using raw pointers in BundleInserter and BundleSpawner. This is primarily to get around the need to split the borrow on the World, but it leaves a lot to be desired in terms of safety guarantees. There's no type level guarantee the code can't dereference a null pointer, and it's restoring them to borrows fairly liberally within the associated functions.
Solution
NonNulland a newbevy_ptr::ConstNonNullthat only allows conversion back to read-only borrows