Cargo.lock 🔗
@@ -7390,6 +7390,7 @@ dependencies = [
"parking",
"parking_lot",
"pathfinder_geometry",
+ "pin-project",
"postage",
"pretty_assertions",
"profiling",
gpui: Remove some unnecessary unsafe code (#40483)
Lukas Wirth created
Release Notes:
- N/A *or* Added/Fixed/Improved ...Change summary
Cargo.lock | 1
crates/gpui/Cargo.toml | 1
crates/gpui/src/app/entity_map.rs | 9 -
crates/gpui/src/app/test_context.rs | 4
crates/gpui/src/arena.rs | 120 +++++++++++++-----------------
crates/gpui/src/executor.rs | 4
crates/gpui/src/platform.rs | 2
crates/gpui/src/taffy.rs | 11 ++
crates/gpui/src/util.rs | 13 +-
crates/gpui/src/window.rs | 5 -
10 files changed, 77 insertions(+), 93 deletions(-)
Detailed changes
crates/gpui/Cargo.toml 🔗
@@ -135,6 +135,7 @@ waker-fn = "1.2.0"
lyon = "1.0"
workspace-hack.workspace = true
libc.workspace = true
+pin-project = "1.1.10"
[target.'cfg(target_os = "macos")'.dependencies]
block = "0.1"
crates/gpui/src/app/entity_map.rs 🔗
@@ -378,11 +378,9 @@ pub struct Entity<T> {
#[deref]
#[deref_mut]
pub(crate) any_entity: AnyEntity,
- pub(crate) entity_type: PhantomData<T>,
+ pub(crate) entity_type: PhantomData<fn(T) -> T>,
}
-unsafe impl<T> Send for Entity<T> {}
-unsafe impl<T> Sync for Entity<T> {}
impl<T> Sealed for Entity<T> {}
impl<T: 'static> Entity<T> {
@@ -657,7 +655,7 @@ pub struct WeakEntity<T> {
#[deref]
#[deref_mut]
any_entity: AnyWeakEntity,
- entity_type: PhantomData<T>,
+ entity_type: PhantomData<fn(T) -> T>,
}
impl<T> std::fmt::Debug for WeakEntity<T> {
@@ -669,9 +667,6 @@ impl<T> std::fmt::Debug for WeakEntity<T> {
}
}
-unsafe impl<T> Send for WeakEntity<T> {}
-unsafe impl<T> Sync for WeakEntity<T> {}
-
impl<T> Clone for WeakEntity<T> {
fn clone(&self) -> Self {
Self {
crates/gpui/src/app/test_context.rs 🔗
@@ -888,7 +888,9 @@ impl VisualTestContext {
// safety: on_quit will be called after the test has finished.
// the executor will ensure that all tasks related to the test have stopped.
// so there is no way for cx to be accessed after on_quit is called.
- let cx = Box::leak(unsafe { Box::from_raw(ptr) });
+ // todo: This is unsound under stacked borrows (also tree borrows probably?)
+ // the mutable reference invalidates `ptr` which is later used in the closure
+ let cx = unsafe { &mut *ptr };
cx.on_quit(move || unsafe {
drop(Box::from_raw(ptr));
});
crates/gpui/src/arena.rs 🔗
@@ -15,9 +15,7 @@ struct ArenaElement {
impl Drop for ArenaElement {
#[inline(always)]
fn drop(&mut self) {
- unsafe {
- (self.drop)(self.value);
- }
+ unsafe { (self.drop)(self.value) };
}
}
@@ -40,33 +38,29 @@ impl Drop for Chunk {
impl Chunk {
fn new(chunk_size: NonZeroUsize) -> Self {
- unsafe {
- // this only fails if chunk_size is unreasonably huge
- let layout = alloc::Layout::from_size_align(chunk_size.get(), 1).unwrap();
- let start = alloc::alloc(layout);
- if start.is_null() {
- handle_alloc_error(layout);
- }
- let end = start.add(chunk_size.get());
- Self {
- start,
- end,
- offset: start,
- }
+ // this only fails if chunk_size is unreasonably huge
+ let layout = alloc::Layout::from_size_align(chunk_size.get(), 1).unwrap();
+ let start = unsafe { alloc::alloc(layout) };
+ if start.is_null() {
+ handle_alloc_error(layout);
+ }
+ let end = unsafe { start.add(chunk_size.get()) };
+ Self {
+ start,
+ end,
+ offset: start,
}
}
fn allocate(&mut self, layout: alloc::Layout) -> Option<NonNull<u8>> {
- unsafe {
- let aligned = self.offset.add(self.offset.align_offset(layout.align()));
- let next = aligned.add(layout.size());
-
- if next <= self.end {
- self.offset = next;
- NonNull::new(aligned)
- } else {
- None
- }
+ let aligned = unsafe { self.offset.add(self.offset.align_offset(layout.align())) };
+ let next = unsafe { aligned.add(layout.size()) };
+
+ if next <= self.end {
+ self.offset = next;
+ NonNull::new(aligned)
+ } else {
+ None
}
}
@@ -122,54 +116,48 @@ impl Arena {
where
F: FnOnce() -> T,
{
- unsafe {
- ptr::write(ptr, f());
- }
+ unsafe { ptr::write(ptr, f()) };
}
unsafe fn drop<T>(ptr: *mut u8) {
- unsafe {
- std::ptr::drop_in_place(ptr.cast::<T>());
- }
+ unsafe { std::ptr::drop_in_place(ptr.cast::<T>()) };
}
- unsafe {
- let layout = alloc::Layout::new::<T>();
- let mut current_chunk = &mut self.chunks[self.current_chunk_index];
- let ptr = if let Some(ptr) = current_chunk.allocate(layout) {
+ let layout = alloc::Layout::new::<T>();
+ let mut current_chunk = &mut self.chunks[self.current_chunk_index];
+ let ptr = if let Some(ptr) = current_chunk.allocate(layout) {
+ ptr.as_ptr()
+ } else {
+ self.current_chunk_index += 1;
+ if self.current_chunk_index >= self.chunks.len() {
+ self.chunks.push(Chunk::new(self.chunk_size));
+ assert_eq!(self.current_chunk_index, self.chunks.len() - 1);
+ log::trace!(
+ "increased element arena capacity to {}kb",
+ self.capacity() / 1024,
+ );
+ }
+ current_chunk = &mut self.chunks[self.current_chunk_index];
+ if let Some(ptr) = current_chunk.allocate(layout) {
ptr.as_ptr()
} else {
- self.current_chunk_index += 1;
- if self.current_chunk_index >= self.chunks.len() {
- self.chunks.push(Chunk::new(self.chunk_size));
- assert_eq!(self.current_chunk_index, self.chunks.len() - 1);
- log::trace!(
- "increased element arena capacity to {}kb",
- self.capacity() / 1024,
- );
- }
- current_chunk = &mut self.chunks[self.current_chunk_index];
- if let Some(ptr) = current_chunk.allocate(layout) {
- ptr.as_ptr()
- } else {
- panic!(
- "Arena chunk_size of {} is too small to allocate {} bytes",
- self.chunk_size,
- layout.size()
- );
- }
- };
-
- inner_writer(ptr.cast(), f);
- self.elements.push(ArenaElement {
- value: ptr,
- drop: drop::<T>,
- });
-
- ArenaBox {
- ptr: ptr.cast(),
- valid: self.valid.clone(),
+ panic!(
+ "Arena chunk_size of {} is too small to allocate {} bytes",
+ self.chunk_size,
+ layout.size()
+ );
}
+ };
+
+ unsafe { inner_writer(ptr.cast(), f) };
+ self.elements.push(ArenaElement {
+ value: ptr,
+ drop: drop::<T>,
+ });
+
+ ArenaBox {
+ ptr: ptr.cast(),
+ valid: self.valid.clone(),
}
}
}
crates/gpui/src/executor.rs 🔗
@@ -525,9 +525,7 @@ where
"local task dropped by a thread that didn't spawn it. Task spawned at {}",
self.location
);
- unsafe {
- ManuallyDrop::drop(&mut self.inner);
- }
+ unsafe { ManuallyDrop::drop(&mut self.inner) };
}
}
crates/gpui/src/platform.rs 🔗
@@ -348,8 +348,6 @@ impl Debug for DisplayId {
}
}
-unsafe impl Send for DisplayId {}
-
/// Which part of the window to resize
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ResizeEdge {
crates/gpui/src/taffy.rs 🔗
@@ -69,9 +69,7 @@ impl TaffyLayoutEngine {
} else {
self.taffy
// This is safe because LayoutId is repr(transparent) to taffy::tree::NodeId.
- .new_with_children(taffy_style, unsafe {
- std::mem::transmute::<&[LayoutId], &[taffy::NodeId]>(children)
- })
+ .new_with_children(taffy_style, LayoutId::to_taffy_slice(children))
.expect(EXPECT_MESSAGE)
.into()
}
@@ -265,6 +263,13 @@ impl TaffyLayoutEngine {
#[repr(transparent)]
pub struct LayoutId(NodeId);
+impl LayoutId {
+ fn to_taffy_slice(node_ids: &[Self]) -> &[taffy::NodeId] {
+ // SAFETY: LayoutId is repr(transparent) to taffy::tree::NodeId.
+ unsafe { std::mem::transmute::<&[LayoutId], &[taffy::NodeId]>(node_ids) }
+ }
+}
+
impl std::hash::Hash for LayoutId {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
u64::from(self.0).hash(state);
crates/gpui/src/util.rs 🔗
@@ -83,8 +83,11 @@ impl<T: Future> FutureExt for T {
}
}
+#[pin_project::pin_project]
pub struct WithTimeout<T> {
+ #[pin]
future: T,
+ #[pin]
timer: Task<()>,
}
@@ -97,15 +100,11 @@ impl<T: Future> Future for WithTimeout<T> {
type Output = Result<T::Output, Timeout>;
fn poll(self: Pin<&mut Self>, cx: &mut task::Context) -> task::Poll<Self::Output> {
- // SAFETY: the fields of Timeout are private and we never move the future ourselves
- // And its already pinned since we are being polled (all futures need to be pinned to be polled)
- let this = unsafe { &raw mut *self.get_unchecked_mut() };
- let future = unsafe { Pin::new_unchecked(&mut (*this).future) };
- let timer = unsafe { Pin::new_unchecked(&mut (*this).timer) };
+ let this = self.project();
- if let task::Poll::Ready(output) = future.poll(cx) {
+ if let task::Poll::Ready(output) = this.future.poll(cx) {
task::Poll::Ready(Ok(output))
- } else if timer.poll(cx).is_ready() {
+ } else if this.timer.poll(cx).is_ready() {
task::Poll::Ready(Err(Timeout))
} else {
task::Poll::Pending
crates/gpui/src/window.rs 🔗
@@ -4650,7 +4650,7 @@ pub struct WindowHandle<V> {
#[deref]
#[deref_mut]
pub(crate) any_handle: AnyWindowHandle,
- state_type: PhantomData<V>,
+ state_type: PhantomData<fn(V) -> V>,
}
impl<V> Debug for WindowHandle<V> {
@@ -4786,9 +4786,6 @@ impl<V: 'static> From<WindowHandle<V>> for AnyWindowHandle {
}
}
-unsafe impl<V> Send for WindowHandle<V> {}
-unsafe impl<V> Sync for WindowHandle<V> {}
-
/// A handle to a window with any root view type, which can be downcast to a window with a specific root view type.
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
pub struct AnyWindowHandle {