Skip to content

Commit 77b0db9

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 b8b18ab commit 77b0db9

File tree

5 files changed

+39
-45
lines changed

5 files changed

+39
-45
lines changed

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -420,12 +420,9 @@ impl<A: hub::HalApi> ResourceMetadataProvider<'_, A> {
420420
}
421421
ResourceMetadataProvider::Indirect { metadata } => {
422422
metadata.tracker_assert_in_bounds(index);
423-
(unsafe { *metadata.epochs.get_unchecked(index) }, unsafe {
424-
metadata
425-
.ref_counts
426-
.get_unchecked(index)
427-
.clone()
428-
.unwrap_unchecked()
423+
(unsafe { *metadata.epochs.get_unchecked(index) }, {
424+
let ref_count = unsafe { metadata.ref_counts.get_unchecked(index) };
425+
unsafe { ref_count.clone().unwrap_unchecked() }
429426
})
430427
}
431428
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,
@@ -913,9 +914,10 @@ impl super::Adapter {
913914
pub unsafe fn new_external(
914915
fun: impl FnMut(&str) -> *const ffi::c_void,
915916
) -> Option<crate::ExposedAdapter<super::Api>> {
917+
let context = unsafe { glow::Context::from_loader_function(fun) };
916918
unsafe {
917919
Self::expose(AdapterContext {
918-
glow: Mutex::new(glow::Context::from_loader_function(fun)),
920+
glow: Mutex::new(context),
919921
egl: None,
920922
})
921923
}
@@ -1240,14 +1242,9 @@ impl crate::Surface<super::Api> for Surface {
12401242
.destroy_surface(self.egl.display, surface)
12411243
.unwrap();
12421244
if let Some(window) = wl_window {
1243-
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> = unsafe {
1244-
self.wsi
1245-
.library
1246-
.as_ref()
1247-
.expect("unsupported window")
1248-
.get(b"wl_egl_window_destroy")
1249-
}
1250-
.unwrap();
1245+
let library = self.wsi.library.as_ref().expect("unsupported window");
1246+
let wl_egl_window_destroy: libloading::Symbol<WlEglWindowDestroyFun> =
1247+
unsafe { library.get(b"wl_egl_window_destroy") }.unwrap();
12511248
unsafe { wl_egl_window_destroy(window) };
12521249
}
12531250
}

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