Skip to content

Commit 150da05

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 d7bb00a commit 150da05

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
@@ -441,12 +441,9 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
441441
}
442442
ResourceMetadataProvider::Indirect { metadata } => {
443443
metadata.tracker_assert_in_bounds(index);
444-
(unsafe { *metadata.epochs.get_unchecked(index) }, unsafe {
445-
metadata
446-
.ref_counts
447-
.get_unchecked(index)
448-
.clone()
449-
.unwrap_unchecked()
444+
(unsafe { *metadata.epochs.get_unchecked(index) }, {
445+
let ref_count = unsafe { metadata.ref_counts.get_unchecked(index) };
446+
unsafe { ref_count.clone().unwrap_unchecked() }
450447
})
451448
}
452449
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
@@ -101,7 +101,8 @@ impl super::Device {
101101
if gl.supports_debug() {
102102
//TODO: remove all transmutes from `object_label`
103103
// https://github.com/grovesNL/glow/issues/186
104-
unsafe { gl.object_label(glow::SHADER, mem::transmute(raw), label) };
104+
let name = unsafe { mem::transmute(raw) };
105+
unsafe { gl.object_label(glow::SHADER, name, label) };
105106
}
106107

107108
unsafe { gl.shader_source(raw, shader) };
@@ -203,7 +204,8 @@ impl super::Device {
203204
#[cfg(not(target_arch = "wasm32"))]
204205
if let Some(label) = label {
205206
if gl.supports_debug() {
206-
unsafe { gl.object_label(glow::PROGRAM, mem::transmute(program), Some(label)) };
207+
let name = unsafe { mem::transmute(program) };
208+
unsafe { gl.object_label(glow::PROGRAM, name, Some(label)) };
207209
}
208210
}
209211

@@ -289,12 +291,8 @@ impl super::Device {
289291
return Err(crate::DeviceError::Lost.into());
290292
}
291293
super::BindingRegister::Textures | super::BindingRegister::Images => {
292-
unsafe {
293-
gl.uniform_1_i32(
294-
gl.get_uniform_location(program, name).as_ref(),
295-
slot as _,
296-
)
297-
};
294+
let location = unsafe { gl.get_uniform_location(program, name) };
295+
unsafe { gl.uniform_1_i32(location.as_ref(), slot as _) };
298296
}
299297
}
300298
}
@@ -442,7 +440,8 @@ impl crate::Device<super::Api> for super::Device {
442440
#[cfg(not(target_arch = "wasm32"))]
443441
if let Some(label) = desc.label {
444442
if gl.supports_debug() {
445-
unsafe { gl.object_label(glow::BUFFER, mem::transmute(raw), Some(label)) };
443+
let name = unsafe { mem::transmute(raw) };
444+
unsafe { gl.object_label(glow::BUFFER, name, Some(label)) };
446445
}
447446
}
448447

@@ -586,9 +585,8 @@ impl crate::Device<super::Api> for super::Device {
586585
#[cfg(not(target_arch = "wasm32"))]
587586
if let Some(label) = desc.label {
588587
if gl.supports_debug() {
589-
unsafe {
590-
gl.object_label(glow::RENDERBUFFER, mem::transmute(raw), Some(label))
591-
};
588+
let name = unsafe { mem::transmute(raw) };
589+
unsafe { gl.object_label(glow::RENDERBUFFER, name, Some(label)) };
592590
}
593591
}
594592

@@ -678,7 +676,8 @@ impl crate::Device<super::Api> for super::Device {
678676
#[cfg(not(target_arch = "wasm32"))]
679677
if let Some(label) = desc.label {
680678
if gl.supports_debug() {
681-
unsafe { gl.object_label(glow::TEXTURE, mem::transmute(raw), Some(label)) };
679+
let name = unsafe { mem::transmute(raw) };
680+
unsafe { gl.object_label(glow::TEXTURE, name, Some(label)) };
682681
}
683682
}
684683

@@ -819,7 +818,8 @@ impl crate::Device<super::Api> for super::Device {
819818
#[cfg(not(target_arch = "wasm32"))]
820819
if let Some(label) = desc.label {
821820
if gl.supports_debug() {
822-
unsafe { gl.object_label(glow::SAMPLER, mem::transmute(raw), Some(label)) };
821+
let name = unsafe { mem::transmute(raw) };
822+
unsafe { gl.object_label(glow::SAMPLER, name, Some(label)) };
823823
}
824824
}
825825

@@ -1111,9 +1111,8 @@ impl crate::Device<super::Api> for super::Device {
11111111
if let Some(label) = desc.label {
11121112
temp_string.clear();
11131113
let _ = write!(temp_string, "{}[{}]", label, i);
1114-
unsafe {
1115-
gl.object_label(glow::QUERY, mem::transmute(query), Some(&temp_string))
1116-
};
1114+
let name = unsafe { mem::transmute(query) };
1115+
unsafe { gl.object_label(glow::QUERY, name, Some(&temp_string)) };
11171116
}
11181117
}
11191118
queries.push(query);

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -742,8 +742,9 @@ impl crate::Instance<super::Api> for Instance {
742742
&& client_ext_str.contains(&"EGL_KHR_debug")
743743
{
744744
log::info!("Enabling EGL debug output");
745-
let function: EglDebugMessageControlFun = unsafe {
746-
std::mem::transmute(egl.get_proc_address("eglDebugMessageControlKHR").unwrap())
745+
let function: EglDebugMessageControlFun = {
746+
let addr = egl.get_proc_address("eglDebugMessageControlKHR").unwrap();
747+
unsafe { std::mem::transmute(addr) }
747748
};
748749
let attributes = [
749750
EGL_DEBUG_MSG_CRITICAL_KHR as egl::Attrib,
@@ -915,9 +916,10 @@ impl super::Adapter {
915916
pub unsafe fn new_external(
916917
fun: impl FnMut(&str) -> *const ffi::c_void,
917918
) -> Option<crate::ExposedAdapter<super::Api>> {
919+
let context = unsafe { glow::Context::from_loader_function(fun) };
918920
unsafe {
919921
Self::expose(AdapterContext {
920-
glow: Mutex::new(glow::Context::from_loader_function(fun)),
922+
glow: Mutex::new(context),
921923
egl: None,
922924
})
923925
}
@@ -1242,14 +1244,9 @@ impl crate::Surface<super::Api> for Surface {
12421244
.destroy_surface(self.egl.display, surface)
12431245
.unwrap();
12441246
if let Some(window) = wl_window {
1245-
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> = unsafe {
1246-
self.wsi
1247-
.library
1248-
.as_ref()
1249-
.expect("unsupported window")
1250-
.get(b"wl_egl_window_destroy")
1251-
}
1252-
.unwrap();
1247+
let library = self.wsi.library.as_ref().expect("unsupported window");
1248+
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> =
1249+
unsafe { library.get(b"wl_egl_window_destroy") }.unwrap();
12531250
unsafe { wl_egl_window_destroy(window) };
12541251
}
12551252
}

‎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)