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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - some cleanup for bevy_ptr #4668

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - some cleanup for bevy_ptr #4668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9fe591
change as_ptr signature from self to &self for non-copy pointers
jakobhellermann May 5, 2022
f62ba15
make as_ptr safe
jakobhellermann May 5, 2022
81b9306
rename offset/add to byte_offset/byte_add
jakobhellermann May 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/storage/blob_vec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl BlobVec {
#[inline]
pub unsafe fn get_unchecked(&self, index: usize) -> Ptr<'_> {
debug_assert!(index < self.len());
self.get_ptr().add(index * self.item_layout.size())
self.get_ptr().byte_add(index * self.item_layout.size())
}

/// # Safety
Expand All @@ -221,7 +221,7 @@ impl BlobVec {
pub unsafe fn get_unchecked_mut(&mut self, index: usize) -> PtrMut<'_> {
debug_assert!(index < self.len());
let layout_size = self.item_layout.size();
self.get_ptr_mut().add(index * layout_size)
self.get_ptr_mut().byte_add(index * layout_size)
}

/// Gets a [`Ptr`] to the start of the vec
Expand Down Expand Up @@ -258,7 +258,7 @@ impl BlobVec {
unsafe {
// NOTE: this doesn't use self.get_unchecked(i) because the debug_assert on index
// will panic here due to self.len being set to 0
let ptr = self.get_ptr_mut().add(i * layout_size).promote();
let ptr = self.get_ptr_mut().byte_add(i * layout_size).promote();
(drop)(ptr);
}
}
Expand Down
22 changes: 5 additions & 17 deletions crates/bevy_ptr/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ macro_rules! impl_ptr {
///
/// [ptr_offset]: https://doc.rust-lang.org/std/primitive.pointer.html#method.offset
#[inline]
pub unsafe fn offset(self, count: isize) -> Self {
pub unsafe fn byte_offset(self, count: isize) -> Self {
Self(
NonNull::new_unchecked(self.as_ptr().offset(count)),
PhantomData,
Expand All @@ -73,7 +73,7 @@ macro_rules! impl_ptr {
///
/// [ptr_add]: https://doc.rust-lang.org/std/primitive.pointer.html#method.add
#[inline]
pub unsafe fn add(self, count: usize) -> Self {
pub unsafe fn byte_add(self, count: usize) -> Self {
Self(
NonNull::new_unchecked(self.as_ptr().add(count)),
PhantomData,
Expand Down Expand Up @@ -116,13 +116,9 @@ impl<'a> Ptr<'a> {
///
/// If possible, it is strongly encouraged to use [`deref`](Self::deref) over this function,
/// as it retains the lifetime.
///
/// # Safety
/// All subsequent operations to the returned pointer must be valid inside the
/// associated lifetime.
#[inline]
#[allow(clippy::wrong_self_convention)]
pub unsafe fn as_ptr(self) -> *mut u8 {
pub fn as_ptr(self) -> *mut u8 {
Copy link
Member

Choose a reason for hiding this comment

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

this isnt taking &self

Copy link
Member

Choose a reason for hiding this comment

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

it probably doesnt matter but idk if we want to be consistent here or not 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not needed here because Ptr: Copy so I kept it as is but it could also be &self for consistency. I don't have a strong opinion here.

self.0.as_ptr()
}
}
Expand Down Expand Up @@ -150,13 +146,9 @@ impl<'a> PtrMut<'a> {
///
/// If possible, it is strongly encouraged to use [`deref_mut`](Self::deref_mut) over
/// this function, as it retains the lifetime.
///
/// # Safety
/// All subsequent operations to the returned pointer must be valid inside the
/// associated lifetime.
#[inline]
#[allow(clippy::wrong_self_convention)]
pub unsafe fn as_ptr(self) -> *mut u8 {
pub fn as_ptr(&self) -> *mut u8 {
self.0.as_ptr()
}
}
Expand Down Expand Up @@ -192,13 +184,9 @@ impl<'a> OwningPtr<'a> {
///
/// If possible, it is strongly encouraged to use the other more type-safe functions
/// over this function.
///
/// # Safety
/// All subsequent operations to the returned pointer must be valid inside the
/// associated lifetime.
#[inline]
#[allow(clippy::wrong_self_convention)]
pub unsafe fn as_ptr(self) -> *mut u8 {
pub fn as_ptr(&self) -> *mut u8 {
self.0.as_ptr()
}
}
Expand Down