From 5a4d16410fe4e670099b57e69287ff909a6b1ec9 Mon Sep 17 00:00:00 2001 From: Kenny Kerr Date: Wed, 28 Sep 2022 14:49:48 -0500 Subject: [PATCH 1/3] const hstring --- .../libs/windows/src/core/strings/hstring.rs | 58 +++++++++++-------- crates/tests/core/tests/hstring.rs | 1 + 2 files changed, 35 insertions(+), 24 deletions(-) diff --git a/crates/libs/windows/src/core/strings/hstring.rs b/crates/libs/windows/src/core/strings/hstring.rs index aa08fe2202..b118331209 100644 --- a/crates/libs/windows/src/core/strings/hstring.rs +++ b/crates/libs/windows/src/core/strings/hstring.rs @@ -3,44 +3,43 @@ use super::*; /// A WinRT string ([HSTRING](https://docs.microsoft.com/en-us/windows/win32/winrt/hstring)) /// is reference-counted and immutable. #[repr(transparent)] -pub struct HSTRING(*mut Header); +pub struct HSTRING(Option>); impl HSTRING { /// Create an empty `HSTRING`. /// /// This function does not allocate memory. pub const fn new() -> Self { - Self(std::ptr::null_mut()) + Self(None) } /// Returns `true` if the string is empty. - pub fn is_empty(&self) -> bool { + pub const fn is_empty(&self) -> bool { // An empty HSTRING is represented by a null pointer. - self.0.is_null() + self.0.is_none() } /// Returns the length of the string. - pub fn len(&self) -> usize { - if self.is_empty() { - return 0; + pub const fn len(&self) -> usize { + if let Some(header) = self.get_header() { + header.len as usize + } else { + 0 } - - unsafe { (*self.0).len as usize } } /// Get the string as 16-bit wide characters (wchars). - pub fn as_wide(&self) -> &[u16] { + pub const fn as_wide(&self) -> &[u16] { unsafe { std::slice::from_raw_parts(self.as_ptr(), self.len()) } } /// Returns a raw pointer to the `HSTRING` buffer. - pub fn as_ptr(&self) -> *const u16 { - if self.is_empty() { + pub const fn as_ptr(&self) -> *const u16 { + if let Some(header) = self.get_header() { + header.data + } else { const EMPTY: [u16; 1] = [0]; EMPTY.as_ptr() - } else { - let header = self.0; - unsafe { (*header).data } } } @@ -80,7 +79,16 @@ impl HSTRING { // Write a 0 byte to the end of the buffer. std::ptr::write((*ptr).data.offset((*ptr).len as isize), 0); - Self(ptr) + Self(std::mem::transmute(ptr)) + } + + const fn get_header(&self) -> Option<&Header> { + if let Some(header) = &self.0 { + // TODO: this can be replaced with `as_ref` in future: https://github.com/rust-lang/rust/issues/91822 + unsafe { Some(&*(header.as_ptr() as *const Header)) } + } else { + None + } } } @@ -104,11 +112,11 @@ impl Default for HSTRING { impl Clone for HSTRING { fn clone(&self) -> Self { - if self.is_empty() { + if let Some(header) = self.get_header() { + unsafe { Self(std::mem::transmute(header.duplicate())) } + } else { return Self::new(); } - - unsafe { Self((*self.0).duplicate()) } } } @@ -119,11 +127,13 @@ impl Drop for HSTRING { } unsafe { - let header = std::mem::replace(&mut self.0, std::ptr::null_mut()); - // REFERENCE_FLAG indicates a string backed by static or stack memory that is - // thus not reference-counted and does not need to be freed. - if (*header).flags & REFERENCE_FLAG == 0 && (*header).count.release() == 0 { - heap_free(header as *mut std::ffi::c_void); + if let Some(header) = self.0.take() { + // REFERENCE_FLAG indicates a string backed by static or stack memory that is + // thus not reference-counted and does not need to be freed. + let header = header.as_ref(); + if header.flags & REFERENCE_FLAG == 0 && header.count.release() == 0 { + heap_free(header as *const _ as *mut _); + } } } } diff --git a/crates/tests/core/tests/hstring.rs b/crates/tests/core/tests/hstring.rs index b1553a3426..9579d781cd 100644 --- a/crates/tests/core/tests/hstring.rs +++ b/crates/tests/core/tests/hstring.rs @@ -3,6 +3,7 @@ use windows::core::*; #[test] fn hstring_works() { + assert_eq!(std::mem::size_of::(), std::mem::size_of::()); let empty = HSTRING::new(); assert!(empty.is_empty()); assert!(empty.is_empty()); From e10f2bc65d2f499a232aa7f777a54ded1d017868 Mon Sep 17 00:00:00 2001 From: Kenny Kerr Date: Thu, 29 Sep 2022 07:49:02 -0500 Subject: [PATCH 2/3] msrv & clippy --- .github/workflows/build.yml | 2 +- crates/libs/implement/Cargo.toml | 2 +- crates/libs/interface/Cargo.toml | 2 +- crates/libs/windows/Cargo.toml | 2 +- crates/libs/windows/src/core/strings/hstring.rs | 2 +- crates/libs/windows/src/core/weak_ref_count.rs | 6 +++--- crates/tools/windows/src/main.rs | 2 +- crates/tools/yml/src/main.rs | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a35d129e3c..3ae5d586a2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -64,7 +64,7 @@ jobs: name: Check windows strategy: matrix: - rust: [1.59.0, stable, nightly] + rust: [1.64.0, stable, nightly] runs-on: - windows-2019 - ubuntu-latest diff --git a/crates/libs/implement/Cargo.toml b/crates/libs/implement/Cargo.toml index c1f1e93a81..e5e65ba244 100644 --- a/crates/libs/implement/Cargo.toml +++ b/crates/libs/implement/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" license = "MIT OR Apache-2.0" description = "The implement macro for the windows crate" repository = "https://github.com/microsoft/windows-rs" -rust-version = "1.61" +rust-version = "1.64" [package.metadata.docs.rs] default-target = "x86_64-pc-windows-msvc" diff --git a/crates/libs/interface/Cargo.toml b/crates/libs/interface/Cargo.toml index abbe87e6aa..0656db119f 100644 --- a/crates/libs/interface/Cargo.toml +++ b/crates/libs/interface/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Microsoft"] license = "MIT OR Apache-2.0" description = "The interface macro for the windows crate" repository = "https://github.com/microsoft/windows-rs" -rust-version = "1.61" +rust-version = "1.64" [package.metadata.docs.rs] default-target = "x86_64-pc-windows-msvc" diff --git a/crates/libs/windows/Cargo.toml b/crates/libs/windows/Cargo.toml index 80eb654018..b7b38d8c7e 100644 --- a/crates/libs/windows/Cargo.toml +++ b/crates/libs/windows/Cargo.toml @@ -9,7 +9,7 @@ description = "Rust for Windows" repository = "https://github.com/microsoft/windows-rs" documentation = "https://microsoft.github.io/windows-docs-rs/" readme = "../../../docs/readme.md" -rust-version = "1.59" +rust-version = "1.64" [package.metadata.docs.rs] default-target = "x86_64-pc-windows-msvc" diff --git a/crates/libs/windows/src/core/strings/hstring.rs b/crates/libs/windows/src/core/strings/hstring.rs index b118331209..b30abfeb08 100644 --- a/crates/libs/windows/src/core/strings/hstring.rs +++ b/crates/libs/windows/src/core/strings/hstring.rs @@ -115,7 +115,7 @@ impl Clone for HSTRING { if let Some(header) = self.get_header() { unsafe { Self(std::mem::transmute(header.duplicate())) } } else { - return Self::new(); + Self::new() } } } diff --git a/crates/libs/windows/src/core/weak_ref_count.rs b/crates/libs/windows/src/core/weak_ref_count.rs index 42e2f4bc91..cf557bad02 100644 --- a/crates/libs/windows/src/core/weak_ref_count.rs +++ b/crates/libs/windows/src/core/weak_ref_count.rs @@ -13,11 +13,11 @@ impl WeakRefCount { } pub fn add_ref(&self) -> u32 { - self.0.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |count_or_pointer| (!is_weak_ref(count_or_pointer)).then(|| count_or_pointer + 1)).map(|u| u as u32 + 1).unwrap_or_else(|pointer| unsafe { TearOff::decode(pointer).strong_count.add_ref() }) + self.0.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |count_or_pointer| (!is_weak_ref(count_or_pointer)).then_some(count_or_pointer + 1)).map(|u| u as u32 + 1).unwrap_or_else(|pointer| unsafe { TearOff::decode(pointer).strong_count.add_ref() }) } pub fn release(&self) -> u32 { - self.0.fetch_update(Ordering::Release, Ordering::Relaxed, |count_or_pointer| (!is_weak_ref(count_or_pointer)).then(|| count_or_pointer - 1)).map(|u| u as u32 - 1).unwrap_or_else(|pointer| unsafe { + self.0.fetch_update(Ordering::Release, Ordering::Relaxed, |count_or_pointer| (!is_weak_ref(count_or_pointer)).then_some(count_or_pointer - 1)).map(|u| u as u32 - 1).unwrap_or_else(|pointer| unsafe { let tear_off = TearOff::decode(pointer); let remaining = tear_off.strong_count.release(); @@ -214,7 +214,7 @@ impl TearOff { .fetch_update(Ordering::Acquire, Ordering::Relaxed, |count| { // Attempt to acquire a strong reference count to stabilize the object for the duration // of the `QueryInterface` call. - (count != 0).then(|| count + 1) + (count != 0).then_some(count + 1) }) .map(|_| { // Let the object respond to the upgrade query. diff --git a/crates/tools/windows/src/main.rs b/crates/tools/windows/src/main.rs index 087ef1ca7f..45facc4c6f 100644 --- a/crates/tools/windows/src/main.rs +++ b/crates/tools/windows/src/main.rs @@ -47,7 +47,7 @@ description = "Rust for Windows" repository = "https://github.com/microsoft/windows-rs" documentation = "https://microsoft.github.io/windows-docs-rs/" readme = "../../../docs/readme.md" -rust-version = "1.59" +rust-version = "1.64" [package.metadata.docs.rs] default-target = "x86_64-pc-windows-msvc" diff --git a/crates/tools/yml/src/main.rs b/crates/tools/yml/src/main.rs index 853695dc86..35bc0efb68 100644 --- a/crates/tools/yml/src/main.rs +++ b/crates/tools/yml/src/main.rs @@ -215,7 +215,7 @@ jobs: name: Check windows strategy: matrix: - rust: [1.59.0, stable, nightly] + rust: [1.64.0, stable, nightly] runs-on: - windows-2019 - ubuntu-latest From 0733904635f27f4cf02ec379a6e0f3ce7015b67e Mon Sep 17 00:00:00 2001 From: Kenny Kerr Date: Thu, 29 Sep 2022 10:34:11 -0500 Subject: [PATCH 3/3] feedback --- crates/libs/windows/src/core/strings/hstring.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/libs/windows/src/core/strings/hstring.rs b/crates/libs/windows/src/core/strings/hstring.rs index b30abfeb08..f222ebcd85 100644 --- a/crates/libs/windows/src/core/strings/hstring.rs +++ b/crates/libs/windows/src/core/strings/hstring.rs @@ -79,7 +79,7 @@ impl HSTRING { // Write a 0 byte to the end of the buffer. std::ptr::write((*ptr).data.offset((*ptr).len as isize), 0); - Self(std::mem::transmute(ptr)) + Self(std::ptr::NonNull::new(ptr)) } const fn get_header(&self) -> Option<&Header> { @@ -113,7 +113,7 @@ impl Default for HSTRING { impl Clone for HSTRING { fn clone(&self) -> Self { if let Some(header) = self.get_header() { - unsafe { Self(std::mem::transmute(header.duplicate())) } + Self(std::ptr::NonNull::new(header.duplicate())) } else { Self::new() } @@ -126,10 +126,10 @@ impl Drop for HSTRING { return; } - unsafe { - if let Some(header) = self.0.take() { - // REFERENCE_FLAG indicates a string backed by static or stack memory that is - // thus not reference-counted and does not need to be freed. + if let Some(header) = self.0.take() { + // REFERENCE_FLAG indicates a string backed by static or stack memory that is + // thus not reference-counted and does not need to be freed. + unsafe { let header = header.as_ref(); if header.flags & REFERENCE_FLAG == 0 && header.count.release() == 0 { heap_free(header as *const _ as *mut _);