From e3f68a768e462151181c9ccca3f5ca35ebbd3d45 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sun, 10 Sep 2023 13:05:59 -0700 Subject: [PATCH] breaking: Deprecate the HasRaw*Handle traits After reading #135, I've realized that the HasRawDisplayHandle and HasRawWindowHandle traits have little value in a post-borrowed-handle world. Borrowed handles do everything better, and the raw handle can be extracted from the borrowed handle. Therefore it makes sense to remove these traits. Thus far I've deprecated them instead of removing them, in order to be more kind to downstream crates. Closes #135. Signed-off-by: John Nunley --- .github/workflows/ci.yml | 2 +- src/borrowed.rs | 126 +++++++++++++++++++++++++-------------- src/lib.rs | 59 ++++-------------- 3 files changed, 92 insertions(+), 95 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 36f629c..9caf99a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,7 +39,6 @@ jobs: - uses: hecrj/setup-rust-action@v1 with: rust-version: ${{ matrix.rust_version }} - - run: rustup target add wasm32-unknown-unknown - name: Check documentation @@ -52,3 +51,4 @@ jobs: - name: Run tests for wasm32-unknown-unknown run: cargo hack check --target wasm32-unknown-unknown --feature-powerset + diff --git a/src/borrowed.rs b/src/borrowed.rs index 9ee9d58..567e6af 100644 --- a/src/borrowed.rs +++ b/src/borrowed.rs @@ -2,12 +2,11 @@ //! //! These should be 100% safe to pass around and use, no possibility of dangling or invalidity. +use core::borrow::Borrow; use core::fmt; use core::marker::PhantomData; -use crate::{ - HandleError, HasRawDisplayHandle, HasRawWindowHandle, RawDisplayHandle, RawWindowHandle, -}; +use crate::{HandleError, RawDisplayHandle, RawWindowHandle}; /// A display that acts as a wrapper around a display handle. /// @@ -16,7 +15,7 @@ use crate::{ /// return an error if the application is inactive. /// /// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing -/// systems should implement this trait on types that already implement [`HasRawDisplayHandle`]. It +/// systems should implement this trait on types that represent the top-level display server. It /// should be implemented by tying the lifetime of the [`DisplayHandle`] to the lifetime of the /// display object. /// @@ -24,17 +23,9 @@ use crate::{ /// should be generic over a type that implements `HasDisplayHandle`, and should use the /// [`DisplayHandle`] type to access the display handle. /// -/// # Safety -/// -/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the -/// [`DisplayHandle`] must contain a valid window handle for its lifetime. -/// -/// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code. -/// /// Note that these requirements are not enforced on `HasDisplayHandle`, rather, they are enforced on the /// constructors of [`DisplayHandle`]. This is because the `HasDisplayHandle` trait is safe to implement. /// -/// [`HasRawDisplayHandle`]: crate::HasRawDisplayHandle /// [`winit`]: https://crates.io/crates/winit /// [`sdl2`]: https://crates.io/crates/sdl2 /// [`wgpu`]: https://crates.io/crates/wgpu @@ -57,6 +48,7 @@ impl HasDisplayHandle for &mut H { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::boxed::Box { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -64,6 +56,7 @@ impl HasDisplayHandle for alloc::boxed::Box { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::rc::Rc { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -71,6 +64,7 @@ impl HasDisplayHandle for alloc::rc::Rc { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasDisplayHandle for alloc::sync::Arc { fn display_handle(&self) -> Result, HandleError> { (**self).display_handle() @@ -81,8 +75,6 @@ impl HasDisplayHandle for alloc::sync::Arc { /// /// This is the primary return type of the [`HasDisplayHandle`] trait. It is guaranteed to contain /// a valid platform-specific display handle for its lifetime. -/// -/// Get the underlying raw display handle with the [`HasRawDisplayHandle`] trait. #[repr(transparent)] #[derive(PartialEq, Eq, Hash, Copy, Clone)] pub struct DisplayHandle<'a> { @@ -101,18 +93,43 @@ impl<'a> DisplayHandle<'a> { /// /// # Safety /// - /// The `RawDisplayHandle` must be valid for the lifetime. + /// Users can safely assume that non-`null`/`0` fields are valid handles, and it is up to the + /// implementer of this trait to ensure that condition is upheld. + /// + /// Despite that qualification, implementors should still make a best-effort attempt to fill in all + /// available fields. If an implementation doesn't, and a downstream user needs the field, it should + /// try to derive the field from other fields the implementer *does* provide via whatever methods the + /// platform provides. + /// + /// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code. pub unsafe fn borrow_raw(raw: RawDisplayHandle) -> Self { Self { raw, _marker: PhantomData, } } + + /// Get the underlying raw display handle. + pub fn as_raw(&self) -> RawDisplayHandle { + self.raw + } } -unsafe impl HasRawDisplayHandle for DisplayHandle<'_> { - fn raw_display_handle(&self) -> Result { - Ok(self.raw) +impl AsRef for DisplayHandle<'_> { + fn as_ref(&self) -> &RawDisplayHandle { + &self.raw + } +} + +impl Borrow for DisplayHandle<'_> { + fn borrow(&self) -> &RawDisplayHandle { + &self.raw + } +} + +impl From> for RawDisplayHandle { + fn from(handle: DisplayHandle<'_>) -> Self { + handle.raw } } @@ -129,7 +146,7 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// return an error if the application is inactive. /// /// Implementors of this trait will be windowing systems, like [`winit`] and [`sdl2`]. These windowing -/// systems should implement this trait on types that already implement [`HasRawWindowHandle`]. +/// systems should implement this trait on types that represent windows. /// /// Users of this trait will include graphics libraries, like [`wgpu`] and [`glutin`]. These APIs /// should be generic over a type that implements `HasWindowHandle`, and should use the @@ -137,27 +154,6 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> { /// while the window is being used, in order to ensure that the window is not deleted while it is in /// use. /// -/// # Safety -/// -/// All pointers within the resulting [`WindowHandle`] must be valid and not dangling for the lifetime of -/// the handle. -/// -/// Note that this guarantee only applies to *pointers*, and not any window ID types in the handle. -/// This includes Window IDs (XIDs) from X11 and the window ID for web platforms. There is no way for -/// Rust to enforce any kind of invariant on these types, since: -/// -/// - For all three listed platforms, it is possible for safe code in the same process to delete -/// the window. -/// - For X11, it is possible for code in a different process to delete the window. In fact, it is -/// possible for code on a different *machine* to delete the window. -/// -/// It is *also* possible for the window to be replaced with another, valid-but-different window. User -/// code should be aware of this possibility, and should be ready to soundly handle the possible error -/// conditions that can arise from this. -/// -/// Note that these requirements are not enforced on `HasWindowHandle`, rather, they are enforced on the -/// constructors of [`WindowHandle`]. This is because the `HasWindowHandle` trait is safe to implement. -/// /// [`winit`]: https://crates.io/crates/winit /// [`sdl2`]: https://crates.io/crates/sdl2 /// [`wgpu`]: https://crates.io/crates/wgpu @@ -180,6 +176,7 @@ impl HasWindowHandle for &mut H { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::boxed::Box { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -187,6 +184,7 @@ impl HasWindowHandle for alloc::boxed::Box { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::rc::Rc { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -194,6 +192,7 @@ impl HasWindowHandle for alloc::rc::Rc { } #[cfg(feature = "alloc")] +#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] impl HasWindowHandle for alloc::sync::Arc { fn window_handle(&self) -> Result, HandleError> { (**self).window_handle() @@ -207,8 +206,7 @@ impl HasWindowHandle for alloc::sync::Arc { /// like XIDs and the window ID for web platforms. See the documentation on the [`HasWindowHandle`] /// trait for more information about these safety requirements. /// -/// This handle is guaranteed to be safe and valid. Get the underlying raw window handle with the -/// [`HasRawWindowHandle`] trait. +/// This handle is guaranteed to be safe and valid. #[derive(PartialEq, Eq, Hash, Copy, Clone)] pub struct WindowHandle<'a> { raw: RawWindowHandle, @@ -226,18 +224,54 @@ impl<'a> WindowHandle<'a> { /// /// # Safety /// - /// The [`RawWindowHandle`] must be valid for the lifetime provided. + /// Users can safely assume that non-`null`/`0` fields are valid handles, and it is up to the + /// implementer of this trait to ensure that condition is upheld. + /// + /// Despite that qualification, implementers should still make a best-effort attempt to fill in all + /// available fields. If an implementation doesn't, and a downstream user needs the field, it should + /// try to derive the field from other fields the implementer *does* provide via whatever methods the + /// platform provides. + /// + /// Note that this guarantee only applies to *pointers*, and not any window ID types in the handle. + /// This includes Window IDs (XIDs) from X11 and the window ID for web platforms. There is no way for + /// Rust to enforce any kind of invariant on these types, since: + /// + /// - For all three listed platforms, it is possible for safe code in the same process to delete + /// the window. + /// - For X11, it is possible for code in a different process to delete the window. In fact, it is + /// possible for code on a different *machine* to delete the window. + /// + /// It is *also* possible for the window to be replaced with another, valid-but-different window. User + /// code should be aware of this possibility, and should be ready to soundly handle the possible error + /// conditions that can arise from this. pub unsafe fn borrow_raw(raw: RawWindowHandle) -> Self { Self { raw, _marker: PhantomData, } } + + /// Get the underlying raw window handle. + pub fn as_raw(&self) -> RawWindowHandle { + self.raw.clone() + } +} + +impl AsRef for WindowHandle<'_> { + fn as_ref(&self) -> &RawWindowHandle { + &self.raw + } +} + +impl Borrow for WindowHandle<'_> { + fn borrow(&self) -> &RawWindowHandle { + &self.raw + } } -unsafe impl HasRawWindowHandle for WindowHandle<'_> { - fn raw_window_handle(&self) -> Result { - Ok(self.raw) +impl From> for RawWindowHandle { + fn from(handle: WindowHandle<'_>) -> Self { + handle.raw } } diff --git a/src/lib.rs b/src/lib.rs index 4bf6912..c00b27a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,7 @@ //! //! ## Safety guarantees //! -//! Please see the docs of [`HasRawWindowHandle`] and [`HasRawDisplayHandle`]. +//! Please see the docs of [`HasWindowHandle`] and [`HasDisplayHandle`]. //! //! ## Platform handle initialization //! @@ -74,32 +74,15 @@ use core::fmt; /// /// The exact handles returned by `raw_window_handle` must remain consistent between multiple calls /// to `raw_window_handle` as long as not indicated otherwise by platform specific events. +#[deprecated = "Use `HasWindowHandle` instead"] pub unsafe trait HasRawWindowHandle { fn raw_window_handle(&self) -> Result; } -unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T { +#[allow(deprecated)] +unsafe impl HasRawWindowHandle for T { fn raw_window_handle(&self) -> Result { - (*self).raw_window_handle() - } -} -unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a mut T { - fn raw_window_handle(&self) -> Result { - (**self).raw_window_handle() - } -} -#[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -unsafe impl HasRawWindowHandle for alloc::rc::Rc { - fn raw_window_handle(&self) -> Result { - (**self).raw_window_handle() - } -} -#[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -unsafe impl HasRawWindowHandle for alloc::sync::Arc { - fn raw_window_handle(&self) -> Result { - (**self).raw_window_handle() + self.window_handle().map(Into::into) } } @@ -119,7 +102,7 @@ unsafe impl HasRawWindowHandle for alloc::sync:: /// some hints on where this variant might be expected. /// /// Note that these "Availability Hints" are not normative. That is to say, a -/// [`HasRawWindowHandle`] implementor is completely allowed to return something +/// [`HasWindowHandle`] implementor is completely allowed to return something /// unexpected. (For example, it's legal for someone to return a /// [`RawWindowHandle::Xlib`] on macOS, it would just be weird, and probably /// requires something like XQuartz be used). @@ -233,35 +216,15 @@ pub enum RawWindowHandle { /// /// The exact handles returned by `raw_display_handle` must remain consistent between multiple calls /// to `raw_display_handle` as long as not indicated otherwise by platform specific events. +#[deprecated = "Use `HasDisplayHandle` instead"] pub unsafe trait HasRawDisplayHandle { fn raw_display_handle(&self) -> Result; } -unsafe impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a T { - fn raw_display_handle(&self) -> Result { - (*self).raw_display_handle() - } -} - -unsafe impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a mut T { - fn raw_display_handle(&self) -> Result { - (**self).raw_display_handle() - } -} - -#[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -unsafe impl HasRawDisplayHandle for alloc::rc::Rc { - fn raw_display_handle(&self) -> Result { - (**self).raw_display_handle() - } -} - -#[cfg(feature = "alloc")] -#[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] -unsafe impl HasRawDisplayHandle for alloc::sync::Arc { +#[allow(deprecated)] +unsafe impl HasRawDisplayHandle for T { fn raw_display_handle(&self) -> Result { - (**self).raw_display_handle() + self.display_handle().map(Into::into) } } @@ -287,7 +250,7 @@ unsafe impl HasRawDisplayHandle for alloc::sync /// some hints on where this variant might be expected. /// /// Note that these "Availability Hints" are not normative. That is to say, a -/// [`HasRawDisplayHandle`] implementor is completely allowed to return something +/// [`HasDisplayHandle`] implementor is completely allowed to return something /// unexpected. (For example, it's legal for someone to return a /// [`RawDisplayHandle::Xlib`] on macOS, it would just be weird, and probably /// requires something like XQuartz be used).