Skip to content

Commit e67cfdf

Browse files
authored
Enable clippy::undocumented_unsafe_blocks warning across the workspace (#10646)
# Objective Enables warning on `clippy::undocumented_unsafe_blocks` across the workspace rather than only in `bevy_ecs`, `bevy_transform` and `bevy_utils`. This adds a little awkwardness in a few areas of code that have trivial safety or explain safety for multiple unsafe blocks with one comment however automatically prevents these comments from being missed. ## Solution This adds `undocumented_unsafe_blocks = "warn"` to the workspace `Cargo.toml` and fixes / adds a few missed safety comments. I also added `#[allow(clippy::undocumented_unsafe_blocks)]` where the safety is explained somewhere above. There are a couple of safety comments I added I'm not 100% sure about in `bevy_animation` and `bevy_render/src/view` and I'm not sure about the use of `#[allow(clippy::undocumented_unsafe_blocks)]` compared to adding comments like `// SAFETY: See above`.
1 parent 897b13b commit e67cfdf

File tree

13 files changed

+28
-16
lines changed

13 files changed

+28
-16
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ members = [
3232
[workspace.lints.clippy]
3333
type_complexity = "allow"
3434
doc_markdown = "warn"
35+
undocumented_unsafe_blocks = "warn"
3536

3637
[lints]
3738
workspace = true

crates/bevy_animation/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,7 @@ fn apply_animation(
646646
let Ok(mut transform) = (unsafe { transforms.get_unchecked(target) }) else {
647647
continue;
648648
};
649+
// SAFETY: As above, there can't be other AnimationPlayers with this target so this fetch can't alias
649650
let mut morphs = unsafe { morphs.get_unchecked(target) };
650651
for curve in curves {
651652
// Some curves have only one keyframe used to set a transform

crates/bevy_ecs/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#![warn(clippy::undocumented_unsafe_blocks)]
21
#![warn(missing_docs)]
32
#![doc = include_str!("../README.md")]
43

crates/bevy_mikktspace/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(clippy::all)]
1+
#![allow(clippy::all, clippy::undocumented_unsafe_blocks)]
22

33
use glam::{Vec2, Vec3};
44

crates/bevy_ptr/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,14 +244,14 @@ impl<'a, A: IsAligned> PtrMut<'a, A> {
244244
/// Gets a [`PtrMut`] from this with a smaller lifetime.
245245
#[inline]
246246
pub fn reborrow(&mut self) -> PtrMut<'_, A> {
247-
// SAFE: the ptrmut we're borrowing from is assumed to be valid
247+
// SAFETY: the ptrmut we're borrowing from is assumed to be valid
248248
unsafe { PtrMut::new(self.0) }
249249
}
250250

251251
/// Gets an immutable reference from this mutable reference
252252
#[inline]
253253
pub fn as_ref(&self) -> Ptr<'_, A> {
254-
// SAFE: The `PtrMut` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
254+
// SAFETY: The `PtrMut` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
255255
unsafe { Ptr::new(self.0) }
256256
}
257257
}
@@ -327,14 +327,14 @@ impl<'a, A: IsAligned> OwningPtr<'a, A> {
327327
/// Gets an immutable pointer from this owned pointer.
328328
#[inline]
329329
pub fn as_ref(&self) -> Ptr<'_, A> {
330-
// SAFE: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
330+
// SAFETY: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
331331
unsafe { Ptr::new(self.0) }
332332
}
333333

334334
/// Gets a mutable pointer from this owned pointer.
335335
#[inline]
336336
pub fn as_mut(&mut self) -> PtrMut<'_, A> {
337-
// SAFE: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
337+
// SAFETY: The `Owning` type's guarantees about the validity of this pointer are a superset of `Ptr` s guarantees
338338
unsafe { PtrMut::new(self.0) }
339339
}
340340
}

crates/bevy_render/src/lib.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ impl Plugin for RenderPlugin {
251251
app.insert_resource(FutureRendererResources(
252252
future_renderer_resources_wrapper.clone(),
253253
));
254+
// SAFETY: Plugins should be set up on the main thread.
254255
unsafe { initialize_render_app(app) };
255256
}
256257
RenderCreation::Automatic(render_creation) => {
@@ -271,8 +272,8 @@ impl Plugin for RenderPlugin {
271272
backends,
272273
dx12_shader_compiler: settings.dx12_shader_compiler.clone(),
273274
});
275+
// SAFETY: Plugins should be set up on the main thread.
274276
let surface = primary_window.map(|wrapper| unsafe {
275-
// SAFETY: Plugins should be set up on the main thread.
276277
let handle = wrapper.get_handle();
277278
instance
278279
.create_surface(&handle)
@@ -313,6 +314,7 @@ impl Plugin for RenderPlugin {
313314
#[cfg(not(target_arch = "wasm32"))]
314315
futures_lite::future::block_on(async_renderer);
315316

317+
// SAFETY: Plugins should be set up on the main thread.
316318
unsafe { initialize_render_app(app) };
317319
}
318320
}
@@ -453,7 +455,7 @@ unsafe fn initialize_render_app(app: &mut App) {
453455
"An entity was spawned after the entity list was cleared last frame and before the extract schedule began. This is not supported",
454456
);
455457

456-
// This is safe given the clear_entities call in the past frame and the assert above
458+
// SAFETY: This is safe given the clear_entities call in the past frame and the assert above
457459
unsafe {
458460
render_app
459461
.world

crates/bevy_render/src/render_resource/resource_macros.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ macro_rules! render_resource_wrapper {
5858
// If in future there is a case where a wrapper is required for a non-send/sync type
5959
// we can implement a macro variant that omits these manual Send + Sync impls
6060
unsafe impl Send for $wrapper_type {}
61+
// SAFETY: As explained above, we ensure correctness by checking that $wgpu_type implements Send and Sync.
6162
unsafe impl Sync for $wrapper_type {}
6263
const _: () = {
6364
trait AssertSendSyncBound: Send + Sync {}

crates/bevy_render/src/view/window/mod.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -252,12 +252,15 @@ pub fn prepare_windows(
252252
let surface_data = window_surfaces
253253
.surfaces
254254
.entry(window.entity)
255-
.or_insert_with(|| unsafe {
256-
// NOTE: On some OSes this MUST be called from the main thread.
257-
// As of wgpu 0.15, only fallible if the given window is a HTML canvas and obtaining a WebGPU or WebGL2 context fails.
258-
let surface = render_instance
259-
.create_surface(&window.handle.get_handle())
260-
.expect("Failed to create wgpu surface");
255+
.or_insert_with(|| {
256+
// SAFETY: The window handles in ExtractedWindows will always be valid objects to create surfaces on
257+
let surface = unsafe {
258+
// NOTE: On some OSes this MUST be called from the main thread.
259+
// As of wgpu 0.15, only fallible if the given window is a HTML canvas and obtaining a WebGPU or WebGL2 context fails.
260+
render_instance
261+
.create_surface(&window.handle.get_handle())
262+
.expect("Failed to create wgpu surface")
263+
};
261264
let caps = surface.get_capabilities(&render_adapter);
262265
let formats = caps.formats;
263266
// For future HDR output support, we'll need to request a format that supports HDR,

crates/bevy_tasks/src/task_pool.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,12 +353,16 @@ impl TaskPool {
353353
// Any usages of the references passed into `Scope` must be accessed through
354354
// the transmuted reference for the rest of this function.
355355
let executor: &async_executor::Executor = &self.executor;
356+
// SAFETY: As above, all futures must complete in this function so we can change the lifetime
356357
let executor: &'env async_executor::Executor = unsafe { mem::transmute(executor) };
358+
// SAFETY: As above, all futures must complete in this function so we can change the lifetime
357359
let external_executor: &'env ThreadExecutor<'env> =
358360
unsafe { mem::transmute(external_executor) };
361+
// SAFETY: As above, all futures must complete in this function so we can change the lifetime
359362
let scope_executor: &'env ThreadExecutor<'env> = unsafe { mem::transmute(scope_executor) };
360363
let spawned: ConcurrentQueue<FallibleTask<T>> = ConcurrentQueue::unbounded();
361364
// shadow the variable so that the owned value cannot be used for the rest of the function
365+
// SAFETY: As above, all futures must complete in this function so we can change the lifetime
362366
let spawned: &'env ConcurrentQueue<
363367
FallibleTask<Result<T, Box<(dyn std::any::Any + Send)>>>,
364368
> = unsafe { mem::transmute(&spawned) };
@@ -373,6 +377,7 @@ impl TaskPool {
373377
};
374378

375379
// shadow the variable so that the owned value cannot be used for the rest of the function
380+
// SAFETY: As above, all futures must complete in this function so we can change the lifetime
376381
let scope: &'env Scope<'_, 'env, T> = unsafe { mem::transmute(&scope) };
377382

378383
f(scope);

crates/bevy_transform/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(missing_docs)]
2-
#![warn(clippy::undocumented_unsafe_blocks)]
32
#![doc = include_str!("../README.md")]
43

54
pub mod commands;

crates/bevy_ui/src/ui_node.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,7 @@ pub struct GridPlacement {
13091309
impl GridPlacement {
13101310
pub const DEFAULT: Self = Self {
13111311
start: None,
1312+
// SAFETY: This is trivially safe as 1 is non-zero.
13121313
span: Some(unsafe { NonZeroU16::new_unchecked(1) }),
13131314
end: None,
13141315
};

crates/bevy_utils/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
//!
55
66
#![warn(missing_docs)]
7-
#![warn(clippy::undocumented_unsafe_blocks)]
87

98
#[allow(missing_docs)]
109
pub mod prelude {

crates/bevy_window/src/raw_handle.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ impl RawHandleWrapper {
3434
// A recommendation for this pattern (and more context) is available here:
3535
// https://github.com/rust-windowing/raw-window-handle/issues/59
3636
unsafe impl Send for RawHandleWrapper {}
37+
// SAFETY: This is safe for the same reasons as the Send impl above.
3738
unsafe impl Sync for RawHandleWrapper {}
3839

3940
/// A [`RawHandleWrapper`] that cannot be sent across threads.

0 commit comments

Comments
 (0)