Skip to content

Commit ab50f4d

Browse files
ErichDonGublerErichDonGubler
committed
chore: separate new unsafe ops into blocks
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
1 parent abb4960 commit ab50f4d

File tree

6 files changed

+42
-47
lines changed

6 files changed

+42
-47
lines changed

‎wgpu-core/src/track/mod.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -445,12 +445,9 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
445445
}
446446
ResourceMetadataProvider::Indirect { metadata } => {
447447
metadata.tracker_assert_in_bounds(index);
448-
(unsafe { *metadata.epochs.get_unchecked(index) }, unsafe {
449-
metadata
450-
.ref_counts
451-
.get_unchecked(index)
452-
.clone()
453-
.unwrap_unchecked()
448+
(unsafe { *metadata.epochs.get_unchecked(index) }, {
449+
let ref_count = unsafe { metadata.ref_counts.get_unchecked(index) };
450+
unsafe { ref_count.clone().unwrap_unchecked() }
454451
})
455452
}
456453
ResourceMetadataProvider::Resource { epoch } => {

‎wgpu-core/src/track/texture.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ impl<A: hub::HalApi> TextureUsageScope<A> {
291291
self.tracker_assert_in_bounds(index);
292292
scope.tracker_assert_in_bounds(index);
293293

294+
let texture_data = unsafe { texture_data_from_texture(storage, index32) };
294295
unsafe {
295296
insert_or_merge(
296-
texture_data_from_texture(storage, index32),
297+
texture_data,
297298
&mut self.set,
298299
&mut self.metadata,
299300
index32,
@@ -359,9 +360,10 @@ impl<A: hub::HalApi> TextureUsageScope<A> {
359360

360361
self.tracker_assert_in_bounds(index);
361362

363+
let texture_data = unsafe { texture_data_from_texture(storage, index32) };
362364
unsafe {
363365
insert_or_merge(
364-
texture_data_from_texture(storage, index32),
366+
texture_data,
365367
&mut self.set,
366368
&mut self.metadata,
367369
index32,
@@ -467,13 +469,8 @@ impl<A: hub::HalApi> TextureTracker<A> {
467469

468470
self.tracker_assert_in_bounds(index);
469471

470-
unsafe {
471-
self.metadata
472-
.ref_counts
473-
.get_unchecked(index)
474-
.as_ref()
475-
.unwrap_unchecked()
476-
}
472+
let ref_count = unsafe { self.metadata.ref_counts.get_unchecked(index) };
473+
unsafe { ref_count.as_ref().unwrap_unchecked() }
477474
}
478475

479476
/// Inserts a single texture and a state into the resource tracker.
@@ -683,9 +680,10 @@ impl<A: hub::HalApi> TextureTracker<A> {
683680
if unsafe { !scope.metadata.owned.get(index).unwrap_unchecked() } {
684681
continue;
685682
}
683+
let texture_data = unsafe { texture_data_from_texture(storage, index32) };
686684
unsafe {
687685
insert_or_barrier_update(
688-
texture_data_from_texture(storage, index32),
686+
texture_data,
689687
Some(&mut self.start_set),
690688
&mut self.end_set,
691689
&mut self.metadata,

‎wgpu-hal/src/gles/device.rs

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,8 @@ impl super::Device {
173173
if gl.supports_debug() {
174174
//TODO: remove all transmutes from `object_label`
175175
// https://github.com/grovesNL/glow/issues/186
176-
unsafe { gl.object_label(glow::SHADER, mem::transmute(raw), label) };
176+
let name = unsafe { mem::transmute(raw) };
177+
unsafe { gl.object_label(glow::SHADER, name, label) };
177178
}
178179

179180
unsafe { gl.shader_source(raw, shader) };
@@ -276,7 +277,8 @@ impl super::Device {
276277
#[cfg(not(target_arch = "wasm32"))]
277278
if let Some(label) = label {
278279
if gl.supports_debug() {
279-
unsafe { gl.object_label(glow::PROGRAM, mem::transmute(program), Some(label)) };
280+
let name = unsafe { mem::transmute(program) };
281+
unsafe { gl.object_label(glow::PROGRAM, name, Some(label)) };
280282
}
281283
}
282284

@@ -363,12 +365,8 @@ impl super::Device {
363365
return Err(crate::DeviceError::Lost.into());
364366
}
365367
super::BindingRegister::Textures | super::BindingRegister::Images => {
366-
unsafe {
367-
gl.uniform_1_i32(
368-
gl.get_uniform_location(program, name).as_ref(),
369-
slot as _,
370-
)
371-
};
368+
let location = unsafe { gl.get_uniform_location(program, name) };
369+
unsafe { gl.uniform_1_i32(location.as_ref(), slot as _) };
372370
}
373371
}
374372
}
@@ -516,7 +514,8 @@ impl crate::Device<super::Api> for super::Device {
516514
#[cfg(not(target_arch = "wasm32"))]
517515
if let Some(label) = desc.label {
518516
if gl.supports_debug() {
519-
unsafe { gl.object_label(glow::BUFFER, mem::transmute(raw), Some(label)) };
517+
let name = unsafe { mem::transmute(raw) };
518+
unsafe { gl.object_label(glow::BUFFER, name, Some(label)) };
520519
}
521520
}
522521

@@ -660,9 +659,8 @@ impl crate::Device<super::Api> for super::Device {
660659
#[cfg(not(target_arch = "wasm32"))]
661660
if let Some(label) = desc.label {
662661
if gl.supports_debug() {
663-
unsafe {
664-
gl.object_label(glow::RENDERBUFFER, mem::transmute(raw), Some(label))
665-
};
662+
let name = unsafe { mem::transmute(raw) };
663+
unsafe { gl.object_label(glow::RENDERBUFFER, name, Some(label)) };
666664
}
667665
}
668666

@@ -728,7 +726,8 @@ impl crate::Device<super::Api> for super::Device {
728726
#[cfg(not(target_arch = "wasm32"))]
729727
if let Some(label) = desc.label {
730728
if gl.supports_debug() {
731-
unsafe { gl.object_label(glow::TEXTURE, mem::transmute(raw), Some(label)) };
729+
let name = unsafe { mem::transmute(raw) };
730+
unsafe { gl.object_label(glow::TEXTURE, name, Some(label)) };
732731
}
733732
}
734733

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

@@ -1170,9 +1170,8 @@ impl crate::Device<super::Api> for super::Device {
11701170
if let Some(label) = desc.label {
11711171
temp_string.clear();
11721172
let _ = write!(temp_string, "{}[{}]", label, i);
1173-
unsafe {
1174-
gl.object_label(glow::QUERY, mem::transmute(query), Some(&temp_string))
1175-
};
1173+
let name = unsafe { mem::transmute(query) };
1174+
unsafe { gl.object_label(glow::QUERY, name, Some(&temp_string)) };
11761175
}
11771176
}
11781177
queries.push(query);

‎wgpu-hal/src/gles/egl.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -738,8 +738,9 @@ impl crate::Instance<super::Api> for Instance {
738738
&& client_ext_str.contains("EGL_KHR_debug")
739739
{
740740
log::info!("Enabling EGL debug output");
741-
let function: EglDebugMessageControlFun = unsafe {
742-
std::mem::transmute(egl.get_proc_address("eglDebugMessageControlKHR").unwrap())
741+
let function: EglDebugMessageControlFun = {
742+
let addr = egl.get_proc_address("eglDebugMessageControlKHR").unwrap();
743+
unsafe { std::mem::transmute(addr) }
743744
};
744745
let attributes = [
745746
EGL_DEBUG_MSG_CRITICAL_KHR as egl::Attrib,
@@ -911,9 +912,10 @@ impl super::Adapter {
911912
pub unsafe fn new_external(
912913
fun: impl FnMut(&str) -> *const ffi::c_void,
913914
) -> Option<crate::ExposedAdapter<super::Api>> {
915+
let context = unsafe { glow::Context::from_loader_function(fun) };
914916
unsafe {
915917
Self::expose(AdapterContext {
916-
glow: Mutex::new(glow::Context::from_loader_function(fun)),
918+
glow: Mutex::new(context),
917919
egl: None,
918920
})
919921
}
@@ -1238,14 +1240,9 @@ impl crate::Surface<super::Api> for Surface {
12381240
.destroy_surface(self.egl.display, surface)
12391241
.unwrap();
12401242
if let Some(window) = wl_window {
1241-
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> = unsafe {
1242-
self.wsi
1243-
.library
1244-
.as_ref()
1245-
.expect("unsupported window")
1246-
.get(b"wl_egl_window_destroy")
1247-
}
1248-
.unwrap();
1243+
let library = self.wsi.library.as_ref().expect("unsupported window");
1244+
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> =
1245+
unsafe { library.get(b"wl_egl_window_destroy") }.unwrap();
12491246
unsafe { wl_egl_window_destroy(window) };
12501247
}
12511248
}

‎wgpu-hal/src/metal/surface.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ impl super::Surface {
8383
delegate: Option<&HalManagedMetalLayerDelegate>,
8484
) -> Self {
8585
let view = view as *mut Object;
86-
let render_layer = unsafe {
87-
mem::transmute::<_, &mtl::MetalLayerRef>(Self::get_metal_layer(view, delegate))
86+
let render_layer = {
87+
let layer = unsafe { Self::get_metal_layer(view, delegate) };
88+
unsafe { mem::transmute::<_, &mtl::MetalLayerRef>(layer) }
8889
}
8990
.to_owned();
9091
let _: *mut c_void = msg_send![view, retain];

‎wgpu-hal/src/vulkan/device.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,16 @@ impl super::DeviceShared {
4848
.collect();
4949
&buffer_vec
5050
};
51+
52+
let name = unsafe { CStr::from_bytes_with_nul_unchecked(name_bytes) };
53+
5154
let _result = unsafe {
5255
extension.debug_utils_set_object_name(
5356
self.raw.handle(),
5457
&vk::DebugUtilsObjectNameInfoEXT::builder()
5558
.object_type(object_type)
5659
.object_handle(object.as_raw())
57-
.object_name(CStr::from_bytes_with_nul_unchecked(name_bytes)),
60+
.object_name(name),
5861
)
5962
};
6063
}

0 commit comments

Comments
 (0)