From f781c0edd0af7ab166e10f816978ffed2761376b Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Mon, 23 Sep 2024 17:52:23 +0300 Subject: [PATCH] refctor!: mark functions as unsafe and document its safety (#227) * refctor!: mark functions as unsafe and document its safety * fix examples * fix doctests * fix example in README.md --- .changes/acccelerator-typo.md | 2 +- .changes/unsafe.md | 16 ++++ .changes/use-objc2.md | 6 +- README.md | 4 +- examples/tao.rs | 6 +- examples/winit.rs | 6 +- examples/wry.rs | 6 +- src/items/submenu.rs | 14 ++-- src/lib.rs | 28 +++++-- src/menu.rs | 63 ++++++++++++---- src/platform_impl/macos/mod.rs | 5 +- src/platform_impl/windows/mod.rs | 125 +++++++++++++++---------------- 12 files changed, 172 insertions(+), 109 deletions(-) create mode 100644 .changes/unsafe.md diff --git a/.changes/acccelerator-typo.md b/.changes/acccelerator-typo.md index 98ef5af..12717d2 100644 --- a/.changes/acccelerator-typo.md +++ b/.changes/acccelerator-typo.md @@ -1,5 +1,5 @@ --- -"muda": "patch" +"muda": "minor" --- **Breaking change** Renamed the `acccelerator` method (which has an extra `c`) on `MenuItemBuilder`, `CheckMenuItemBuilder`, and `IconMenuItemBuilder` to `accelerator`. diff --git a/.changes/unsafe.md b/.changes/unsafe.md new file mode 100644 index 0000000..f7a330c --- /dev/null +++ b/.changes/unsafe.md @@ -0,0 +1,16 @@ +--- +"muda": "minor" +--- + +**Breaking change** Marked a few methods with `unsafe` to better represent the safety guarantees: + +- `ContextMenu::show_context_menu_for_hwnd` +- `ContextMenu::attach_menu_subclass_for_hwnd` +- `ContextMenu::detach_menu_subclass_from_hwnd` +- `Menu::init_for_hwnd` +- `Menu::init_for_hwnd_with_theme` +- `Menu::set_theme_for_hwnd` +- `Menu::remove_for_hwnd` +- `Menu::hide_for_hwnd` +- `Menu::show_for_hwnd` +- `Menu::is_visible_on_hwnd` diff --git a/.changes/use-objc2.md b/.changes/use-objc2.md index 4cf7547..e7eaf20 100644 --- a/.changes/use-objc2.md +++ b/.changes/use-objc2.md @@ -1,7 +1,5 @@ --- -"muda": patch +"muda": minor --- -Use `objc2` internally, leading to much better memory safety. - -The crate will panic now if used from a thread that is not the main thread. +Use `objc2` internally, leading to much better memory safety. The crate will panic now if used from a thread that is not the main thread. diff --git a/README.md b/README.md index 55ddb39..08ccabe 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ or use it as your global app menu on macOS ```rs // --snip-- #[cfg(target_os = "windows")] -menu.init_for_hwnd(window.hwnd() as isize); +unsafe { menu.init_for_hwnd(window.hwnd() as isize) }; #[cfg(target_os = "linux")] menu.init_for_gtk_window(>k_window, Some(&vertical_gtk_box)); #[cfg(target_os = "macos")] @@ -81,7 +81,7 @@ You can also use a [`Menu`] or a [`Submenu`] show a context menu. // --snip-- let position = muda::PhysicalPosition { x: 100., y: 120. }; #[cfg(target_os = "windows")] -menu.show_context_menu_for_hwnd(window.hwnd() as isize, Some(position.into())); +unsafe { menu.show_context_menu_for_hwnd(window.hwnd() as isize, Some(position.into())) }; #[cfg(target_os = "linux")] menu.show_context_menu_for_gtk_window(>k_window, Some(position.into())); #[cfg(target_os = "macos")] diff --git a/examples/tao.rs b/examples/tao.rs index de29f15..465a524 100644 --- a/examples/tao.rs +++ b/examples/tao.rs @@ -138,7 +138,7 @@ fn main() { edit_m.append_items(&[©_i, &PredefinedMenuItem::separator(), &paste_i]); #[cfg(target_os = "windows")] - { + unsafe { menu_bar.init_for_hwnd(window.hwnd() as _); menu_bar.init_for_hwnd(window2.hwnd() as _); } @@ -221,7 +221,9 @@ fn main() { fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option) { println!("Show context menu at position {position:?}"); #[cfg(target_os = "windows")] - menu.show_context_menu_for_hwnd(window.hwnd() as _, position); + unsafe { + menu.show_context_menu_for_hwnd(window.hwnd() as _, position); + } #[cfg(target_os = "linux")] menu.show_context_menu_for_gtk_window(window.gtk_window().as_ref(), position); #[cfg(target_os = "macos")] diff --git a/examples/winit.rs b/examples/winit.rs index b8e0ded..2b82d72 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -131,10 +131,10 @@ fn main() { { use winit::raw_window_handle::*; if let RawWindowHandle::Win32(handle) = window.window_handle().unwrap().as_raw() { - menu_bar.init_for_hwnd(handle.hwnd.get()); + unsafe { menu_bar.init_for_hwnd(handle.hwnd.get()) }; } if let RawWindowHandle::Win32(handle) = window2.window_handle().unwrap().as_raw() { - menu_bar.init_for_hwnd(handle.hwnd.get()); + unsafe { menu_bar.init_for_hwnd(handle.hwnd.get()) }; } } #[cfg(target_os = "macos")] @@ -206,7 +206,7 @@ fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option

wry::Result<()> { .unwrap(); #[cfg(target_os = "windows")] - { + unsafe { menu_bar.init_for_hwnd(window.hwnd() as _).unwrap(); menu_bar.init_for_hwnd(window2.hwnd() as _).unwrap(); } @@ -306,7 +306,9 @@ fn main() -> wry::Result<()> { fn show_context_menu(window: &Window, menu: &dyn ContextMenu, position: Option) { println!("Show context menu at position {position:?}"); #[cfg(target_os = "windows")] - menu.show_context_menu_for_hwnd(window.hwnd() as _, position); + unsafe { + menu.show_context_menu_for_hwnd(window.hwnd() as _, position); + } #[cfg(target_os = "linux")] menu.show_context_menu_for_gtk_window(window.gtk_window().as_ref(), position); #[cfg(target_os = "macos")] diff --git a/src/items/submenu.rs b/src/items/submenu.rs index 0e250c2..cae6740 100644 --- a/src/items/submenu.rs +++ b/src/items/submenu.rs @@ -217,19 +217,19 @@ impl ContextMenu for Submenu { } #[cfg(target_os = "windows")] - fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option) { + unsafe fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option) { self.inner .borrow_mut() .show_context_menu_for_hwnd(hwnd, position) } #[cfg(target_os = "windows")] - fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { + unsafe fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { self.inner.borrow().attach_menu_subclass_for_hwnd(hwnd) } #[cfg(target_os = "windows")] - fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { + unsafe fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { self.inner.borrow().detach_menu_subclass_from_hwnd(hwnd) } @@ -251,11 +251,9 @@ impl ContextMenu for Submenu { view: *const std::ffi::c_void, position: Option, ) { - unsafe { - self.inner - .borrow_mut() - .show_context_menu_for_nsview(view, position) - } + self.inner + .borrow_mut() + .show_context_menu_for_nsview(view, position) } #[cfg(target_os = "macos")] diff --git a/src/lib.rs b/src/lib.rs index 4909958..6357384 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,7 +83,7 @@ //! # let vertical_gtk_box = gtk::Box::new(gtk::Orientation::Vertical, 0); //! // --snip-- //! #[cfg(target_os = "windows")] -//! menu.init_for_hwnd(window_hwnd); +//! unsafe { menu.init_for_hwnd(window_hwnd) }; //! #[cfg(target_os = "linux")] //! menu.init_for_gtk_window(>k_window, Some(&vertical_gtk_box)); //! #[cfg(target_os = "macos")] @@ -105,7 +105,7 @@ //! // --snip-- //! let position = muda::dpi::PhysicalPosition { x: 100., y: 120. }; //! #[cfg(target_os = "windows")] -//! menu.show_context_menu_for_hwnd(window_hwnd, Some(position.into())); +//! unsafe { menu.show_context_menu_for_hwnd(window_hwnd, Some(position.into())) }; //! #[cfg(target_os = "linux")] //! menu.show_context_menu_for_gtk_window(>k_window, Some(position.into())); //! #[cfg(target_os = "macos")] @@ -301,6 +301,8 @@ impl Default for MenuItemType { pub trait ContextMenu { /// Get the popup [`HMENU`] for this menu. /// + /// The returned [`HMENU`] is valid as long as the `ContextMenu` is. + /// /// [`HMENU`]: windows_sys::Win32::UI::WindowsAndMessaging::HMENU #[cfg(target_os = "windows")] fn hpopupmenu(&self) -> isize; @@ -308,19 +310,33 @@ pub trait ContextMenu { /// Shows this menu as a context menu inside a win32 window. /// /// - `position` is relative to the window top-left corner, if `None`, the cursor position is used. + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option); + unsafe fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option); /// Attach the menu subclass handler to the given hwnd /// so you can recieve events from that window using [MenuEvent::receiver] /// /// This can be used along with [`ContextMenu::hpopupmenu`] when implementing a tray icon menu. + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - fn attach_menu_subclass_for_hwnd(&self, hwnd: isize); + unsafe fn attach_menu_subclass_for_hwnd(&self, hwnd: isize); /// Remove the menu subclass handler from the given hwnd + /// + /// The view must be a pointer to a valid `NSView`. + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - fn detach_menu_subclass_from_hwnd(&self, hwnd: isize); + unsafe fn detach_menu_subclass_from_hwnd(&self, hwnd: isize); /// Shows this menu as a context menu inside a [`gtk::Window`] /// @@ -329,6 +345,8 @@ pub trait ContextMenu { fn show_context_menu_for_gtk_window(&self, w: >k::Window, position: Option); /// Get the underlying gtk menu reserved for context menus. + /// + /// The returned [`gtk::Menu`] is valid as long as the `ContextMenu` is. #[cfg(target_os = "linux")] fn gtk_context_menu(&self) -> gtk::Menu; diff --git a/src/menu.rs b/src/menu.rs index 589dfab..18d9aaa 100644 --- a/src/menu.rs +++ b/src/menu.rs @@ -196,6 +196,10 @@ impl Menu { /// Adds this menu to a win32 window. /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. + /// /// ## Note about accelerators: /// /// For accelerators to work, the event loop needs to call @@ -219,7 +223,7 @@ impl Menu { /// } /// ``` #[cfg(target_os = "windows")] - pub fn init_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn init_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { self.inner.borrow_mut().init_for_hwnd(hwnd) } @@ -228,8 +232,16 @@ impl Menu { /// See [Menu::init_for_hwnd] for more info. /// /// Note that the theme only affects the menu bar itself and not submenus or context menu. + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn init_for_hwnd_with_theme(&self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { + pub unsafe fn init_for_hwnd_with_theme( + &self, + hwnd: isize, + theme: MenuTheme, + ) -> crate::Result<()> { self.inner .borrow_mut() .init_for_hwnd_with_theme(hwnd, theme) @@ -238,14 +250,20 @@ impl Menu { /// Set a theme for the menu bar on this window. /// /// Note that the theme only affects the menu bar itself and not submenus or context menu. + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn set_theme_for_hwnd(&self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { + pub unsafe fn set_theme_for_hwnd(&self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { self.inner.borrow().set_theme_for_hwnd(hwnd, theme) } /// Returns The [`HACCEL`](windows_sys::Win32::UI::WindowsAndMessaging::HACCEL) associated with this menu /// It can be used with [`TranslateAcceleratorW`](windows_sys::Win32::UI::WindowsAndMessaging::TranslateAcceleratorW) /// in the event loop to enable accelerators + /// + /// The returned [`HACCEL`](windows_sys::Win32::UI::WindowsAndMessaging::HACCEL) is valid as long as the [Menu] is. #[cfg(target_os = "windows")] pub fn haccel(&self) -> isize { self.inner.borrow_mut().haccel() @@ -261,8 +279,12 @@ impl Menu { } /// Removes this menu from a win32 window + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn remove_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn remove_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { self.inner.borrow_mut().remove_for_hwnd(hwnd) } @@ -276,8 +298,12 @@ impl Menu { } /// Hides this menu from a win32 window + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn hide_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn hide_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { self.inner.borrow().hide_for_hwnd(hwnd) } @@ -291,8 +317,12 @@ impl Menu { } /// Shows this menu on a win32 window + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn show_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn show_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { self.inner.borrow().show_for_hwnd(hwnd) } @@ -316,8 +346,12 @@ impl Menu { } /// Returns whether this menu visible on a on a win32 window + /// + /// # Safety + /// + /// The `hwnd` must be a valid window HWND. #[cfg(target_os = "windows")] - pub fn is_visible_on_hwnd(&self, hwnd: isize) -> bool { + pub unsafe fn is_visible_on_hwnd(&self, hwnd: isize) -> bool { self.inner.borrow().is_visible_on_hwnd(hwnd) } @@ -341,19 +375,19 @@ impl ContextMenu for Menu { } #[cfg(target_os = "windows")] - fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option) { + unsafe fn show_context_menu_for_hwnd(&self, hwnd: isize, position: Option) { self.inner .borrow_mut() .show_context_menu_for_hwnd(hwnd, position) } #[cfg(target_os = "windows")] - fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { + unsafe fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { self.inner.borrow().attach_menu_subclass_for_hwnd(hwnd) } #[cfg(target_os = "windows")] - fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { + unsafe fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { self.inner.borrow().detach_menu_subclass_from_hwnd(hwnd) } @@ -375,12 +409,9 @@ impl ContextMenu for Menu { view: *const std::ffi::c_void, position: Option, ) { - // SAFETY: Upheld by caller - unsafe { - self.inner - .borrow_mut() - .show_context_menu_for_nsview(view, position) - } + self.inner + .borrow_mut() + .show_context_menu_for_nsview(view, position) } #[cfg(target_os = "macos")] diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 11659cc..8ac3b91 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -177,7 +177,7 @@ impl Menu { position: Option, ) { // SAFETY: Upheld by caller - unsafe { show_context_menu(&self.ns_menu.1, view, position) } + show_context_menu(&self.ns_menu.1, view, position) } pub fn ns_menu(&self) -> *mut std::ffi::c_void { @@ -661,8 +661,7 @@ impl MenuChild { view: *const c_void, position: Option, ) { - // SAFETY: Upheld by caller - unsafe { show_context_menu(&self.ns_menu.as_ref().unwrap().1, view, position) } + show_context_menu(&self.ns_menu.as_ref().unwrap().1, view, position) } pub fn set_as_windows_menu_for_nsapp(&self) { diff --git a/src/platform_impl/windows/mod.rs b/src/platform_impl/windows/mod.rs index 9c15077..e8af74d 100644 --- a/src/platform_impl/windows/mod.rs +++ b/src/platform_impl/windows/mod.rs @@ -113,7 +113,7 @@ pub(crate) struct Menu { impl Drop for Menu { fn drop(&mut self) { for hwnd in self.hwnds.keys().copied().collect::>() { - let _ = self.remove_for_hwnd(hwnd); + let _ = unsafe { self.remove_for_hwnd(hwnd) }; } fn remove_from_children_stores(internal_id: u32, children: &Vec>>) { @@ -328,108 +328,107 @@ impl Menu { self.hpopupmenu as _ } - pub fn init_for_hwnd_with_theme(&mut self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { + pub unsafe fn init_for_hwnd_with_theme( + &mut self, + hwnd: isize, + theme: MenuTheme, + ) -> crate::Result<()> { if self.hwnds.contains_key(&hwnd) { return Err(crate::Error::AlreadyInitialized); } self.hwnds.insert(hwnd, theme); - unsafe { - SetMenu(hwnd as _, self.hmenu); - SetWindowSubclass( - hwnd as _, - Some(menu_subclass_proc), - MENU_SUBCLASS_ID, - dwrefdata_from_obj(self), - ); - DrawMenuBar(hwnd as _); - }; + // SAFETY: HWND validity is upheld by caller + SetMenu(hwnd as _, self.hmenu); + SetWindowSubclass( + hwnd as _, + Some(menu_subclass_proc), + MENU_SUBCLASS_ID, + dwrefdata_from_obj(self), + ); + DrawMenuBar(hwnd as _); Ok(()) } - pub fn init_for_hwnd(&mut self, hwnd: isize) -> crate::Result<()> { + + pub unsafe fn init_for_hwnd(&mut self, hwnd: isize) -> crate::Result<()> { self.init_for_hwnd_with_theme(hwnd, MenuTheme::Auto) } - pub fn remove_for_hwnd(&mut self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn remove_for_hwnd(&mut self, hwnd: isize) -> crate::Result<()> { self.hwnds .remove(&hwnd) .ok_or(crate::Error::NotInitialized)?; - unsafe { - SetMenu(hwnd as _, std::ptr::null_mut()); - DrawMenuBar(hwnd as _); - } + // SAFETY: HWND validity is upheld by caller + SetMenu(hwnd as _, std::ptr::null_mut()); + DrawMenuBar(hwnd as _); Ok(()) } - pub fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { - unsafe { - SetWindowSubclass( - hwnd as _, - Some(menu_subclass_proc), - MENU_SUBCLASS_ID, - dwrefdata_from_obj(self), - ); - } + pub unsafe fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { + // SAFETY: HWND validity is upheld by caller + SetWindowSubclass( + hwnd as _, + Some(menu_subclass_proc), + MENU_SUBCLASS_ID, + dwrefdata_from_obj(self), + ); } - pub fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { - unsafe { - RemoveWindowSubclass(hwnd as _, Some(menu_subclass_proc), MENU_SUBCLASS_ID); - } + pub unsafe fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { + // SAFETY: HWND validity is upheld by caller + RemoveWindowSubclass(hwnd as _, Some(menu_subclass_proc), MENU_SUBCLASS_ID); } - pub fn hide_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn hide_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { if !self.hwnds.contains_key(&hwnd) { return Err(crate::Error::NotInitialized); } - unsafe { - SetMenu(hwnd as _, std::ptr::null_mut()); - DrawMenuBar(hwnd as _); - } + // SAFETY: HWND validity is upheld by caller + SetMenu(hwnd as _, std::ptr::null_mut()); + DrawMenuBar(hwnd as _); Ok(()) } - pub fn show_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { + pub unsafe fn show_for_hwnd(&self, hwnd: isize) -> crate::Result<()> { if !self.hwnds.contains_key(&hwnd) { return Err(crate::Error::NotInitialized); } - unsafe { - SetMenu(hwnd as _, self.hmenu); - DrawMenuBar(hwnd as _); - } + // SAFETY: HWND validity is upheld by caller + SetMenu(hwnd as _, self.hmenu); + DrawMenuBar(hwnd as _); Ok(()) } - pub fn is_visible_on_hwnd(&self, hwnd: isize) -> bool { + pub unsafe fn is_visible_on_hwnd(&self, hwnd: isize) -> bool { self.hwnds .get(&hwnd) + // SAFETY: HWND validity is upheld by caller .map(|_| !unsafe { GetMenu(hwnd as _) }.is_null()) .unwrap_or(false) } - pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { + pub unsafe fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { let rc = show_context_menu(hwnd as _, self.hpopupmenu, position); if let Some(item) = rc.and_then(|rc| self.find_by_id(rc)) { - unsafe { - menu_selected(hwnd as _, &mut item.borrow_mut()); - } + menu_selected(hwnd as _, &mut item.borrow_mut()); } } - pub fn set_theme_for_hwnd(&self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { + pub unsafe fn set_theme_for_hwnd(&self, hwnd: isize, theme: MenuTheme) -> crate::Result<()> { if !self.hwnds.contains_key(&hwnd) { return Err(crate::Error::NotInitialized); } - unsafe { SendMessageW(hwnd as _, MENU_UPDATE_THEME, 0, theme as _) }; + // SAFETY: HWND validity is upheld by caller + SendMessageW(hwnd as _, MENU_UPDATE_THEME, 0, theme as _); Ok(()) } @@ -922,7 +921,7 @@ impl MenuChild { .collect() } - pub fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { + pub unsafe fn show_context_menu_for_hwnd(&mut self, hwnd: isize, position: Option) { let rc = show_context_menu(hwnd as _, self.hpopupmenu, position); if let Some(item) = rc.and_then(|rc| self.find_by_id(rc)) { unsafe { @@ -931,21 +930,19 @@ impl MenuChild { } } - pub fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { - unsafe { - SetWindowSubclass( - hwnd as _, - Some(menu_subclass_proc), - SUBMENU_SUBCLASS_ID, - dwrefdata_from_obj(self), - ); - } + pub unsafe fn attach_menu_subclass_for_hwnd(&self, hwnd: isize) { + // SAFETY: HWND validity is upheld by caller + SetWindowSubclass( + hwnd as _, + Some(menu_subclass_proc), + SUBMENU_SUBCLASS_ID, + dwrefdata_from_obj(self), + ); } - pub fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { - unsafe { - RemoveWindowSubclass(hwnd as _, Some(menu_subclass_proc), SUBMENU_SUBCLASS_ID); - } + pub unsafe fn detach_menu_subclass_from_hwnd(&self, hwnd: isize) { + // SAFETY: HWND validity is upheld by caller + RemoveWindowSubclass(hwnd as _, Some(menu_subclass_proc), SUBMENU_SUBCLASS_ID); } } @@ -973,7 +970,9 @@ fn find_by_id(id: u32, children: &Vec>>) -> Option,