Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows: bump windows-sys to 0.52 #3639

Merged
merged 1 commit into from
Apr 22, 2024
Merged

windows: bump windows-sys to 0.52 #3639

merged 1 commit into from
Apr 22, 2024

Conversation

kchibisov
Copy link
Member

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@kchibisov kchibisov requested a review from MarijnS95 April 18, 2024 15:13
@kchibisov kchibisov requested a review from msiglreith as a code owner April 18, 2024 15:13
@@ -187,7 +187,7 @@ impl FileDropHandler {
let get_data_fn = unsafe { (*(*data_obj).cast::<IDataObjectVtbl>()).GetData };
let get_data_result = unsafe { get_data_fn(data_obj as *mut _, &drop_format, &mut medium) };
if get_data_result >= 0 {
let hdrop = unsafe { medium.Anonymous.hGlobal };
let hdrop = unsafe { medium.u.hGlobal as HDROP };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could someone verify that it's a correct change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the docs, I think windows-sys changed internal unions from being called Anonymous to being called u.

Additionally, HGLOBAL changed from isize to *mut c_void, I assume that's intentional too, at least that's what I'm getting from reading microsoft/windows-rs#1643 (comment).

Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annoying that we have to do the as u32 in a few places, but I think that's just how the Win32 API is, e.g. DwmSetWindowAttribute takes a DWORD so there isn't much that windows-rs can do here.

@@ -187,7 +187,7 @@ impl FileDropHandler {
let get_data_fn = unsafe { (*(*data_obj).cast::<IDataObjectVtbl>()).GetData };
let get_data_result = unsafe { get_data_fn(data_obj as *mut _, &drop_format, &mut medium) };
if get_data_result >= 0 {
let hdrop = unsafe { medium.Anonymous.hGlobal };
let hdrop = unsafe { medium.u.hGlobal as HDROP };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the docs, I think windows-sys changed internal unions from being called Anonymous to being called u.

Additionally, HGLOBAL changed from isize to *mut c_void, I assume that's intentional too, at least that's what I'm getting from reading microsoft/windows-rs#1643 (comment).

@kchibisov
Copy link
Member Author

Additionally, HGLOBAL changed from isize to *mut c_void, I assume that's intentional too, at least that's what I'm getting from reading microsoft/windows-rs#1643 (comment).

Yeah, it's just I'm not sure if all of that is sound now. Like I understood that it's like that.

@daxpedda daxpedda added this to the Version 0.30.0 milestone Apr 18, 2024
@kchibisov kchibisov force-pushed the kchibisov/windows-052 branch from fceb930 to af11b7c Compare April 22, 2024 13:10
@@ -105,7 +105,7 @@ pub unsafe fn hwnd_dpi(hwnd: HWND) -> u32 {
if unsafe { IsProcessDPIAware() } != false.into() {
// If the process is DPI aware, then scaling must be handled by the application using
// this DPI value.
unsafe { GetDeviceCaps(hdc, LOGPIXELSX) as u32 }
unsafe { GetDeviceCaps(hdc, LOGPIXELSX as i32) as u32 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ask for upstream clarification about these. At least in the windows crate where these are newtyped, the underlying ABI is "kept in sync" (IIRC by casting in the wrapper implementation).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that casting small u32 to positive i32 have the same binary repr, it's safe to do it here.

In general, a lot of casts seems to be that way because sometimes they accept enum but enum itself is i32 and not u32, and so on.

You can still ask though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not saying that it's unsafe, just that it's inconvenient after these Windows crates already have all the type information they need; looks like it's all reserved for the high-level windows crate though.

@kchibisov kchibisov merged commit 2491f2b into master Apr 22, 2024
52 checks passed
@kchibisov kchibisov deleted the kchibisov/windows-052 branch April 22, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants