Skip to content

Commit

Permalink
chore: separate new unsafe ops into blocks
Browse files Browse the repository at this point in the history
Unsafe operations can be exhausting to validate by themselves. Therefore, it's often interesting to
separate these out so we can justify each individual operation, and let a human reader accumulate
and drop supporting safety context in the smallest increments necessary. To that end, this commit
can pave the way for future work where we may do something like enable
[`clippy::undocumented_unsafe_blocks`], which calls out `unsafe` blocks that do not have a `SAFETY`
comment immediately above them.

This commit only separates the operations in `unsafe` blocks I added in this same PR; I'm
deliberately leaving existing `unsafe` blocks alone, ATM.

[`clippy::undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/stable/index.html#undocumented_unsafe_blocks
  • Loading branch information
ErichDonGubler committed Nov 14, 2022
1 parent 02cdc85 commit f56a393
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 47 deletions.
9 changes: 3 additions & 6 deletions wgpu-core/src/track/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,12 +445,9 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
}
ResourceMetadataProvider::Indirect { metadata } => {
metadata.tracker_assert_in_bounds(index);
(unsafe { *metadata.epochs.get_unchecked(index) }, unsafe {
metadata
.ref_counts
.get_unchecked(index)
.clone()
.unwrap_unchecked()
(unsafe { *metadata.epochs.get_unchecked(index) }, {
let ref_count = unsafe { metadata.ref_counts.get_unchecked(index) };
unsafe { ref_count.clone().unwrap_unchecked() }
})
}
ResourceMetadataProvider::Resource { epoch } => {
Expand Down
18 changes: 8 additions & 10 deletions wgpu-core/src/track/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,10 @@ impl<A: hub::HalApi> TextureUsageScope<A> {
self.tracker_assert_in_bounds(index);
scope.tracker_assert_in_bounds(index);

let texture_data = unsafe { texture_data_from_texture(storage, index32) };
unsafe {
insert_or_merge(
texture_data_from_texture(storage, index32),
texture_data,
&mut self.set,
&mut self.metadata,
index32,
Expand Down Expand Up @@ -359,9 +360,10 @@ impl<A: hub::HalApi> TextureUsageScope<A> {

self.tracker_assert_in_bounds(index);

let texture_data = unsafe { texture_data_from_texture(storage, index32) };
unsafe {
insert_or_merge(
texture_data_from_texture(storage, index32),
texture_data,
&mut self.set,
&mut self.metadata,
index32,
Expand Down Expand Up @@ -467,13 +469,8 @@ impl<A: hub::HalApi> TextureTracker<A> {

self.tracker_assert_in_bounds(index);

unsafe {
self.metadata
.ref_counts
.get_unchecked(index)
.as_ref()
.unwrap_unchecked()
}
let ref_count = unsafe { self.metadata.ref_counts.get_unchecked(index) };
unsafe { ref_count.as_ref().unwrap_unchecked() }
}

/// Inserts a single texture and a state into the resource tracker.
Expand Down Expand Up @@ -683,9 +680,10 @@ impl<A: hub::HalApi> TextureTracker<A> {
if unsafe { !scope.metadata.owned.get(index).unwrap_unchecked() } {
continue;
}
let texture_data = unsafe { texture_data_from_texture(storage, index32) };
unsafe {
insert_or_barrier_update(
texture_data_from_texture(storage, index32),
texture_data,
Some(&mut self.start_set),
&mut self.end_set,
&mut self.metadata,
Expand Down
33 changes: 16 additions & 17 deletions wgpu-hal/src/gles/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ impl super::Device {
if gl.supports_debug() {
//TODO: remove all transmutes from `object_label`
// https://github.com/grovesNL/glow/issues/186
unsafe { gl.object_label(glow::SHADER, mem::transmute(raw), label) };
let name = unsafe { mem::transmute(raw) };
unsafe { gl.object_label(glow::SHADER, name, label) };
}

unsafe { gl.shader_source(raw, shader) };
Expand Down Expand Up @@ -276,7 +277,8 @@ impl super::Device {
#[cfg(not(target_arch = "wasm32"))]
if let Some(label) = label {
if gl.supports_debug() {
unsafe { gl.object_label(glow::PROGRAM, mem::transmute(program), Some(label)) };
let name = unsafe { mem::transmute(program) };
unsafe { gl.object_label(glow::PROGRAM, name, Some(label)) };
}
}

Expand Down Expand Up @@ -363,12 +365,8 @@ impl super::Device {
return Err(crate::DeviceError::Lost.into());
}
super::BindingRegister::Textures | super::BindingRegister::Images => {
unsafe {
gl.uniform_1_i32(
gl.get_uniform_location(program, name).as_ref(),
slot as _,
)
};
let location = unsafe { gl.get_uniform_location(program, name) };
unsafe { gl.uniform_1_i32(location.as_ref(), slot as _) };
}
}
}
Expand Down Expand Up @@ -516,7 +514,8 @@ impl crate::Device<super::Api> for super::Device {
#[cfg(not(target_arch = "wasm32"))]
if let Some(label) = desc.label {
if gl.supports_debug() {
unsafe { gl.object_label(glow::BUFFER, mem::transmute(raw), Some(label)) };
let name = unsafe { mem::transmute(raw) };
unsafe { gl.object_label(glow::BUFFER, name, Some(label)) };
}
}

Expand Down Expand Up @@ -660,9 +659,8 @@ impl crate::Device<super::Api> for super::Device {
#[cfg(not(target_arch = "wasm32"))]
if let Some(label) = desc.label {
if gl.supports_debug() {
unsafe {
gl.object_label(glow::RENDERBUFFER, mem::transmute(raw), Some(label))
};
let name = unsafe { mem::transmute(raw) };
unsafe { gl.object_label(glow::RENDERBUFFER, name, Some(label)) };
}
}

Expand Down Expand Up @@ -728,7 +726,8 @@ impl crate::Device<super::Api> for super::Device {
#[cfg(not(target_arch = "wasm32"))]
if let Some(label) = desc.label {
if gl.supports_debug() {
unsafe { gl.object_label(glow::TEXTURE, mem::transmute(raw), Some(label)) };
let name = unsafe { mem::transmute(raw) };
unsafe { gl.object_label(glow::TEXTURE, name, Some(label)) };
}
}

Expand Down Expand Up @@ -876,7 +875,8 @@ impl crate::Device<super::Api> for super::Device {
#[cfg(not(target_arch = "wasm32"))]
if let Some(label) = desc.label {
if gl.supports_debug() {
unsafe { gl.object_label(glow::SAMPLER, mem::transmute(raw), Some(label)) };
let name = unsafe { mem::transmute(raw) };
unsafe { gl.object_label(glow::SAMPLER, name, Some(label)) };
}
}

Expand Down Expand Up @@ -1170,9 +1170,8 @@ impl crate::Device<super::Api> for super::Device {
if let Some(label) = desc.label {
temp_string.clear();
let _ = write!(temp_string, "{}[{}]", label, i);
unsafe {
gl.object_label(glow::QUERY, mem::transmute(query), Some(&temp_string))
};
let name = unsafe { mem::transmute(query) };
unsafe { gl.object_label(glow::QUERY, name, Some(&temp_string)) };
}
}
queries.push(query);
Expand Down
19 changes: 8 additions & 11 deletions wgpu-hal/src/gles/egl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,9 @@ impl crate::Instance<super::Api> for Instance {
&& client_ext_str.contains("EGL_KHR_debug")
{
log::info!("Enabling EGL debug output");
let function: EglDebugMessageControlFun = unsafe {
std::mem::transmute(egl.get_proc_address("eglDebugMessageControlKHR").unwrap())
let function: EglDebugMessageControlFun = {
let addr = egl.get_proc_address("eglDebugMessageControlKHR").unwrap();
unsafe { std::mem::transmute(addr) }
};
let attributes = [
EGL_DEBUG_MSG_CRITICAL_KHR as egl::Attrib,
Expand Down Expand Up @@ -911,9 +912,10 @@ impl super::Adapter {
pub unsafe fn new_external(
fun: impl FnMut(&str) -> *const ffi::c_void,
) -> Option<crate::ExposedAdapter<super::Api>> {
let context = unsafe { glow::Context::from_loader_function(fun) };
unsafe {
Self::expose(AdapterContext {
glow: Mutex::new(glow::Context::from_loader_function(fun)),
glow: Mutex::new(context),
egl: None,
})
}
Expand Down Expand Up @@ -1238,14 +1240,9 @@ impl crate::Surface<super::Api> for Surface {
.destroy_surface(self.egl.display, surface)
.unwrap();
if let Some(window) = wl_window {
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> = unsafe {
self.wsi
.library
.as_ref()
.expect("unsupported window")
.get(b"wl_egl_window_destroy")
}
.unwrap();
let library = self.wsi.library.as_ref().expect("unsupported window");
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> =
unsafe { library.get(b"wl_egl_window_destroy") }.unwrap();
unsafe { wl_egl_window_destroy(window) };
}
}
Expand Down
5 changes: 3 additions & 2 deletions wgpu-hal/src/metal/surface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ impl super::Surface {
delegate: Option<&HalManagedMetalLayerDelegate>,
) -> Self {
let view = view as *mut Object;
let render_layer = unsafe {
mem::transmute::<_, &mtl::MetalLayerRef>(Self::get_metal_layer(view, delegate))
let render_layer = {
let layer = unsafe { Self::get_metal_layer(view, delegate) };
unsafe { mem::transmute::<_, &mtl::MetalLayerRef>(layer) }
}
.to_owned();
let _: *mut c_void = msg_send![view, retain];
Expand Down
5 changes: 4 additions & 1 deletion wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ impl super::DeviceShared {
.collect();
&buffer_vec
};

let name = unsafe { CStr::from_bytes_with_nul_unchecked(name_bytes) };

let _result = unsafe {
extension.debug_utils_set_object_name(
self.raw.handle(),
&vk::DebugUtilsObjectNameInfoEXT::builder()
.object_type(object_type)
.object_handle(object.as_raw())
.object_name(CStr::from_bytes_with_nul_unchecked(name_bytes)),
.object_name(name),
)
};
}
Expand Down

0 comments on commit f56a393

Please sign in to comment.