Minimise unsafe block size, in library (#2592)

* Minimise unsafe block size, in library

* Missed one

Cargo.toml

@@ -83,11 +83,12 @@ rand = "0.8"
 ron = "0.8"
 
 [workspace.lints]
-rust.missing_docs = "allow"            # TODO: warn eventually
+rust.missing_docs = "allow"                                       # TODO: warn eventually
 rust.rust_2018_idioms = { level = "warn", priority = -1 }
 rust.rust_2024_compatibility = { level = "allow", priority = -1 } # TODO: warn eventually
 clippy.borrow_as_ptr = "warn"
-clippy.missing_safety_doc = "allow"    # TODO: warn eventually
+clippy.missing_safety_doc = "allow"                               # TODO: warn eventually
+clippy.multiple_unsafe_ops_per_block = "warn"
 clippy.ptr_as_ptr = "warn"
 clippy.ptr_cast_constness = "warn"
 # clippy.ref_as_ptr = "warn"                 # TODO: enable once it's stable

vulkano-taskgraph/src/command_buffer/commands/dynamic_state.rs

@@ -780,7 +780,7 @@ impl RecordingCommandBuffer<'_> {
         fragment_size: [u32; 2],
         combiner_ops: [FragmentShadingRateCombinerOp; 2],
     ) -> Result<&mut Self> {
-        unsafe { Ok(self.set_fragment_shading_rate_unchecked(fragment_size, combiner_ops)) }
+        Ok(unsafe { self.set_fragment_shading_rate_unchecked(fragment_size, combiner_ops) })
     }
 
     pub unsafe fn set_fragment_shading_rate_unchecked(

vulkano-taskgraph/src/graph/execute.rs

@@ -566,7 +566,8 @@ impl<W: ?Sized + 'static> ExecutableTaskGraph<W> {
                 ObjectType::Buffer => {
                     // SAFETY: The caller must ensure that `resource_map` maps the virtual IDs
                     // exhaustively.
-                    let state = unsafe { resource_map.buffer_unchecked(id.parametrize()) };
+                    let id_p = unsafe { id.parametrize() };
+                    let state = unsafe { resource_map.buffer_unchecked(id_p) };
                     let access = BufferAccess::from_masks(
                         access.stage_mask,
                         access.access_mask,
@@ -577,7 +578,8 @@ impl<W: ?Sized + 'static> ExecutableTaskGraph<W> {
                 ObjectType::Image => {
                     // SAFETY: The caller must ensure that `resource_map` maps the virtual IDs
                     // exhaustively.
-                    let state = unsafe { resource_map.image_unchecked(id.parametrize()) };
+                    let id_p = unsafe { id.parametrize() };
+                    let state = unsafe { resource_map.image_unchecked(id_p) };
                     let access = ImageAccess::from_masks(
                         access.stage_mask,
                         access.access_mask,
@@ -589,7 +591,8 @@ impl<W: ?Sized + 'static> ExecutableTaskGraph<W> {
                 ObjectType::Swapchain => {
                     // SAFETY: The caller must ensure that `resource_map` maps the virtual IDs
                     // exhaustively.
-                    let state = unsafe { resource_map.swapchain_unchecked(id.parametrize()) };
+                    let id_p = unsafe { id.parametrize() };
+                    let state = unsafe { resource_map.swapchain_unchecked(id_p) };
                     let access = ImageAccess::from_masks(
                         access.stage_mask,
                         access.access_mask,
@@ -1670,17 +1673,26 @@ impl<'a> ResourceMap<'a> {
 
             *slot = match physical_id.object_type() {
                 // SAFETY: We own an `epoch::Guard`.
-                ObjectType::Buffer => <*const _>::cast(unsafe {
-                    physical_resources.buffer_unprotected(physical_id.parametrize())
-                }?),
+                ObjectType::Buffer => {
+                    let physical_id_p = unsafe { physical_id.parametrize() };
+                    <*const _>::cast(unsafe {
+                        physical_resources.buffer_unprotected(physical_id_p)
+                    }?)
+                }
                 // SAFETY: We own an `epoch::Guard`.
-                ObjectType::Image => <*const _>::cast(unsafe {
-                    physical_resources.image_unprotected(physical_id.parametrize())
-                }?),
+                ObjectType::Image => {
+                    let physical_id_p = unsafe { physical_id.parametrize() };
+                    <*const _>::cast(unsafe {
+                        physical_resources.image_unprotected(physical_id_p)
+                    }?)
+                }
                 // SAFETY: We own an `epoch::Guard`.
-                ObjectType::Swapchain => <*const _>::cast(unsafe {
-                    physical_resources.swapchain_unprotected(physical_id.parametrize())
-                }?),
+                ObjectType::Swapchain => {
+                    let physical_id_p = unsafe { physical_id.parametrize() };
+                    <*const _>::cast(unsafe {
+                        physical_resources.swapchain_unprotected(physical_id_p)
+                    }?)
+                }
                 _ => unreachable!(),
             };
         }

vulkano-taskgraph/src/lib.rs

@@ -389,7 +389,8 @@ impl<'a> TaskContext<'a> {
         let mapped_slice = subbuffer.mapped_slice().unwrap();
 
         // SAFETY: The caller must ensure that access to the data is synchronized.
-        let data = unsafe { &*T::ptr_from_slice(mapped_slice) };
+        let data_ptr = unsafe { T::ptr_from_slice(mapped_slice) };
+        let data = unsafe { &*data_ptr };
 
         Ok(data)
     }
@@ -492,7 +493,8 @@ impl<'a> TaskContext<'a> {
         let mapped_slice = subbuffer.mapped_slice().unwrap();
 
         // SAFETY: The caller must ensure that access to the data is synchronized.
-        let data = unsafe { &mut *T::ptr_from_slice(mapped_slice) };
+        let data_ptr = unsafe { T::ptr_from_slice(mapped_slice) };
+        let data = unsafe { &mut *data_ptr };
 
         Ok(data)
     }

vulkano/src/acceleration_structure.rs

@@ -245,8 +245,8 @@ impl AccelerationStructure {
     pub fn device_address(&self) -> NonNullDeviceAddress {
         let info_vk = ash::vk::AccelerationStructureDeviceAddressInfoKHR::default()
             .acceleration_structure(self.handle);
+        let fns = self.device.fns();
         let ptr = unsafe {
-            let fns = self.device.fns();
             (fns.khr_acceleration_structure
                 .get_acceleration_structure_device_address_khr)(
                 self.device.handle(), &info_vk
@@ -260,8 +260,8 @@ impl AccelerationStructure {
 impl Drop for AccelerationStructure {
     #[inline]
     fn drop(&mut self) {
+        let fns = self.device.fns();
         unsafe {
-            let fns = self.device.fns();
             (fns.khr_acceleration_structure
                 .destroy_acceleration_structure_khr)(
                 self.device.handle(), self.handle, ptr::null()
@@ -917,13 +917,16 @@ impl AccelerationStructureGeometryTrianglesData {
             ])
         })?;
 
-        if unsafe {
-            !device
+        let format_properties = unsafe {
+            device
                 .physical_device()
                 .format_properties_unchecked(vertex_format)
-                .buffer_features
-                .intersects(FormatFeatures::ACCELERATION_STRUCTURE_VERTEX_BUFFER)
-        } {
+        };
+
+        if !format_properties
+            .buffer_features
+            .intersects(FormatFeatures::ACCELERATION_STRUCTURE_VERTEX_BUFFER)
+        {
             return Err(Box::new(ValidationError {
                 context: "vertex_format".into(),
                 problem: "format features do not contain \

vulkano/src/buffer/allocator.rs

@@ -182,7 +182,8 @@ where
     /// The next time you allocate a subbuffer, a new arena will be allocated with the new size,
     /// and all subsequently allocated arenas will also share the new size.
     pub fn set_arena_size(&self, size: DeviceSize) {
-        let state = unsafe { &mut *self.state.get() };
+        let state_ptr = self.state.get();
+        let state = unsafe { &mut *state_ptr };
         state.arena_size = size;
         state.arena = None;
         state.reserve = None;
@@ -195,7 +196,8 @@ where
     /// this has no effect.
     pub fn reserve(&self, size: DeviceSize) -> Result<(), MemoryAllocatorError> {
         if size > self.arena_size() {
-            let state = unsafe { &mut *self.state.get() };
+            let state_ptr = self.state.get();
+            let state = unsafe { &mut *state_ptr };
             state.arena_size = size;
             state.reserve = None;
             state.arena = Some(state.next_arena()?);
@@ -211,7 +213,9 @@ where
     {
         let layout = T::LAYOUT.unwrap_sized();
 
-        unsafe { &mut *self.state.get() }
+        let state_ptr = self.state.get();
+        let state = unsafe { &mut *state_ptr };
+        state
             .allocate(layout)
             .map(|subbuffer| unsafe { subbuffer.reinterpret_unchecked() })
     }

vulkano/src/buffer/mod.rs

@@ -405,12 +405,10 @@ impl Buffer {
         let allocation = unsafe { ResourceMemory::from_allocation(allocator, allocation) };
 
         // SAFETY: we just created this raw buffer and hasn't bound any memory to it.
-        let buffer = unsafe {
-            raw_buffer.bind_memory(allocation).map_err(|(err, _, _)| {
-                err.map(AllocateBufferError::BindMemory)
-                    .map_validation(|err| err.add_context("RawBuffer::bind_memory"))
-            })?
-        };
+        let buffer = unsafe { raw_buffer.bind_memory(allocation) }.map_err(|(err, _, _)| {
+            err.map(AllocateBufferError::BindMemory)
+                .map_validation(|err| err.add_context("RawBuffer::bind_memory"))
+        })?;
 
         Ok(Arc::new(buffer))
     }
@@ -472,7 +470,7 @@ impl Buffer {
     pub fn device_address(&self) -> Result<NonNullDeviceAddress, Box<ValidationError>> {
         self.validate_device_address()?;
 
-        unsafe { Ok(self.device_address_unchecked()) }
+        Ok(unsafe { self.device_address_unchecked() })
     }
 
     fn validate_device_address(&self) -> Result<(), Box<ValidationError>> {

vulkano/src/buffer/subbuffer.rs

@@ -377,7 +377,8 @@ where
         }
 
         // SAFETY: `Subbuffer` guarantees that its contents are laid out correctly for `T`.
-        let data = unsafe { &*T::ptr_from_slice(mapped_slice) };
+        let data_ptr = unsafe { T::ptr_from_slice(mapped_slice) };
+        let data = unsafe { &*data_ptr };
 
         Ok(BufferReadGuard {
             subbuffer: self,
@@ -464,7 +465,8 @@ where
         }
 
         // SAFETY: `Subbuffer` guarantees that its contents are laid out correctly for `T`.
-        let data = unsafe { &mut *T::ptr_from_slice(mapped_slice) };
+        let data_ptr = unsafe { T::ptr_from_slice(mapped_slice) };
+        let data = unsafe { &mut *data_ptr };
 
         Ok(BufferWriteGuard {
             subbuffer: self,
@@ -710,7 +712,7 @@ impl<T: ?Sized> Drop for BufferWriteGuard<'_, T> {
                 _ne: crate::NonExhaustive(()),
             };
 
-            unsafe { allocation.flush_range_unchecked(memory_range).unwrap() };
+            unsafe { allocation.flush_range_unchecked(memory_range) }.unwrap();
         }
 
         let mut state = self.subbuffer.buffer().state();

vulkano/src/buffer/sys.rs

@@ -58,7 +58,7 @@ impl RawBuffer {
     ) -> Result<Self, Validated<VulkanError>> {
         Self::validate_new(&device, &create_info)?;
 
-        unsafe { Ok(Self::new_unchecked(device, create_info)?) }
+        Ok(unsafe { Self::new_unchecked(device, create_info) }?)
     }
 
     fn validate_new(
@@ -195,33 +195,37 @@ impl RawBuffer {
         let mut memory_requirements2_vk =
             MemoryRequirements::to_mut_vk2(&mut memory_requirements2_extensions_vk);
 
-        unsafe {
-            let fns = device.fns();
+        let fns = device.fns();
 
-            if device.api_version() >= Version::V1_1
-                || device.enabled_extensions().khr_get_memory_requirements2
-            {
-                if device.api_version() >= Version::V1_1 {
+        if device.api_version() >= Version::V1_1
+            || device.enabled_extensions().khr_get_memory_requirements2
+        {
+            if device.api_version() >= Version::V1_1 {
+                unsafe {
                     (fns.v1_1.get_buffer_memory_requirements2)(
                         device.handle(),
                         &info_vk,
                         &mut memory_requirements2_vk,
-                    );
-                } else {
+                    )
+                };
+            } else {
+                unsafe {
                     (fns.khr_get_memory_requirements2
                         .get_buffer_memory_requirements2_khr)(
                         device.handle(),
                         &info_vk,
                         &mut memory_requirements2_vk,
-                    );
-                }
-            } else {
+                    )
+                };
+            }
+        } else {
+            unsafe {
                 (fns.v1_0.get_buffer_memory_requirements)(
                     device.handle(),
                     handle,
                     &mut memory_requirements2_vk.memory_requirements,
-                );
-            }
+                )
+            };
         }
 
         // Unborrow
@@ -556,10 +560,8 @@ impl Drop for RawBuffer {
     #[inline]
     fn drop(&mut self) {
         if self.needs_destruction {
-            unsafe {
-                let fns = self.device.fns();
-                (fns.v1_0.destroy_buffer)(self.device.handle(), self.handle, ptr::null());
-            }
+            let fns = self.device.fns();
+            unsafe { (fns.v1_0.destroy_buffer)(self.device.handle(), self.handle, ptr::null()) };
         }
     }
 }

vulkano/src/buffer/view.rs

@@ -71,7 +71,7 @@ impl BufferView {
         let subbuffer = subbuffer.into_bytes();
         Self::validate_new(&subbuffer, &create_info)?;
 
-        unsafe { Ok(Self::new_unchecked(subbuffer, create_info)?) }
+        Ok(unsafe { Self::new_unchecked(subbuffer, create_info) }?)
     }
 
     fn validate_new(
@@ -89,12 +89,9 @@ impl BufferView {
         let buffer = subbuffer.buffer();
         let properties = device.physical_device().properties();
 
-        let format_features = unsafe {
-            device
-                .physical_device()
-                .format_properties_unchecked(format)
-                .buffer_features
-        };
+        let format_properties =
+            unsafe { device.physical_device().format_properties_unchecked(format) };
+        let format_features = format_properties.buffer_features;
 
         if !buffer
             .usage()
@@ -290,18 +287,20 @@ impl BufferView {
         let device = subbuffer.device();
         let create_info_vk = create_info.to_vk(subbuffer.as_bytes());
 
-        let handle = unsafe {
-            let fns = device.fns();
+        let fns = device.fns();
+        let handle = {
             let mut output = MaybeUninit::uninit();
-            (fns.v1_0.create_buffer_view)(
-                device.handle(),
-                &create_info_vk,
-                ptr::null(),
-                output.as_mut_ptr(),
-            )
+            unsafe {
+                (fns.v1_0.create_buffer_view)(
+                    device.handle(),
+                    &create_info_vk,
+                    ptr::null(),
+                    output.as_mut_ptr(),
+                )
+            }
             .result()
             .map_err(VulkanError::from)?;
-            output.assume_init()
+            unsafe { output.assume_init() }
         };
 
         Ok(Self::from_handle(subbuffer, handle, create_info))
@@ -320,13 +319,13 @@ impl BufferView {
     ) -> Arc<BufferView> {
         let &BufferViewCreateInfo { format, _ne: _ } = &create_info;
         let size = subbuffer.size();
-        let format_features = unsafe {
+        let format_properties = unsafe {
             subbuffer
                 .device()
                 .physical_device()
                 .format_properties_unchecked(format)
-                .buffer_features
         };
+        let format_features = format_properties.buffer_features;
 
         Arc::new(BufferView {
             handle,
@@ -366,14 +365,14 @@ impl BufferView {
 impl Drop for BufferView {
     #[inline]
     fn drop(&mut self) {
+        let fns = self.subbuffer.device().fns();
         unsafe {
-            let fns = self.subbuffer.device().fns();
             (fns.v1_0.destroy_buffer_view)(
                 self.subbuffer.device().handle(),
                 self.handle,
                 ptr::null(),
-            );
-        }
+            )
+        };
     }
 }