Skip to content

Commit

Permalink
ash: Remove unnecessary trivial_casts and trivial_numeric_casts
Browse files Browse the repository at this point in the history
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]: EmbarkStudios/rust-ecosystem#68
  • Loading branch information
MarijnS95 committed Jan 26, 2022
1 parent 0fd7327 commit 73f8fa0
Show file tree
Hide file tree
Showing 9 changed files with 258 additions and 241 deletions.
6 changes: 4 additions & 2 deletions ash-window/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down
13 changes: 9 additions & 4 deletions ash/src/extensions/experimental/amd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
26 changes: 9 additions & 17 deletions ash/src/extensions/khr/acceleration_structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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()
}

Expand All @@ -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()
}

Expand Down Expand Up @@ -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.2-extensions/man/html/vkCmdCopyMemoryToAccelerationStructureKHR.html>
Expand All @@ -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.2-extensions/man/html/vkGetAccelerationStructureHandleKHR.html>
Expand All @@ -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.2-extensions/man/html/vkCmdWriteAccelerationStructuresPropertiesKHR.html>
Expand Down Expand Up @@ -279,7 +271,7 @@ impl AccelerationStructure {
self.fp.get_device_acceleration_structure_compatibility_khr(
self.handle,
version,
&mut compatibility as *mut _,
&mut compatibility,
);

compatibility
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions ash/src/extensions/khr/ray_tracing_pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions ash/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![deny(clippy::use_self)]
#![warn(trivial_casts, trivial_numeric_casts)]
#![allow(
clippy::too_many_arguments,
clippy::missing_safety_doc,
Expand Down Expand Up @@ -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(),
}
}
Expand All @@ -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,
&corner as *const _ as usize,
<*mut _>::cast(&mut variable_pointers),
<*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);
Expand Down
5 changes: 3 additions & 2 deletions ash/src/vk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 73f8fa0

Please sign in to comment.