Skip to content

Commit

Permalink
Replace pointer castings (as) by their API equivalent (#11818)
Browse files Browse the repository at this point in the history
# Objective

Since rust `1.76`,
[`ptr::from_ref`](https://doc.rust-lang.org/stable/std/ptr/fn.from_ref.html)
and
[`ptr::from_mut`](https://doc.rust-lang.org/stable/std/ptr/fn.from_mut.html)
are stable.

This PR replaces code that use `as` casting by one of `ptr::from_ref`,
`ptr::from_mut`, `cast_mut`, `cast_const`, or `cast` methods, which are
less error-prone.

## Solution

- Bump MSRV to `1.76.0`
- Enables the following clippy lints:
-
[`ptr_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#/ptr_as_ptr)
-
[`ptr_cast_constness`](https://rust-lang.github.io/rust-clippy/master/index.html#/ptr_cast_constness)
-
[`ref_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#/ref_as_ptr)
(I fix all warnings for this one, but it requires rust 1.77 to be
enabled)
- Fix the lints mentioned above
  • Loading branch information
tguichaoua authored Feb 11, 2024
1 parent 94ab84e commit c1a4e29
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 12 deletions.
7 changes: 6 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ license = "MIT OR Apache-2.0"
readme = "README.md"
repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy"
rust-version = "1.74.0"
rust-version = "1.76.0"

[workspace]
exclude = [
Expand Down Expand Up @@ -40,6 +40,11 @@ match_same_arms = "warn"
semicolon_if_nothing_returned = "warn"
map_flatten = "warn"

ptr_as_ptr = "warn"
ptr_cast_constness = "warn"
#TODO(rust 1.77): enable `ref_as_ptr`
# ref_as_ptr = "warn"

[workspace.lints.rust]
unsafe_op_in_unsafe_fn = "warn"
missing_docs = "warn"
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
#[cfg(feature = "trace")]
use bevy_utils::tracing::Span;
use fixedbitset::FixedBitSet;
use std::{any::TypeId, borrow::Borrow, fmt, mem::MaybeUninit};
use std::{any::TypeId, borrow::Borrow, fmt, mem::MaybeUninit, ptr};

use super::{
NopWorldQuery, QueryBuilder, QueryData, QueryEntityError, QueryFilter, QueryManyIter,
Expand Down Expand Up @@ -93,7 +93,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
>(
&self,
) -> &QueryState<NewD, NewF> {
&*(self as *const QueryState<D, F> as *const QueryState<NewD, NewF>)
&*ptr::from_ref(self).cast::<QueryState<NewD, NewF>>()
}

/// Returns the archetype components accessed by this query.
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/world/unsafe_world_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{
system::{Res, Resource},
};
use bevy_ptr::Ptr;
use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData};
use std::{any::TypeId, cell::UnsafeCell, fmt::Debug, marker::PhantomData, ptr};

/// Variant of the [`World`] where resource and component accesses take `&self`, and the responsibility to avoid
/// aliasing violations are given to the caller instead of being checked at compile-time by rust's unique XOR shared rule.
Expand Down Expand Up @@ -85,13 +85,13 @@ impl<'w> UnsafeWorldCell<'w> {
/// Creates a [`UnsafeWorldCell`] that can be used to access everything immutably
#[inline]
pub(crate) fn new_readonly(world: &'w World) -> Self {
UnsafeWorldCell(world as *const World as *mut World, PhantomData)
Self(ptr::from_ref(world).cast_mut(), PhantomData)
}

/// Creates [`UnsafeWorldCell`] that can be used to access everything mutably
#[inline]
pub(crate) fn new_mutable(world: &'w mut World) -> Self {
Self(world as *mut World, PhantomData)
Self(ptr::from_mut(world), PhantomData)
}

/// Gets a mutable reference to the [`World`] this [`UnsafeWorldCell`] belongs to.
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_mikktspace/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#![allow(
unsafe_op_in_unsafe_fn,
clippy::all,
clippy::undocumented_unsafe_blocks
clippy::undocumented_unsafe_blocks,
clippy::ptr_cast_constness
)]
// FIXME(3492): remove once docs are ready
#![allow(missing_docs)]
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ptr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ impl<'a, T> Copy for ThinSlicePtr<'a, T> {}
impl<'a, T> From<&'a [T]> for ThinSlicePtr<'a, T> {
#[inline]
fn from(slice: &'a [T]) -> Self {
let ptr = slice.as_ptr() as *mut T;
let ptr = slice.as_ptr().cast_mut();
Self {
// SAFETY: a reference can never be null
ptr: unsafe { NonNull::new_unchecked(ptr.debug_ensure_aligned()) },
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_utils/src/synccell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
//!
//! [`std::sync::Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html
use std::ptr;

/// See [`Exclusive`](https://github.com/rust-lang/rust/issues/98407) for stdlib's upcoming implementation,
/// which should replace this one entirely.
///
Expand Down Expand Up @@ -41,7 +43,7 @@ impl<T: ?Sized> SyncCell<T> {
/// to its inner value, to skip constructing with [`new()`](SyncCell::new()).
pub fn from_mut(r: &'_ mut T) -> &'_ mut SyncCell<T> {
// SAFETY: repr is transparent, so refs have the same layout; and `SyncCell` properties are `&mut`-agnostic
unsafe { &mut *(r as *mut T as *mut SyncCell<T>) }
unsafe { &mut *(ptr::from_mut(r) as *mut SyncCell<T>) }
}
}

Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_utils/src/syncunsafecell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
//! [`std::cell::SyncUnsafeCell`]: https://doc.rust-lang.org/nightly/std/cell/struct.SyncUnsafeCell.html
pub use core::cell::UnsafeCell;
use core::ptr;

/// [`UnsafeCell`], but [`Sync`].
///
Expand Down Expand Up @@ -72,13 +73,13 @@ impl<T: ?Sized> SyncUnsafeCell<T> {
// We can just cast the pointer from `SyncUnsafeCell<T>` to `T` because
// of #[repr(transparent)] on both SyncUnsafeCell and UnsafeCell.
// See UnsafeCell::raw_get.
this as *const T as *mut T
(this as *const T).cast_mut()
}

#[inline]
/// Returns a `&mut SyncUnsafeCell<T>` from a `&mut T`.
pub fn from_mut(t: &mut T) -> &mut SyncUnsafeCell<T> {
let ptr = t as *mut T as *mut SyncUnsafeCell<T>;
let ptr = ptr::from_mut(t) as *mut SyncUnsafeCell<T>;
// SAFETY: `ptr` must be safe to mutably dereference, since it was originally
// obtained from a mutable reference. `SyncUnsafeCell` has the same representation
// as the original type `T`, since the former is annotated with #[repr(transparent)].
Expand All @@ -105,7 +106,7 @@ impl<T> SyncUnsafeCell<[T]> {
// - `SyncUnsafeCell<T>` has the same layout as `T`
// - `SyncUnsafeCell<[T]>` has the same layout as `[T]`
// - `SyncUnsafeCell<[T]>` has the same layout as `[SyncUnsafeCell<T>]`
unsafe { &*(self as *const SyncUnsafeCell<[T]> as *const [SyncUnsafeCell<T>]) }
unsafe { &*(ptr::from_ref(self) as *const [SyncUnsafeCell<T>]) }
}
}

Expand Down

0 comments on commit c1a4e29

Please sign in to comment.