From 9bae276a37b29201f164e5c93611042e0979c5fc Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Wed, 5 Jan 2022 11:29:25 +0100 Subject: [PATCH] ash: Remove unnecessary trivial_casts and trivial_numeric_casts While making the code only marginally harder to read such casts can also introduce subtle bugs when used incorrectly, and are best omitted whenever unnecessary: Rust already coerces borrows into raw pointers when the types on both ends are clear, and even then there remain many casts that are identical to the source type. In addition these errors show up when using a local crate reference to `ash` in a workspace that uses "the `.cargo/config.toml` setup" from [EmbarkStudios/rust-ecosystem#68] to configure linter warnings project-wide instead of for all crates in that workspace individually. In our case aforementioned linter warnings are enabled on top of Embark's configuration, leading to a lot of these warnings in our build process. [EmbarkStudios/rust-ecosystem#68]: https://github.com/EmbarkStudios/rust-ecosystem/pull/68 --- ash-window/src/lib.rs | 6 +- .../extensions/khr/acceleration_structure.rs | 26 +++----- .../extensions/khr/ray_tracing_pipeline.rs | 8 +-- ash/src/lib.rs | 4 +- ash/src/vk.rs | 3 +- ash/src/vk/definitions.rs | 66 +++++++++---------- ash/src/vk/macros.rs | 4 +- generator/src/lib.rs | 23 +++++-- 8 files changed, 72 insertions(+), 68 deletions(-) diff --git a/ash-window/src/lib.rs b/ash-window/src/lib.rs index 1c74435f6..da0f27ec0 100644 --- a/ash-window/src/lib.rs +++ b/ash-window/src/lib.rs @@ -1,3 +1,5 @@ +#![warn(trivial_casts, trivial_numeric_casts)] + use ash::{extensions::khr, prelude::*, vk, Entry, Instance}; use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; use std::ffi::CStr; @@ -69,7 +71,7 @@ pub unsafe fn create_surface( ))] RawWindowHandle::Xcb(handle) => { let surface_desc = vk::XcbSurfaceCreateInfoKHR::builder() - .connection(handle.connection as *mut _) + .connection(handle.connection) .window(handle.window); let surface_fn = khr::XcbSurface::new(entry, instance); surface_fn.create_xcb_surface(&surface_desc, allocation_callbacks) @@ -78,7 +80,7 @@ pub unsafe fn create_surface( #[cfg(any(target_os = "android"))] RawWindowHandle::Android(handle) => { let surface_desc = - vk::AndroidSurfaceCreateInfoKHR::builder().window(handle.a_native_window as _); + vk::AndroidSurfaceCreateInfoKHR::builder().window(handle.a_native_window); let surface_fn = khr::AndroidSurface::new(entry, instance); surface_fn.create_android_surface(&surface_desc, allocation_callbacks) } diff --git a/ash/src/extensions/khr/acceleration_structure.rs b/ash/src/extensions/khr/acceleration_structure.rs index c6c527759..f361117f2 100644 --- a/ash/src/extensions/khr/acceleration_structure.rs +++ b/ash/src/extensions/khr/acceleration_structure.rs @@ -156,7 +156,7 @@ impl AccelerationStructure { info: &vk::CopyAccelerationStructureInfoKHR, ) -> VkResult<()> { self.fp - .copy_acceleration_structure_khr(self.handle, deferred_operation, info as *const _) + .copy_acceleration_structure_khr(self.handle, deferred_operation, info) .result() } @@ -167,11 +167,7 @@ impl AccelerationStructure { info: &vk::CopyAccelerationStructureToMemoryInfoKHR, ) -> VkResult<()> { self.fp - .copy_acceleration_structure_to_memory_khr( - self.handle, - deferred_operation, - info as *const _, - ) + .copy_acceleration_structure_to_memory_khr(self.handle, deferred_operation, info) .result() } @@ -182,11 +178,7 @@ impl AccelerationStructure { info: &vk::CopyMemoryToAccelerationStructureInfoKHR, ) -> VkResult<()> { self.fp - .copy_memory_to_acceleration_structure_khr( - self.handle, - deferred_operation, - info as *const _, - ) + .copy_memory_to_acceleration_structure_khr(self.handle, deferred_operation, info) .result() } @@ -228,7 +220,7 @@ impl AccelerationStructure { info: &vk::CopyAccelerationStructureToMemoryInfoKHR, ) { self.fp - .cmd_copy_acceleration_structure_to_memory_khr(command_buffer, info as *const _); + .cmd_copy_acceleration_structure_to_memory_khr(command_buffer, info); } /// @@ -238,7 +230,7 @@ impl AccelerationStructure { info: &vk::CopyMemoryToAccelerationStructureInfoKHR, ) { self.fp - .cmd_copy_memory_to_acceleration_structure_khr(command_buffer, info as *const _); + .cmd_copy_memory_to_acceleration_structure_khr(command_buffer, info); } /// @@ -247,7 +239,7 @@ impl AccelerationStructure { info: &vk::AccelerationStructureDeviceAddressInfoKHR, ) -> vk::DeviceAddress { self.fp - .get_acceleration_structure_device_address_khr(self.handle, info as *const _) + .get_acceleration_structure_device_address_khr(self.handle, info) } /// @@ -279,7 +271,7 @@ impl AccelerationStructure { self.fp.get_device_acceleration_structure_compatibility_khr( self.handle, version, - &mut compatibility as *mut _, + &mut compatibility, ); compatibility @@ -299,9 +291,9 @@ impl AccelerationStructure { self.fp.get_acceleration_structure_build_sizes_khr( self.handle, build_type, - build_info as *const _, + build_info, max_primitive_counts.as_ptr(), - &mut size_info as *mut _, + &mut size_info, ); size_info diff --git a/ash/src/extensions/khr/ray_tracing_pipeline.rs b/ash/src/extensions/khr/ray_tracing_pipeline.rs index 397410bb5..c04a20162 100644 --- a/ash/src/extensions/khr/ray_tracing_pipeline.rs +++ b/ash/src/extensions/khr/ray_tracing_pipeline.rs @@ -46,10 +46,10 @@ impl RayTracingPipeline { ) { self.fp.cmd_trace_rays_khr( command_buffer, - raygen_shader_binding_tables as *const _, - miss_shader_binding_tables as *const _, - hit_shader_binding_tables as *const _, - callable_shader_binding_tables as *const _, + raygen_shader_binding_tables, + miss_shader_binding_tables, + hit_shader_binding_tables, + callable_shader_binding_tables, width, height, depth, diff --git a/ash/src/lib.rs b/ash/src/lib.rs index 86f337d58..17c1add40 100644 --- a/ash/src/lib.rs +++ b/ash/src/lib.rs @@ -1,4 +1,5 @@ #![deny(clippy::use_self)] +#![warn(/* trivial_casts, */ trivial_numeric_casts)] #![allow( clippy::too_many_arguments, clippy::missing_safety_doc, @@ -59,8 +60,7 @@ pub trait RawPtr { impl<'r, T> RawPtr for Option<&'r T> { fn as_raw_ptr(&self) -> *const T { match *self { - Some(inner) => inner as *const T, - + Some(inner) => inner, _ => ::std::ptr::null(), } } diff --git a/ash/src/vk.rs b/ash/src/vk.rs index 32aedb273..f56072eec 100644 --- a/ash/src/vk.rs +++ b/ash/src/vk.rs @@ -29,6 +29,7 @@ pub use prelude::*; /// Native bindings from Vulkan headers, generated by bindgen #[allow(nonstandard_style)] #[allow(deref_nullptr)] +#[allow(trivial_casts, trivial_numeric_casts)] pub mod native; mod platform_types; pub use platform_types::*; @@ -40,7 +41,7 @@ pub(crate) unsafe fn ptr_chain_iter(ptr: &mut T) -> impl Iterator fmt::Result { fmt.debug_struct("ExtensionProperties") .field("extension_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.extension_name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.extension_name.as_ptr()) }) .field("spec_version", &self.spec_version) .finish() @@ -1176,12 +1176,12 @@ impl fmt::Debug for LayerProperties { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt.debug_struct("LayerProperties") .field("layer_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.layer_name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.layer_name.as_ptr()) }) .field("spec_version", &self.spec_version) .field("implementation_version", &self.implementation_version) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .finish() } @@ -4362,7 +4362,7 @@ impl<'a> ShaderModuleCreateInfoBuilder<'a> { } pub fn code(mut self, code: &'a [u32]) -> Self { self.inner.code_size = code.len() * 4; - self.inner.p_code = code.as_ptr() as *const u32; + self.inner.p_code = code.as_ptr(); self } #[doc = r" Prepends the given extension struct between the root and the first pointer. This"] @@ -4853,7 +4853,7 @@ impl<'a> SpecializationInfoBuilder<'a> { self } pub fn data(mut self, data: &'a [u8]) -> Self { - self.inner.data_size = data.len() as _; + self.inner.data_size = data.len(); self.inner.p_data = data.as_ptr() as *const c_void; self } @@ -5709,7 +5709,7 @@ impl<'a> PipelineMultisampleStateCreateInfoBuilder<'a> { self.inner.p_sample_mask = if sample_mask.is_empty() { std::ptr::null() } else { - sample_mask.as_ptr() as *const SampleMask + sample_mask.as_ptr() }; self } @@ -6401,7 +6401,7 @@ impl<'a> PipelineCacheCreateInfoBuilder<'a> { self } pub fn initial_data(mut self, initial_data: &'a [u8]) -> Self { - self.inner.initial_data_size = initial_data.len() as _; + self.inner.initial_data_size = initial_data.len(); self.inner.p_initial_data = initial_data.as_ptr() as *const c_void; self } @@ -11675,7 +11675,7 @@ impl<'a> DebugMarkerObjectTagInfoEXTBuilder<'a> { self } pub fn tag(mut self, tag: &'a [u8]) -> Self { - self.inner.tag_size = tag.len() as _; + self.inner.tag_size = tag.len(); self.inner.p_tag = tag.as_ptr() as *const c_void; self } @@ -14311,10 +14311,10 @@ impl fmt::Debug for PhysicalDeviceDriverProperties { .field("p_next", &self.p_next) .field("driver_id", &self.driver_id) .field("driver_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr()) }) .field("driver_info", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr()) }) .field("conformance_version", &self.conformance_version) .finish() @@ -24097,7 +24097,7 @@ impl<'a> ValidationCacheCreateInfoEXTBuilder<'a> { self } pub fn initial_data(mut self, initial_data: &'a [u8]) -> Self { - self.inner.initial_data_size = initial_data.len() as _; + self.inner.initial_data_size = initial_data.len(); self.inner.p_initial_data = initial_data.as_ptr() as *const c_void; self } @@ -25487,7 +25487,7 @@ impl<'a> DebugUtilsObjectTagInfoEXTBuilder<'a> { self } pub fn tag(mut self, tag: &'a [u8]) -> Self { - self.inner.tag_size = tag.len() as _; + self.inner.tag_size = tag.len(); self.inner.p_tag = tag.as_ptr() as *const c_void; self } @@ -37008,13 +37008,13 @@ impl fmt::Debug for PerformanceCounterDescriptionKHR { .field("p_next", &self.p_next) .field("flags", &self.flags) .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.name.as_ptr()) }) .field("category", &unsafe { - ::std::ffi::CStr::from_ptr(self.category.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.category.as_ptr()) }) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .finish() } @@ -38748,10 +38748,10 @@ impl fmt::Debug for PipelineExecutablePropertiesKHR { .field("p_next", &self.p_next) .field("stages", &self.stages) .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.name.as_ptr()) }) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .field("subgroup_size", &self.subgroup_size) .finish() @@ -38909,10 +38909,10 @@ impl fmt::Debug for PipelineExecutableStatisticKHR { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.name.as_ptr()) }) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .field("format", &self.format) .field("value", &"union") @@ -38998,10 +38998,10 @@ impl fmt::Debug for PipelineExecutableInternalRepresentationKHR { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.name.as_ptr()) }) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .field("is_text", &self.is_text) .field("data_size", &self.data_size) @@ -39060,7 +39060,7 @@ impl<'a> PipelineExecutableInternalRepresentationKHRBuilder<'a> { self } pub fn data(mut self, data: &'a mut [u8]) -> Self { - self.inner.data_size = data.len() as _; + self.inner.data_size = data.len(); self.inner.p_data = data.as_mut_ptr() as *mut c_void; self } @@ -40836,10 +40836,10 @@ impl fmt::Debug for PhysicalDeviceVulkan12Properties { .field("p_next", &self.p_next) .field("driver_id", &self.driver_id) .field("driver_name", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.driver_name.as_ptr()) }) .field("driver_info", &unsafe { - ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.driver_info.as_ptr()) }) .field("conformance_version", &self.conformance_version) .field( @@ -41644,17 +41644,17 @@ impl fmt::Debug for PhysicalDeviceToolPropertiesEXT { .field("s_type", &self.s_type) .field("p_next", &self.p_next) .field("name", &unsafe { - ::std::ffi::CStr::from_ptr(self.name.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.name.as_ptr()) }) .field("version", &unsafe { - ::std::ffi::CStr::from_ptr(self.version.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.version.as_ptr()) }) .field("purposes", &self.purposes) .field("description", &unsafe { - ::std::ffi::CStr::from_ptr(self.description.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.description.as_ptr()) }) .field("layer", &unsafe { - ::std::ffi::CStr::from_ptr(self.layer.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.layer.as_ptr()) }) .finish() } @@ -42923,7 +42923,7 @@ impl<'a> ::std::ops::DerefMut for AccelerationStructureVersionInfoKHRBuilder<'a> } impl<'a> AccelerationStructureVersionInfoKHRBuilder<'a> { pub fn version_data(mut self, version_data: &'a [u8; 2 * UUID_SIZE]) -> Self { - self.inner.p_version_data = version_data as *const [u8; 2 * UUID_SIZE]; + self.inner.p_version_data = version_data; self } #[doc = r" Calling build will **discard** all the lifetime information. Only call this if"] @@ -53094,7 +53094,7 @@ impl<'a> ::std::ops::DerefMut for CuModuleCreateInfoNVXBuilder<'a> { } impl<'a> CuModuleCreateInfoNVXBuilder<'a> { pub fn data(mut self, data: &'a [u8]) -> Self { - self.inner.data_size = data.len() as _; + self.inner.data_size = data.len(); self.inner.p_data = data.as_ptr() as *const c_void; self } @@ -53263,12 +53263,12 @@ impl<'a> CuLaunchInfoNVXBuilder<'a> { self } pub fn params(mut self, params: &'a [*const c_void]) -> Self { - self.inner.param_count = params.len() as _; + self.inner.param_count = params.len(); self.inner.p_params = params.as_ptr(); self } pub fn extras(mut self, extras: &'a [*const c_void]) -> Self { - self.inner.extra_count = extras.len() as _; + self.inner.extra_count = extras.len(); self.inner.p_extras = extras.as_ptr(); self } diff --git a/ash/src/vk/macros.rs b/ash/src/vk/macros.rs index e87d14352..e986be312 100644 --- a/ash/src/vk/macros.rs +++ b/ash/src/vk/macros.rs @@ -94,10 +94,10 @@ macro_rules! handle_nondispatchable { impl Handle for $name { const TYPE: ObjectType = ObjectType::$ty; fn as_raw(self) -> u64 { - self.0 as u64 + self.0 } fn from_raw(x: u64) -> Self { - Self(x as _) + Self(x) } } impl $name { diff --git a/generator/src/lib.rs b/generator/src/lib.rs index 2cc21171d..887234f07 100644 --- a/generator/src/lib.rs +++ b/generator/src/lib.rs @@ -1,4 +1,5 @@ #![recursion_limit = "256"] +#![warn(trivial_casts, trivial_numeric_casts)] use heck::{CamelCase, ShoutySnakeCase, SnakeCase}; use itertools::Itertools; @@ -1499,7 +1500,7 @@ pub fn derive_debug(_struct: &vkxml::Struct, union_types: &HashSet<&str>) -> Opt let debug_value = if is_static_array(field) && field.basetype == "char" { quote! { &unsafe { - ::std::ffi::CStr::from_ptr(self.#param_ident.as_ptr() as *const c_char) + ::std::ffi::CStr::from_ptr(self.#param_ident.as_ptr()) } } } else if param_str.contains("pfn") { @@ -1610,7 +1611,7 @@ pub fn derive_setters( return Some(quote!{ pub fn code(mut self, code: &'a [u32]) -> Self { self.inner.code_size = code.len() * 4; - self.inner.p_code = code.as_ptr() as *const u32; + self.inner.p_code = code.as_ptr(); self } }); @@ -1627,7 +1628,7 @@ pub fn derive_setters( self.inner.p_sample_mask = if sample_mask.is_empty() { std::ptr::null() } else { - sample_mask.as_ptr() as *const SampleMask + sample_mask.as_ptr() }; self } @@ -1681,15 +1682,23 @@ pub fn derive_setters( let array_size = field.c_size.as_ref().unwrap(); let c_size = convert_c_expression(array_size, &BTreeMap::new()); let inner_type = field.inner_type_tokens(); - let mutable = if field.is_const { quote!(const) } else { quote!(mut) }; slice_param_ty_tokens = quote!([#inner_type; #c_size]); - ptr = quote!(as *#mutable #slice_param_ty_tokens); + ptr = quote!(); quote!() } else { let array_size_ident = format_ident!("{}", array_size.to_snake_case().as_str()); - quote!(self.inner.#array_size_ident = #param_ident_short.len() as _;) + + let size_field = members.clone().find(|m| m.name.as_ref() == Some(array_size)).unwrap(); + + let cast = if size_field.basetype == "size_t" { + quote!() + }else{ + quote!(as _) + }; + + quote!(self.inner.#array_size_ident = #param_ident_short.len()#cast;) }; let mutable = if field.is_const { quote!() } else { quote!(mut) }; @@ -1742,7 +1751,7 @@ pub fn derive_setters( /// If the chain looks like `A -> B -> C`, and you call `builder.push_next(&mut D)`, then the /// chain will look like `A -> D -> B -> C`. pub fn push_next(mut self, next: &'a mut T) -> Self { - unsafe{ + unsafe { let next_ptr = next as *mut T as *mut BaseOutStructure; // `next` here can contain a pointer chain. This means that we must correctly // attach he head to the root and the tail to the rest of the chain