Update windows-bindgen to 0.100.0

[ChrisDenton]

View all comments

This release represents a major change in the way bindings are generated. See microsoft/windows-rs#4867 for details.

In short, it now more directly follows the C headers. For the standard library's purposes this is mostly reflected in relatively minor type changes or with some const pointers becoming mut.

There is however one big change that affects a lot of types. In the headers there are a lot of #defines like this:

#define GENERIC_READ                     (0x80000000L)
#define GENERIC_WRITE                    (0x40000000L)

These are constants for the access mode. In this case they're explicitly declared as L (aka signed long) integer types (in other cases there's no type at all). However, this conflicts with how access mode constants are actually used. E.g., see CreateFileW:

WINBASEAPI
HANDLE
WINAPI
CreateFileW(
    _In_ LPCWSTR lpFileName,
    _In_ DWORD dwDesiredAccess,
    _In_ DWORD dwShareMode,
    _In_opt_ LPSECURITY_ATTRIBUTES lpSecurityAttributes,
    _In_ DWORD dwCreationDisposition,
    _In_ DWORD dwFlagsAndAttributes,
    _In_opt_ HANDLE hTemplateFile
    );

Here dwDesiredAccess is a DWORD, which in rust equates to u32. The constant being signed but the usage being unsigned is not a problem for C/C++. They will happily convert between types at the drop of a hat. Rust however is stricter, as you know.

The old metadata used by windows-bindgen tried to try to fixup this mismatch but the new one goes strictly by what can be inferred from the headers. The hope in the future is that the headers themselves will be updated so better bindings can be directly derived from them.

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Fallback group: @Mark-Simulacrum, @jieyouxu
• @Mark-Simulacrum, @jieyouxu expanded to Mark-Simulacrum, jieyouxu

[ChrisDenton]

Note that I've split this into three commits. The middle commit is purely generated code.

[ChrisDenton]

This macro converts types from i32 to u32. This keeps all the as casts in one place instead of needing them to be spread throughout the code. I also added an assert to ensure we remove them once they're no longer needed (which is my hope).

View changes since the review

[ChrisDenton]

@bors try jobs=msvc,mingw

[rust-bors]

☀️ Try build successful (CI)
Build commit: 4d947a7 (4d947a7a2dd7b35cd580c70ee0353d0e1b074115)
Base parent: a69a632 (a69a63265cfd9e006d43137f98301b8d274ad4c9)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[kennykerr]

Just FYI - many of the other functions and structs that are manually declared in c.rs are now included in windows-bindgen, but there may be good reasons why you're declaring them manually.

[ChrisDenton]

I'll look to see if I can clean that up a bit now but there are generally two reasons we manually declare types:

• For wrappers that implement a fallback if the function isn't available.
• We sometimes want to override how specific functions are linked

There's also one function that's not in windows-bingen at all: NtCreateNamedPipeFile (because it doesn't appear in public headers despite being publicly documented).

[ChrisDenton]

This was introduced to workaround #123999 where the ancient mingw on github's windows server 2019 had an incorrect import library. Github no longer has a 2019 runner and in general this shouldn't be a problem since rust-lang/compiler-team#993. That didn't technically set a requirement for the import libraries but it would be highly unusual for people to be using a newer mingw toolchain with very very old import libraries.

View changes since the review

[Mark-Simulacrum]

r=me in general if you are looking for more rubberstamp than real review, otherwise we should re-roll.

View changes since this review

[Mark-Simulacrum]

Does this imply we're pulling in more bindings than we actually need? Why does windows-bindgen itself not insert the allow if it's always necessary? (Or is it expedience, e.g., we have some targets that need a binding and others don't?)

[ChrisDenton]

It's a mix of things. Indeed some targets use things that others don't and those targets in turn may use things that are unused by other targets. Historically the bindings have also tended to pull in more than we needed but I think that's less of an issue nowadays. One big reason though is that I've also found it annoying for contributors if there's a lot of churn in the bindings so I tend to only remove things occasionally when I'm sure they're not going to be needed again. After all it's basically used as if it's an external crate (similar to libc).

[clarfonthey]

It would make sense to summarise mostly what you said in a comment.

[ChrisDenton]

Hm, this probably should get a real review.

r? clarfonthey

[clarfonthey]

I think everything's been addressed, so, r=me minus the note about documenting the allow(unused) bit.