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

Add standalone code generation support for the Rust Standard Library #2439

Merged
merged 5 commits into from
Apr 12, 2023

Conversation

kennykerr
Copy link
Collaborator

A targeted solution to manage some minor discrepancies between the windows-sys definitions and those required by the Rust Standard Library. Ideally we can reduce and eventually eliminate these differences over time, particularly once raw-dylib is ubiquitous.

rust-lang/rust#110152

@ChrisDenton
Copy link
Collaborator

Looks great! Just a couple of issues:

  • Functions need to be pub fn.
  • The standard library is currently checked with #![deny(fuzzy_provenance_casts)] which means ::core::ptr::invalid_mut needs to be used for invalid pointer values.

Also just a quality of life thing but the reason for grouping functions in the same extern block was because of how linking libraries works. Currently rustc will read all the link names and append them, in order, to the linker arguments. E.g.:

#[link(name = "kernel32")]
extern "system" {
    pub fn SetLastError(dwerrcode: WIN32_ERROR) -> ();
}
#[link(name = "ws2_32")]
extern "system" {
    pub fn WSACleanup() -> i32;
}
#[link(name = "kernel32")]
extern "system" {
    pub fn DeleteFileW(lpfilename: PCWSTR) -> BOOL;
}

In this example kernel32 gets added to the command line twice. In more complex situations this can lead to even more duplication. This is not the end of the world but it can make diagnostics very noisy. This used to be a lot worse but rustc does now do some deduplication: if the same library appears consecutively then it will be collapsed into one argument.

@kennykerr
Copy link
Collaborator Author

All the more reason to switch those handles to isize. 😜

Anyway, I've address the main feedback - do check that I got it right.

Collapsing the extern blocks is a little more involved but will get to that next. The way the Rust compiler creates these enormous link command lines is quite frustrating.

@ChrisDenton
Copy link
Collaborator

All the more reason to switch those handles to isize. 😜

Yeah tbh with my libstd hat off I don't really disagree. But with my libstd hat on there's enough controversy around it that I can't really push such a change through the standard library until there's some kind of consensus (or near enough to it).

Anyway, I've address the main feedback - do check that I got it right.

That's working great, thanks!

@kennykerr
Copy link
Collaborator Author

OK, so previously the bindings were sorted by metadata name (including namespace) for stability but now its further sorted by the type names that actually appear in the generated bindings (irrespective of namespace). This makes it a little more readable. The imports are further sorted by library name before function name, so that they should all be grouped together and appear first. They aren't quite collapsed into shared extern blocks. The challenge there is that that would have to further group by calling convention but hopefully this is enough to trigger the compiler's deduplication.

@ChrisDenton
Copy link
Collaborator

This is fantastic, thank you! I've now updated the std PR with the generated bindings. The imports are fine as they are.

@kennykerr kennykerr merged commit 244a88d into master Apr 12, 2023
@kennykerr kennykerr deleted the standalone_std branch April 12, 2023 00:18
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.

2 participants