From 213371f5476c54a436ccea525d5028961b437859 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Tue, 20 Dec 2022 16:25:21 -0500 Subject: [PATCH 1/3] Removes unused assertions.rs --- wgpu-core/src/assertions.rs | 60 ------------------------------------- 1 file changed, 60 deletions(-) delete mode 100644 wgpu-core/src/assertions.rs diff --git a/wgpu-core/src/assertions.rs b/wgpu-core/src/assertions.rs deleted file mode 100644 index fb9314a3c94..00000000000 --- a/wgpu-core/src/assertions.rs +++ /dev/null @@ -1,60 +0,0 @@ -//! Macros for validation internal to the resource tracker. -//! -//! This module defines assertion macros that respect `wgpu-core`'s -//! `"strict_asserts"` feature. -//! -//! Because `wgpu-core`'s public APIs validate their arguments in all -//! types of builds, for performance, the `track` module skips some of -//! Rust's usual run-time checks on its internal operations in release -//! builds. However, some `wgpu-core` applications have a strong -//! preference for robustness over performance. To accommodate them, -//! `wgpu-core`'s `"strict_asserts"` feature enables that validation -//! in both debug and release builds. - -#[cfg(feature = "strict_asserts")] -#[macro_export] -macro_rules! strict_assert { - ( $( $arg:tt )* ) => { - assert!( $( $arg )* ) - } -} - -#[cfg(feature = "strict_asserts")] -#[macro_export] -macro_rules! strict_assert_eq { - ( $( $arg:tt )* ) => { - assert_eq!( $( $arg )* ) - } -} - -#[cfg(feature = "strict_asserts")] -#[macro_export] -macro_rules! strict_assert_ne { - ( $( $arg:tt )* ) => { - assert_ne!( $( $arg )* ) - } -} - -#[cfg(not(feature = "strict_asserts"))] -#[macro_export] -macro_rules! strict_assert { - ( $( $arg:tt )* ) => { - debug_assert!( $( $arg )* ) - }; -} - -#[cfg(not(feature = "strict_asserts"))] -#[macro_export] -macro_rules! strict_assert_eq { - ( $( $arg:tt )* ) => { - debug_assert_eq!( $( $arg )* ) - }; -} - -#[cfg(not(feature = "strict_asserts"))] -#[macro_export] -macro_rules! strict_assert_ne { - ( $( $arg:tt )* ) => { - debug_assert_ne!( $( $arg )* ) - }; -} From 602440886187e4e1a0c7f5fc68c8db2aaa827549 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Tue, 20 Dec 2022 16:25:39 -0500 Subject: [PATCH 2/3] Strict assert macro fixes --- wgpu-hal/src/dx12/suballocation.rs | 24 ++++++++++++++---------- wgpu-types/src/assertions.rs | 28 +++++++++++++++++----------- wgpu/Cargo.toml | 2 +- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/wgpu-hal/src/dx12/suballocation.rs b/wgpu-hal/src/dx12/suballocation.rs index 62186a39649..4822165a5d7 100644 --- a/wgpu-hal/src/dx12/suballocation.rs +++ b/wgpu-hal/src/dx12/suballocation.rs @@ -73,11 +73,13 @@ mod allocation { let name = desc.label.unwrap_or("Unlabeled buffer"); // SAFETY: allocator exists when the windows_rs feature is enabled - let mut allocator = device - .mem_allocator - .as_ref() - .strict_unwrap_unchecked() - .lock(); + let mut allocator = unsafe { + device + .mem_allocator + .as_ref() + .strict_unwrap_unchecked() + .lock() + }; // let mut allocator = unsafe { device.mem_allocator.as_ref().unwrap_unchecked().lock() }; let allocation_desc = AllocationCreateDesc::from_winapi_d3d12_resource_desc( @@ -114,11 +116,13 @@ mod allocation { let name = desc.label.unwrap_or("Unlabeled texture"); // SAFETY: allocator exists when the windows_rs feature is enabled - let mut allocator = device - .mem_allocator - .as_ref() - .strict_unwrap_unchecked() - .lock(); + let mut allocator = unsafe { + device + .mem_allocator + .as_ref() + .strict_unwrap_unchecked() + .lock() + }; let allocation_desc = AllocationCreateDesc::from_winapi_d3d12_resource_desc( allocator.allocator.device().as_winapi(), &raw_desc, diff --git a/wgpu-types/src/assertions.rs b/wgpu-types/src/assertions.rs index 8dc390c0ec4..ee10bfd56c8 100644 --- a/wgpu-types/src/assertions.rs +++ b/wgpu-types/src/assertions.rs @@ -11,7 +11,7 @@ //! `wgpu-core`'s `"strict_asserts"` feature enables that validation //! in both debug and release builds. -/// This is equivalent to [`std::assert`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert`]. #[cfg(feature = "strict_asserts")] #[macro_export] macro_rules! strict_assert { @@ -20,7 +20,7 @@ macro_rules! strict_assert { } } -/// This is equivalent to [`std::assert_eq`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert_eq`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert_eq`]. #[cfg(feature = "strict_asserts")] #[macro_export] macro_rules! strict_assert_eq { @@ -29,7 +29,7 @@ macro_rules! strict_assert_eq { } } -/// This is equivalent to [`std::assert_ne`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert_ne`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert_ne`]. #[cfg(feature = "strict_asserts")] #[macro_export] macro_rules! strict_assert_ne { @@ -38,7 +38,7 @@ macro_rules! strict_assert_ne { } } -/// This is equivalent to [`std::assert`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert`] #[cfg(not(feature = "strict_asserts"))] #[macro_export] macro_rules! strict_assert { @@ -47,7 +47,7 @@ macro_rules! strict_assert { }; } -/// This is equivalent to [`std::assert_eq`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert_eq`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert_eq`] #[cfg(not(feature = "strict_asserts"))] #[macro_export] macro_rules! strict_assert_eq { @@ -56,7 +56,7 @@ macro_rules! strict_assert_eq { }; } -/// This is equivalent to [`std::assert_ne`] if the `strict_asserts` feature is activated. +/// This is equivalent to [`std::assert_ne`] if the `strict_asserts` feature is activated, otherwise equal to [`std::debug_assert_ne`] #[cfg(not(feature = "strict_asserts"))] #[macro_export] macro_rules! strict_assert_ne { @@ -65,22 +65,28 @@ macro_rules! strict_assert_ne { }; } -/// Used to implement strict_assert for unwrap_unchecked +/// Unwrapping using strict_asserts pub trait StrictAssertUnwrapExt { - /// Implementation of strict_assert for unwrap_unchecked - fn strict_unwrap_unchecked(self) -> T; + /// Unchecked unwrap, with a [`strict_assert`] backed assertion of validitly. + /// + /// # Safety + /// + /// It _must_ be valid to call unwrap_unchecked on this value. + unsafe fn strict_unwrap_unchecked(self) -> T; } impl StrictAssertUnwrapExt for Option { - fn strict_unwrap_unchecked(self) -> T { + unsafe fn strict_unwrap_unchecked(self) -> T { strict_assert!(self.is_some(), "Called strict_unwrap_unchecked on None"); + // SAFETY: Checked by above assert, or by assertion by unsafe. unsafe { self.unwrap_unchecked() } } } impl StrictAssertUnwrapExt for Result { - fn strict_unwrap_unchecked(self) -> T { + unsafe fn strict_unwrap_unchecked(self) -> T { strict_assert!(self.is_ok(), "Called strict_unwrap_unchecked on Err"); + // SAFETY: Checked by above assert, or by assertion by unsafe. unsafe { self.unwrap_unchecked() } } } diff --git a/wgpu/Cargo.toml b/wgpu/Cargo.toml index fe884d339b9..ee7f9a5bf50 100644 --- a/wgpu/Cargo.toml +++ b/wgpu/Cargo.toml @@ -79,7 +79,7 @@ test = true default = ["wgsl", "expose-ids", "strict_asserts"] # Apply run-time checks, even in release builds. These are in addition # to the validation carried out at public APIs in all builds. -strict_asserts = ["wgc/strict_asserts", "wgt/strict_asserts"] +strict_asserts = ["wgc?/strict_asserts", "wgt/strict_asserts"] spirv = ["naga/spv-in"] glsl = ["naga/glsl-in"] wgsl = ["wgc?/wgsl"] From df4a1e14457b3c8902b8a0715ce6846db186f06e Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Tue, 20 Dec 2022 17:04:28 -0500 Subject: [PATCH 3/3] Strict asserts shouldn't be default on wgpu --- wgpu/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu/Cargo.toml b/wgpu/Cargo.toml index ee7f9a5bf50..b2e90793fd1 100644 --- a/wgpu/Cargo.toml +++ b/wgpu/Cargo.toml @@ -76,7 +76,7 @@ name = "water" test = true [features] -default = ["wgsl", "expose-ids", "strict_asserts"] +default = ["wgsl", "expose-ids"] # Apply run-time checks, even in release builds. These are in addition # to the validation carried out at public APIs in all builds. strict_asserts = ["wgc?/strict_asserts", "wgt/strict_asserts"]