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

A win32-api proposal, implemented for user32.zig #7195

Merged
merged 4 commits into from
Jan 12, 2021
Merged

A win32-api proposal, implemented for user32.zig #7195

merged 4 commits into from
Jan 12, 2021

Conversation

Aransentin
Copy link
Contributor

  • Each plain win32 function is available in their respective namespace just like before.
  • Each function also has a Zig wrapper. The wrapper has a few specific purposes:
    • It receives regular Zig types as arguments instead of the occasionally confusing Windows ones; e.g. u32 instead of ULONG (yes, these are always the same on Windows!)
    • It checks the return value and turns it to Zig errors. What errors can be returned can sometimes be found in the Microsoft documentation, or more exhaustively in the Wine platform test suite (e.g.).
    • It asserts illegal win32 behaviour when stated in the Microsoft documentation.
    • If the OS target range you are compiling for does not include the function, it raises a compileError.
      If the entire OS target range you are compiling for includes the function, it calls the plain function.
      If the OS target range you are compiling for partially includes the function, it calls a function pointer. It is the responsibility of the caller to load this beforehand.
    • It takes care of odd win32 compatibility quirks. For example SetWindowLongPtrA is typedefed to SetWindowLongA for 32-bit architectures, so we call that functions instead if so. It also requires you to do a SetLastError(ERROR_SUCCESS) before you call it, as it doesn't do that by itself. The wrapper does, as otherwise we'd have no idea if an error is real or a remnant from the previous function call.
  • Types and constants that are used across libraries go in bits.zig, everything else is defined above the family of functions that uses it.
  • Candidates for addition into the win32 API are functions that are not explicitly deprecated by Microsoft, nor entirely superceded by newer functions such as OpenFile / CreateFile.

Copy link
Contributor

@daurnimator daurnimator 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 whole pattern with selectSymbol forces all function calls to be indirect? This would seem to be undesireable....

lib/std/os/windows/user32.zig Show resolved Hide resolved
lib/std/os/windows/user32.zig Outdated Show resolved Hide resolved
lib/std/os/windows/user32.zig Outdated Show resolved Hide resolved
@daurnimator daurnimator added os-windows standard library This issue involves writing Zig code for the standard library. labels Nov 22, 2020
@@ -52,13 +86,21 @@ pub const WM_SYSKEYDOWN = 0x0104;
pub const WM_SYSKEYUP = 0x0105;
pub const WM_SYSCHAR = 0x0106;
pub const WM_SYSDEADCHAR = 0x0107;
pub const WM_UNICHAR = 0x0109;
pub const WM_KEYLAST = 0x0109;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you want to include most of the *LAST definitions: they merely state that this is the next free enum member, and it will likely change in future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The keys are a subset of the WM_ constants, so knowing what the last key or mouse event is specifically might still be useful.

@Aransentin
Copy link
Contributor Author

I think the whole pattern with selectSymbol forces all function calls to be indirect? This would seem to be undesireable....

Seems to me like the optimizer takes care of it:
https://godbolt.org/z/n1fb49

pub const MB_MODEMASK = 0x00003000;
pub const MB_MISCMASK = 0x0000C000;

pub const IDOK = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it should be an enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm copying the values from the Windows headers, where they are not.

I could certainly convert a lot of the values to enums and such, though it might annoy people following the Windows documentation and assuming it'll look identical.

@kubkon
Copy link
Member

kubkon commented Nov 22, 2020

Unless I'm missing something, I thought the idea was to totally get rid of higher-level Win API such as kernel32 or user32 in favour of implementing Zig primitives based directly on ntdll. Has the direction changed in the meantime? FWIW here's the accepted proposal: #1840. With that in mind, I think I lean against this proposal, and would rather see more effort spent on implementing full user32 wrappers in Zig with hooks directly into ntdll. @andrewrk, what are your thoughts about this?

@Aransentin
Copy link
Contributor Author

Aransentin commented Nov 22, 2020

Unless I'm missing something, I thought the idea was to totally get rid of higher-level Win API such as kernel32 or user32 in favour of implementing Zig primitives based directly on ntdll.

For many tasks using ntdll is not practically possible. Creating a window without it is heavily dependent on the OS version, and the kernel entry point (NtUserCreateWindowEx, in this case) will do many callbacks into user32.dll itself before it returns to the caller.

@kubkon
Copy link
Member

kubkon commented Nov 22, 2020

Unless I'm missing something, I thought the idea was to totally get rid of higher-level Win API such as kernel32 or user32 in favour of implementing Zig primitives based directly on ntdll.

For many tasks using ntdll is not practically possible. Creating a window without it is heavily dependent on the OS version, and the kernel entry point (NtUserCreateWindowEx, in this case) will do many callbacks into user32.dll itself before it returns to the caller.

Right, but I thought that was the point of not having any dependence on higher-level APIs whatsoever - to manage it ourselves within Zig's libstd using only the ntdll functions. I'm not saying that having direct wrappers for user32 or kernel32 functions is a bad idea, but perhaps it would be a better fit if it was done as a standalone package separate from Zig's libstd? Otherwise, this PR looks like a step backwards with respect to the plans that have already been agreed on.

@Aransentin
Copy link
Contributor Author

Right, but I thought that was the point of not having any dependence on higher-level APIs whatsoever - to manage it ourselves within Zig's libstd using only the ntdll functions. I'm not saying that having direct wrappers for user32 or kernel32 functions is a bad idea, but perhaps it would be a better fit if it was done as a standalone package separate from Zig's libstd? Otherwise, this PR looks like a step backwards with respect to the plans that have already been agreed on.

I'm not advocating for uprooting #1840 and using kernel32 instead of ntdll in the standard library. 🙂
The question about what to do with the win32 bindings is issue #4426 , and it hasn't been resolved - the second option there "a complete, up-to-date, maintained set of all Windows API functions, for some approved set of DLLs" shouldn't mean that the standard library will depend on the functions unless totally necessary.

@marler8997
Copy link
Contributor

I don't think we've established a criteria for what goes in the standard library, but my guess is we'll use something like this to determine whether it goes into the standard library:

  1. is it needed by the zig build system?
  2. is it needed by the zig package manager?
  3. is it used by the zig compiler (to a reasonable degree)?

There could be other tools we want to include in this list. For the Windows bindings., I think the direction we want to go is keep the full set of Windows bindings in project outside the standard library. That's why I recently started this project: https://github.com/marler8997/zig-os-windows

The goal of my zig-os-windows project is to be able to autogenerate the bindings from a data set representing the API. In the README.md I talk more about direction/goals. One question is how to handle Zig wrappers for the Windows API like you've created here. I'm not sure how hard it will be to autogenerate those as well, but because of the size of the Windows API I think it's worth exploring this option.

By autogenerating the bindings as well, we can generate bindings with different options like normalizing data types to Zig types, organizing by C header or by DLL, etc. One option is to not only generate a full set of bindings, but also a subset needed in the standard library.

@andrewrk
Copy link
Member

Noting that #1840 is still the accepted way forward for the standard library (std lib should only depend on ntdll.dll) and #4426 looming in the future (user32.zig may be deleted from the std lib after all), I think this proposal is appropriate to be accepted as the path forward, for whatever Windows bindings we decide to maintain in the standard library. So I'm happy to merge this and accept this proposal now, with the understanding that (1) the standard library should avoid depending on user32.dll entirely and (2) we may eventually delete all this code anyway (or move it to the standard library orphanage).

@andrewrk andrewrk added accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Jan 12, 2021
@andrewrk andrewrk merged commit 4f5fa90 into ziglang:master Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. os-windows proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants