retep998 / winapi-rs

Rust bindings to Windows API
https://crates.io/crates/winapi
Apache License 2.0
1.85k stars 392 forks source link

Heads-up: winapi 0.2.8 will fail to build soon-ish #1030

Closed RalfJung closed 9 months ago

RalfJung commented 2 years ago

In https://github.com/rust-lang/rust/pull/102513 we are moving ahead with finally closing the ancient soundness issue https://github.com/rust-lang/rust/issues/82523 (originally tracked at https://github.com/rust-lang/rust/issues/27060): we are disallowing creating references to fields of packed structs. It seems like that will make winapi 0.2.8 fail to build:

error[E0791]: `Debug` can't be derived on this `#[repr(packed)]` struct that does not derive `Copy`
    |
    |
263 |           #[repr(C)] #[derive(Debug)] $(#[$attrs])*
    |
   ::: C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.8\src\mmreg.rs:290:1
    |
    |
290 | / STRUCT!{#[repr(packed)] struct WAVEFORMATEX {
291 | |     wFormatTag: ::WORD,
292 | |     nChannels: ::WORD,
293 | |     nSamplesPerSec: ::DWORD,
297 | |     cbSize: ::WORD,
298 | | }}
    | |__- in this macro invocation
    |
    |
    = note: this error originates in the derive macro `Debug` which comes from the expansion of the macro `STRUCT` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0791]: `Debug` can't be derived on this `#[repr(packed)]` struct that does not derive `Copy`
    |
    |
263 |           #[repr(C)] #[derive(Debug)] $(#[$attrs])*
    |
   ::: C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.8\src\mmreg.rs:299:1
    |
    |
299 | / STRUCT!{#[repr(packed)] struct WAVEFORMATEXTENSIBLE {
300 | |     Format: ::WAVEFORMATEX,
301 | |     Samples: ::WORD,
302 | |     dwChannelMask: ::DWORD,
303 | |     SubFormat: ::GUID,
304 | | }}
    |
    = note: this error originates in the derive macro `Debug` which comes from the expansion of the macro `STRUCT` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0791]: `Debug` can't be derived on this `#[repr(packed)]` struct that does not derive `Copy`
    |
    |
263 |           #[repr(C)] #[derive(Debug)] $(#[$attrs])*
    |
    |
   ::: C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.8\src\usbspec.rs:26:1
    |
26  | / STRUCT!{#[repr(packed)] struct USB_CONFIGURATION_DESCRIPTOR {
27  | |     bLength: ::UCHAR,
28  | |     bDescriptorType: ::UCHAR,
29  | |     wTotalLength: ::USHORT,
...   |
34  | |     MaxPower: ::UCHAR,
35  | | }}
    |
    = note: this error originates in the derive macro `Debug` which comes from the expansion of the macro `STRUCT` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0791]: `Debug` can't be derived on this `#[repr(packed)]` struct that does not derive `Copy`
    |
    |
263 |           #[repr(C)] #[derive(Debug)] $(#[$attrs])*
    |
   ::: C:\Users\runneradmin\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.8\src\winusb.rs:8:1
    |
    |
8   | / STRUCT!{#[repr(packed)] struct WINUSB_SETUP_PACKET {
9   | |     RequestType: ::UCHAR,
10  | |     Request: ::UCHAR,
11  | |     Value: ::USHORT,
12  | |     Index: ::USHORT,
13  | |     Length: ::USHORT,
14  | | }}
    |
    = note: this error originates in the derive macro `Debug` which comes from the expansion of the macro `STRUCT` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0791`.

It looks like current versions of winapi avoid that error by deriving Copy for those structs.

I am not sure if there is anything you want to do about this -- winapi 0.2.8 seems to not be supported any more. However, given how widely used this crate is, it still might make sense to do a patch release so that people can fix their build with cargo update. If you want to do that, I'm open to waiting a bit before landing the rustc PR so that you can get the patch release out first.

RalfJung commented 2 years ago

I wrote a patch that fixes the build and pushed it to https://github.com/RalfJung/winapi-rs/tree/0.2. I am not entirely sure, however, if there was some reason why Copy was implemented instead of derived?

tbu- commented 1 year ago

It might have been implemented instead of derived due to compile-duration concerns.

Is there a reason why the derived Debug implementation cannot take the implemented Copy into account? This would solve the issue in a backward compatible way.

RalfJung commented 1 year ago

The reason is "it's a breaking change" (given the constraints on how derive macros work), but we are probably going to do that anyway. See https://github.com/rust-lang/rust/pull/104429.

RalfJung commented 9 months ago

We ended up changing the implementation of derive on packed types to keep winapi 0.2 working, see https://github.com/rust-lang/rust/pull/104429.