microsoft / windows-rs

Rust for Windows
https://kennykerr.ca/rust-getting-started/
Apache License 2.0
10.23k stars 476 forks source link

Make MONITORINFO::default() set the cbSize field automatically. #3009

Closed ted-dokos closed 3 months ago

ted-dokos commented 3 months ago

Suggestion

Right now the default will zero-initialize MONITORINFO. From where I sit, it would be nice to eliminate the extra step of setting cbSize by hand:

let mut info = MONITORINFO::default();
info.cbSize = size_of::<MONITORINFO>() as u32;

Are there good reasons to prefer the zero-initialized version for MONITORINFO::default() ?

tim-weis commented 3 months ago

The metadata driving the code generation does supply the StructSizeField attribute, but the generator doesn't (currently) evaluate it. I don't know whether there is deliberation to this, however, you can turn the two-phase initialization into a single initialization statement:

let info = MONITORINFO { cbSize: size_of::<MONITORINFO>() as u32, ..Default::default() };
kennykerr commented 3 months ago

I was hesitant to roll this out in windows-rs as the attribute was not widely supported. I see now that it has been applied to a few hundred structs. I doubt this is exhaustive but probably worth picking up now. Will take a look soon.

kennykerr commented 3 months ago

It's also somewhat limited. The first example I looked at fell short of being usable. While MONITORINFO has the attribute, the MONITORINFOEXA struct doesn't have the attribute so I guess we'd have to root around the various fields looking for a type with the attribute and then make sure the outer initialization isn't overridden by any inner initialization. I'm just not sure it's worth the complexity.

kennykerr commented 3 months ago

Another example is BLOB that incorrectly uses the attribute.