ash: Remove unnecessary trivial_casts and trivial_numeric_casts (#564)

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

@@ -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)
         }

ash/src/extensions/experimental/amd.rs

@@ -240,11 +240,16 @@ impl<'a> ::std::ops::Deref for PhysicalDeviceGpaPropertiesAmdBuilder<'a> {
     }
 }
 impl<'a> PhysicalDeviceGpaPropertiesAmdBuilder<'a> {
-    pub fn next<T>(mut self, next: &'a mut T) -> PhysicalDeviceGpaPropertiesAmdBuilder<'a>
+    pub fn push_next<T>(mut self, next: &'a mut T) -> PhysicalDeviceGpaPropertiesAmdBuilder<'a>
     where
         T: ExtendsPhysicalDeviceGpaPropertiesAmd,
     {
-        self.inner.p_next = next as *mut T as *mut c_void;
+        unsafe {
+            let next_ptr = <*const T>::cast(next);
+            let last_next = ptr_chain_iter(next).last().unwrap();
+            (*last_next).p_next = self.inner.p_next as _;
+            self.inner.p_next = next_ptr;
+        }
         self
     }
     pub fn build(self) -> PhysicalDeviceGpaPropertiesAmd {
@@ -694,10 +699,10 @@ impl<'a> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a> {
         T: ExtendsPhysicalDeviceWaveLimitPropertiesAmd,
     {
         unsafe {
-            let next_ptr = next as *mut T as *mut BaseOutStructure;
+            let next_ptr = <*const T>::cast(next);
             let last_next = ptr_chain_iter(next).last().unwrap();
             (*last_next).p_next = self.inner.p_next as _;
-            self.inner.p_next = next_ptr as _;
+            self.inner.p_next = next_ptr;
         }
         self
     }

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(
+            .copy_acceleration_structure_to_memory_khr(self.handle, deferred_operation, info)
-                self.handle,
-                deferred_operation,
-                info as *const _,
-            )
             .result()
     }
 
@@ -182,11 +178,7 @@ impl AccelerationStructure {
         info: &vk::CopyMemoryToAccelerationStructureInfoKHR,
     ) -> VkResult<()> {
         self.fp
-            .copy_memory_to_acceleration_structure_khr(
+            .copy_memory_to_acceleration_structure_khr(self.handle, deferred_operation, info)
-                self.handle,
-                deferred_operation,
-                info as *const _,
-            )
             .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);
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCmdCopyMemoryToAccelerationStructureKHR.html>
@@ -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);
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkGetAccelerationStructureHandleKHR.html>
@@ -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)
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCmdWriteAccelerationStructuresPropertiesKHR.html>
@@ -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

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 _,
+            raygen_shader_binding_tables,
-            miss_shader_binding_tables as *const _,
+            miss_shader_binding_tables,
-            hit_shader_binding_tables as *const _,
+            hit_shader_binding_tables,
-            callable_shader_binding_tables as *const _,
+            callable_shader_binding_tables,
             width,
             height,
             depth,

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<T> {
 impl<'r, T> RawPtr<T> 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(),
         }
     }
@@ -74,16 +74,15 @@ mod tests {
         let mut variable_pointers = vk::PhysicalDeviceVariablePointerFeatures::builder();
         let mut corner = vk::PhysicalDeviceCornerSampledImageFeaturesNV::builder();
         let chain = vec![
-            &variable_pointers as *const _ as usize,
+            <*mut _>::cast(&mut variable_pointers),
-            &corner as *const _ as usize,
+            <*mut _>::cast(&mut corner),
         ];
         let mut device_create_info = vk::DeviceCreateInfo::builder()
             .push_next(&mut corner)
             .push_next(&mut variable_pointers);
-        let chain2: Vec<usize> = unsafe {
+        let chain2: Vec<*mut vk::BaseOutStructure> = unsafe {
             vk::ptr_chain_iter(&mut device_create_info)
                 .skip(1)
-                .map(|ptr| ptr as usize)
                 .collect()
         };
         assert_eq!(chain, chain2);

ash/src/vk.rs

@@ -29,18 +29,19 @@ 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::*;
 /// Iterates through the pointer chain. Includes the item that is passed into the function.
 /// Stops at the last [`BaseOutStructure`] that has a null [`BaseOutStructure::p_next`] field.
 pub(crate) unsafe fn ptr_chain_iter<T>(ptr: &mut T) -> impl Iterator<Item = *mut BaseOutStructure> {
-    let ptr: *mut BaseOutStructure = ptr as *mut T as _;
+    let ptr = <*mut T>::cast::<BaseOutStructure>(ptr);
     (0..).scan(ptr, |p_ptr, _| {
         if p_ptr.is_null() {
             return None;
         }
-        let n_ptr = (**p_ptr).p_next as *mut BaseOutStructure;
+        let n_ptr = (**p_ptr).p_next;
         let old = *p_ptr;
         *p_ptr = n_ptr;
         Some(old)

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 {

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") {
@@ -1551,7 +1552,9 @@ pub fn derive_setters(
         _ => None,
     });
 
-    let has_next = members.clone().any(|field| field.param_ident() == "p_next");
+    let next_field = members
+        .clone()
+        .find(|field| field.param_ident() == "p_next");
 
     let nofilter_count_members = [
         "VkPipelineViewportStateCreateInfo.pViewports",
@@ -1610,7 +1613,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 +1630,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 +1684,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) };
@@ -1731,10 +1742,17 @@ pub fn derive_setters(
 
     let extends_name = format_ident!("Extends{}", name);
 
-    let is_root_struct = has_next && root_structs.contains(&name);
+    // The `p_next` field should only be considered if this struct is also a root struct
+    let root_struct_next_field = next_field.filter(|_| root_structs.contains(&name));
 
     // We only implement a next methods for root structs with a `pnext` field.
-    let next_function = if is_root_struct {
+    let next_function = if let Some(next_field) = root_struct_next_field {
+        assert_eq!(next_field.basetype, "void");
+        let mutability = if next_field.is_const {
+            quote!(const)
+        } else {
+            quote!(mut)
+        };
         quote! {
             /// Prepends the given extension struct between the root and the first pointer. This
             /// method only exists on structs that can be passed to a function directly. Only
@@ -1742,8 +1760,8 @@ 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<T: #extends_name>(mut self, next: &'a mut T) -> Self {
-                unsafe{
+                unsafe {
-                    let next_ptr = next as *mut T as *mut BaseOutStructure;
+                    let next_ptr = <*#mutability T>::cast(next);
                     // `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
                     // For example:
@@ -1755,7 +1773,7 @@ pub fn derive_setters(
                     //                 next chain
                     let last_next = ptr_chain_iter(next).last().unwrap();
                     (*last_next).p_next = self.inner.p_next as _;
-                    self.inner.p_next = next_ptr as _;
+                    self.inner.p_next = next_ptr;
                 }
                 self
             }
@@ -1766,7 +1784,7 @@ pub fn derive_setters(
 
     // Root structs come with their own trait that structs that extend
     // this struct will implement
-    let next_trait = if is_root_struct {
+    let next_trait = if root_struct_next_field.is_some() {
         quote!(pub unsafe trait #extends_name {})
     } else {
         quote!()