Make raw handle traits safe to implement

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * **Breaking:** `HasRaw(Display/Window)Handle::raw_(display/window)_handle` returns a result indicating if fetching the window handle failed (#122).
 * **Breaking:** Remove the `Active/ActiveHandle` types from the public API (#126).
+* **Breaking:** It is now safe to implement `HasRaw(Display/Window)Handle` (#130).
 
 ## 0.5.2 (2023-03-31)
 

src/borrowed.rs

@@ -26,8 +26,13 @@ use crate::{
 ///
 /// # Safety
 ///
-/// The safety requirements of [`HasRawDisplayHandle`] apply here as well. To reiterate, the
-/// [`DisplayHandle`] must contain a valid window handle for its 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, 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.
 ///
 /// It is not possible to invalidate a [`DisplayHandle`] on any platform without additional unsafe code.
 ///
@@ -95,7 +100,8 @@ impl<'a> DisplayHandle<'a> {
     ///
     /// # Safety
     ///
-    /// The `RawDisplayHandle` must be valid for the lifetime.
+    /// The `RawDisplayHandle` must be valid for the lifetime. See the documentation on
+    /// [`HasDisplayHandle`] for more information.
     pub unsafe fn borrow_raw(raw: RawDisplayHandle) -> Self {
         Self {
             raw,
@@ -104,7 +110,7 @@ impl<'a> DisplayHandle<'a> {
     }
 }
 
-unsafe impl HasRawDisplayHandle for DisplayHandle<'_> {
+impl HasRawDisplayHandle for DisplayHandle<'_> {
     fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
         Ok(self.raw)
     }
@@ -133,6 +139,14 @@ impl<'a> HasDisplayHandle for DisplayHandle<'a> {
 ///
 /// # Safety
 ///
+/// 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.
+///
 /// All pointers within the resulting [`WindowHandle`] must be valid and not dangling for the lifetime of
 /// the handle.
 ///
@@ -214,7 +228,8 @@ impl<'a> WindowHandle<'a> {
     ///
     /// # Safety
     ///
-    /// The [`RawWindowHandle`] must be valid for the lifetime provided.
+    /// The [`RawWindowHandle`] must be valid for the lifetime provided. See the documentation on
+    /// [`HasWindowHandle`] for more information.
     pub unsafe fn borrow_raw(raw: RawWindowHandle) -> Self {
         Self {
             raw,
@@ -223,7 +238,7 @@ impl<'a> WindowHandle<'a> {
     }
 }
 
-unsafe impl HasRawWindowHandle for WindowHandle<'_> {
+impl HasRawWindowHandle for WindowHandle<'_> {
     fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
         Ok(self.raw)
     }

src/lib.rs

@@ -60,35 +60,30 @@ use core::fmt;
 ///
 /// # Safety
 ///
-/// 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.
+/// The window handle returned by this trait is generally unsafe to manipulate. For instance,
+/// passing the window to any platform specific APIs can result in undefined behavior. Less
+/// general guarantees are placed on the return type for [`HasWindowHandle`].
 ///
-/// 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.
-///
-/// 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.
-pub unsafe trait HasRawWindowHandle {
+/// However, one potential use case is using the raw handle as a key in a hash map.
+pub trait HasRawWindowHandle {
     fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError>;
 }
 
-unsafe impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T {
+impl<'a, T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for &'a T {
     fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
         (*self).raw_window_handle()
     }
 }
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
-unsafe impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::rc::Rc<T> {
+impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::rc::Rc<T> {
     fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
         (**self).raw_window_handle()
     }
 }
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
-unsafe impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::sync::Arc<T> {
+impl<T: HasRawWindowHandle + ?Sized> HasRawWindowHandle for alloc::sync::Arc<T> {
     fn raw_window_handle(&self) -> Result<RawWindowHandle, HandleError> {
         (**self).raw_window_handle()
     }
@@ -200,37 +195,32 @@ pub enum RawWindowHandle {
 ///
 /// # Safety
 ///
-/// 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.
+/// The display handle returned by this crate is generally unsafe to manipulate. For instance,
+/// passing the display to any platform specific APIs can result in undefined behavior. Less
+/// general guarantees are placed on the return type for [`HasDisplayHandle`].
 ///
-/// 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.
-pub unsafe trait HasRawDisplayHandle {
+/// However, one potential use case is using the raw handle as a key in a hash map.
+pub trait HasRawDisplayHandle {
     fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError>;
 }
 
-unsafe impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a T {
+impl<'a, T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for &'a T {
     fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
         (*self).raw_display_handle()
     }
 }
 
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
-unsafe impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::rc::Rc<T> {
+impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::rc::Rc<T> {
     fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
         (**self).raw_display_handle()
     }
 }
 
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
-unsafe impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::sync::Arc<T> {
+impl<T: HasRawDisplayHandle + ?Sized> HasRawDisplayHandle for alloc::sync::Arc<T> {
     fn raw_display_handle(&self) -> Result<RawDisplayHandle, HandleError> {
         (**self).raw_display_handle()
     }