From: Joshua Simmons Date: Sun, 2 Oct 2022 13:09:07 +0000 (+0200) Subject: Fix remaining clippy lints X-Git-Url: https://git.nega.tv//gitweb.cgi?a=commitdiff_plain;h=70b7b691bddaf7ef42f5d10d0f8384ea2b4d0e16;p=josh%2Fnarcissus Fix remaining clippy lints --- diff --git a/narcissus-core/src/fixed_vec.rs b/narcissus-core/src/fixed_vec.rs index d6f20ca..7de170f 100644 --- a/narcissus-core/src/fixed_vec.rs +++ b/narcissus-core/src/fixed_vec.rs @@ -92,6 +92,12 @@ impl FixedVec { self.buf.as_mut_ptr() as _ } + /// Set the length of the vec to `new_len` + /// + /// # Safety + /// + /// - `new_len` must be less than or equal to [`capacity()`]. + /// - The elements at `old_len..new_len` must be initialized. #[inline] pub unsafe fn set_len(&mut self, new_len: usize) { debug_assert!(new_len <= self.capacity()); diff --git a/narcissus-core/src/manual_arc.rs b/narcissus-core/src/manual_arc.rs index 266a6e7..3041ebc 100644 --- a/narcissus-core/src/manual_arc.rs +++ b/narcissus-core/src/manual_arc.rs @@ -82,11 +82,10 @@ impl ManualArc { // barrier on the slow path here to ensure all modifications to inner are visible before we call drop. std::sync::atomic::fence(Ordering::Acquire); - let value; // Safety: Was created by Box::leak in the constructor, so it's valid to re-create a box here. let mut inner = Box::from_raw(ptr.as_ptr()); // extract the value from the container so we can return it. - value = ManuallyDrop::take(&mut inner.value); + let value = ManuallyDrop::take(&mut inner.value); // since the contained value is wrapped in `ManuallyDrop` it won't be dropped here. drop(inner); diff --git a/narcissus-core/src/mutex.rs b/narcissus-core/src/mutex.rs index 603695e..23cb92e 100644 --- a/narcissus-core/src/mutex.rs +++ b/narcissus-core/src/mutex.rs @@ -106,7 +106,7 @@ impl Mutex { self.data.get_mut() } - pub unsafe fn raw_lock(&self) { + unsafe fn raw_lock(&self) { #[cfg(debug_assertions)] if self.thread_id.load(Ordering::Relaxed) == get_thread_id() { panic!("recursion not supported") @@ -165,7 +165,7 @@ impl Mutex { } } - pub unsafe fn raw_try_lock(&self) -> bool { + unsafe fn raw_try_lock(&self) -> bool { #[cfg(debug_assertions)] if self.thread_id.load(Ordering::Relaxed) == get_thread_id() { panic!("recursion not supported") @@ -185,7 +185,7 @@ impl Mutex { } } - pub unsafe fn raw_unlock(&self) { + unsafe fn raw_unlock(&self) { #[cfg(debug_assertions)] self.thread_id.store(0, Ordering::Relaxed); diff --git a/narcissus-core/src/pool.rs b/narcissus-core/src/pool.rs index f28820f..2ef5528 100644 --- a/narcissus-core/src/pool.rs +++ b/narcissus-core/src/pool.rs @@ -435,7 +435,7 @@ impl Pool { mapping_size += MAX_CAPACITY * size_of::(); mapping_size = align_offset(mapping_size, PAGE_SIZE); - let mapping_base = unsafe { virtual_reserve(mapping_size) }; + let mapping_base = virtual_reserve(mapping_size); let free_slots = unsafe { mapping_base.add(free_slots_offset) } as _; let slots = unsafe { mapping_base.add(slots_offset) } as _; let value_slots = unsafe { mapping_base.add(value_slots_offset) } as _; diff --git a/narcissus-core/src/ref_count.rs b/narcissus-core/src/ref_count.rs index 413973b..5408b35 100644 --- a/narcissus-core/src/ref_count.rs +++ b/narcissus-core/src/ref_count.rs @@ -119,6 +119,11 @@ impl Rc { } } + /// # Safety + /// + /// Any other [`Rc`] or [`Arc`] pointers to the same allocation must not be dereferenced for the duration of the + /// returned borrow. This is trivially the case if no such pointers exist, for example immediately after + /// [`Arc::new`]. #[inline] pub unsafe fn get_mut_unchecked(&mut self) -> &mut T { // We are careful to *not* create a reference covering the "count" fields, as @@ -235,6 +240,11 @@ impl Arc { } } + /// # Safety + /// + /// Any other [`Rc`] or [`Arc`] pointers to the same allocation must not be dereferenced for the duration of the + /// returned borrow. This is trivially the case if no such pointers exist, for example immediately after + /// [`Arc::new`]. pub unsafe fn get_mut_unchecked(&mut self) -> &mut T { // We are careful to *not* create a reference covering the "count" fields, as // this would alias with concurrent access to the reference counts. diff --git a/narcissus-core/src/virtual_mem.rs b/narcissus-core/src/virtual_mem.rs index 16faa9c..8a51535 100644 --- a/narcissus-core/src/virtual_mem.rs +++ b/narcissus-core/src/virtual_mem.rs @@ -1,22 +1,47 @@ use crate::libc; +/// Reserve a virtual memory range. +/// +/// Size will be rounded up to align with the system's page size. +/// +/// The range is valid but inaccessible before calling `virtual_commit`. +/// +/// # Panics +/// +/// Panics if mapping fails. #[cold] #[inline(never)] -pub unsafe fn virtual_reserve(size: usize) -> *mut std::ffi::c_void { - let ptr = libc::mmap( - std::ptr::null_mut(), - size, - libc::PROT_NONE, - libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, - -1, - 0, - ); +pub fn virtual_reserve(size: usize) -> *mut std::ffi::c_void { + let ptr = unsafe { + libc::mmap( + std::ptr::null_mut(), + size, + libc::PROT_NONE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; assert!(ptr != libc::MAP_FAILED && !ptr.is_null()); ptr } +/// Commit (part of) a previously reserved memory range. +/// +/// Marks the range as readable and writable. +/// +/// Size will be rounded up to align with the system's page size. +/// +/// # Safety +/// +/// - Must point to an existing assignment created by [`virtual_reserve`]. +/// - size must be within range of that reservation. +/// +/// # Panics +/// +/// Panics if changing page permissions for the range fails. #[cold] #[inline(never)] pub unsafe fn virtual_commit(ptr: *mut std::ffi::c_void, size: usize) { @@ -24,6 +49,16 @@ pub unsafe fn virtual_commit(ptr: *mut std::ffi::c_void, size: usize) { assert!(result == 0); } +/// Release a reserved or comitted virtual memory range. +/// +/// # Safety +/// +/// - Must point to an existing assignment created by [`virtual_reserve`]. +/// - `size` must be within range of that reservation. +/// +/// # Panics +/// +/// Panics if the range could not be unmapped. #[cold] #[inline(never)] pub unsafe fn virtual_free(ptr: *mut std::ffi::c_void, size: usize) { diff --git a/narcissus-gpu/src/vulkan.rs b/narcissus-gpu/src/vulkan.rs index 9ecef9d..5167788 100644 --- a/narcissus-gpu/src/vulkan.rs +++ b/narcissus-gpu/src/vulkan.rs @@ -12,7 +12,7 @@ use narcissus_core::{ cstr, default, manual_arc, manual_arc::ManualArc, Mutex, PhantomUnsend, Pool, }; -use vk::{DeviceFunctions, SurfaceKHRFunctions, SwapchainKHRFunctions}; +use vk::DeviceFunctions; use vulkan_sys as vk; use crate::{ @@ -308,6 +308,13 @@ struct VulkanFrame { recycled_semaphores: Mutex>, } +type SwapchainDestroyQueue = DelayQueue<( + Window, + vk::SwapchainKHR, + vk::SurfaceKHR, + Box<[vk::ImageView]>, +)>; + pub(crate) struct VulkanDevice<'app> { app: &'app dyn App, @@ -325,14 +332,7 @@ pub(crate) struct VulkanDevice<'app> { frames: Box<[UnsafeCell; NUM_FRAMES]>, swapchains: Mutex>, - destroyed_swapchains: Mutex< - DelayQueue<( - Window, - vk::SwapchainKHR, - vk::SurfaceKHR, - Box<[vk::ImageView]>, - )>, - >, + destroyed_swapchains: Mutex, pools: Mutex, semaphores: Mutex>, @@ -409,17 +409,21 @@ impl<'app> VulkanDevice<'app> { _physical_device_properties_13, ) = { let mut properties_13 = vk::PhysicalDeviceVulkan13Properties::default(); - let mut properties_12 = vk::PhysicalDeviceVulkan12Properties::default(); - let mut properties_11 = vk::PhysicalDeviceVulkan11Properties::default(); - let mut properties = vk::PhysicalDeviceProperties2::default(); - - properties._next = - &mut properties_11 as *mut vk::PhysicalDeviceVulkan11Properties as *mut _; - properties_11._next = - &mut properties_12 as *mut vk::PhysicalDeviceVulkan12Properties as *mut _; - properties_12._next = - &mut properties_13 as *mut vk::PhysicalDeviceVulkan13Properties as *mut _; - + let mut properties_12 = vk::PhysicalDeviceVulkan12Properties { + _next: &mut properties_13 as *mut vk::PhysicalDeviceVulkan13Properties + as *mut _, + ..default() + }; + let mut properties_11 = vk::PhysicalDeviceVulkan11Properties { + _next: &mut properties_12 as *mut vk::PhysicalDeviceVulkan12Properties + as *mut _, + ..default() + }; + let mut properties = vk::PhysicalDeviceProperties2 { + _next: &mut properties_11 as *mut vk::PhysicalDeviceVulkan11Properties + as *mut _, + ..default() + }; unsafe { instance_fn .get_physical_device_properties2(physical_device, &mut properties); @@ -434,16 +438,21 @@ impl<'app> VulkanDevice<'app> { physical_device_features_13, ) = { let mut features_13 = vk::PhysicalDeviceVulkan13Features::default(); - let mut features_12 = vk::PhysicalDeviceVulkan12Features::default(); - let mut features_11 = vk::PhysicalDeviceVulkan11Features::default(); - let mut features = vk::PhysicalDeviceFeatures2::default(); - - features._next = - &mut features_11 as *mut vk::PhysicalDeviceVulkan11Features as *mut _; - features_11._next = - &mut features_12 as *mut vk::PhysicalDeviceVulkan12Features as *mut _; - features_12._next = - &mut features_13 as *mut vk::PhysicalDeviceVulkan13Features as *mut _; + let mut features_12 = vk::PhysicalDeviceVulkan12Features { + _next: &mut features_13 as *mut vk::PhysicalDeviceVulkan13Features + as *mut _, + ..default() + }; + let mut features_11 = vk::PhysicalDeviceVulkan11Features { + _next: &mut features_12 as *mut vk::PhysicalDeviceVulkan12Features + as *mut _, + ..default() + }; + let mut features = vk::PhysicalDeviceFeatures2 { + _next: &mut features_11 as *mut vk::PhysicalDeviceVulkan11Features + as *mut _, + ..default() + }; unsafe { instance_fn.get_physical_device_features2(physical_device, &mut features); @@ -782,17 +791,19 @@ impl<'app> VulkanDevice<'app> { } fn destroy_swapchain( - app: &dyn App, - device_fn: &DeviceFunctions, - swapchain_fn: &SwapchainKHRFunctions, - surface_fn: &SurfaceKHRFunctions, - instance: vk::Instance, - device: vk::Device, + &self, window: Window, surface: vk::SurfaceKHR, swapchain: vk::SwapchainKHR, image_views: &[vk::ImageView], ) { + let app = self.app; + let device_fn = &self.device_fn; + let swapchain_fn = &self.swapchain_fn; + let surface_fn = &self.surface_fn; + let instance = self.instance; + let device = self.device; + if !image_views.is_empty() { for &image_view in image_views { unsafe { device_fn.destroy_image_view(device, image_view, None) } @@ -2050,18 +2061,7 @@ impl<'driver> Device for VulkanDevice<'driver> { self.destroyed_swapchains .lock() .expire(|(window, swapchain, surface, image_views)| { - Self::destroy_swapchain( - self.app, - device_fn, - &self.swapchain_fn, - &self.surface_fn, - self.instance, - device, - window, - surface, - swapchain, - &image_views, - ); + self.destroy_swapchain(window, surface, swapchain, &image_views); }); frame_token @@ -2197,21 +2197,15 @@ impl<'app> Drop for VulkanDevice<'app> { unsafe { device_fn.destroy_semaphore(device, *semaphore, None) } } - for (_, (window, swapchain, surface, image_views)) in - self.destroyed_swapchains.get_mut().drain(..) { - Self::destroy_swapchain( - self.app, - &self.device_fn, - &self.swapchain_fn, - &self.surface_fn, - self.instance, - self.device, - window, - surface, - swapchain, - &image_views, - ); + let destroyed_swapchains = self + .destroyed_swapchains + .get_mut() + .drain(..) + .collect::>(); + for (_, (window, swapchain, surface, image_views)) in destroyed_swapchains { + self.destroy_swapchain(window, surface, swapchain, &image_views); + } } for (_, swapchain) in self.swapchains.get_mut().iter() {