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

Implement Default for sys style structs #3520

Merged
merged 4 commits into from
Feb 27, 2025
Merged

Implement Default for sys style structs #3520

merged 4 commits into from
Feb 27, 2025

Conversation

kennykerr
Copy link
Collaborator

The --sys style bindings generated by windows-bindgen implement fewer standard traits to minimize code generation and build time. A few are required and Default is simple enough that it does not appear to impact build time or complexity. So this update adds a Default implementation for structs. Deciding when this can be derived is however more complicated as so many more types decay to pointers in --sys style bindings.

@kennykerr
Copy link
Collaborator Author

It expands the size of the windows-sys crate so that's not great, but I'm more concerned about making windows-bindgen more broadly useful so perhaps this is worthwhile. Long term I think it makes more sense for developers to use windows-bindgen directly.

Thoughts? @dpaoliello @ChrisDenton

Copy link
Collaborator

@dpaoliello dpaoliello left a comment

Choose a reason for hiding this comment

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

I think the increase in the crate size is worth it - it both makes the Rust bindings more useful and makes it easier to port C code.

Copy link
Collaborator

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Reducing the amount of unsafe code end users end up writing is a good thing in my book. Especially when it's trivially safe (and technically could be written in safe code but few are going to bother). Default is also just so much more convenient.

@riverar
Copy link
Collaborator

riverar commented Feb 26, 2025

I'd argue this is moving away from the convention of -sys crates being minimal thin FFI bindings without additional abstractions or traits. But not going to die on that hill for a crate that will probably go away in the future.

@kennykerr
Copy link
Collaborator Author

Ideally there'd be no need for two different modes of code generation but there are some obvious benefits right now so at the very least I'd like to avoid unnecessary differences that make it harder to use one or the other and this seems like one of those.

@kennykerr kennykerr merged commit 1d0fbe0 into master Feb 27, 2025
34 checks passed
@kennykerr kennykerr deleted the sys-default branch February 27, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants