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

delete unused *opaque{} definitions from std.os.windows #20208

Open
expikr opened this issue Jun 6, 2024 · 2 comments
Open

delete unused *opaque{} definitions from std.os.windows #20208

expikr opened this issue Jun 6, 2024 · 2 comments
Labels
os-windows standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@expikr
Copy link
Contributor

expikr commented Jun 6, 2024

AFAIK the following are vestiges;

pub const HBRUSH = *opaque {}; // used for gui window creation in user32
pub const HCURSOR = *opaque {}; // used for gui window creation in user32
pub const HICON = *opaque {}; // used for gui window creation in user32
pub const HINSTANCE = *opaque {}; // used just once for entrypoint in start.zig then untouched
pub const HMENU = *opaque {}; // used for gui window creation in user32
pub const HDC = *opaque {}; // used for gui window drawing ops in gdi32
pub const HGLRC = *opaque {}; // used for gui window drawing ops in gdi32

c.c. @squeek502

@squeek502
Copy link
Collaborator

squeek502 commented Jun 6, 2024

I actually think this is one area where it might make sense to keep unused std.os.windows definitions due to how opaque {} works.

For example, say I'm writing some code that uses HICON and passes it to 2 different libraries foo and bar. With the definition in the standard library, that code could look like:

const foo = @import("foo");
const bar = @import("bar");

pub fn something(icon: std.os.windows.HICON) void {
    foo.fooIcon(icon);
    bar.barIcon(icon);
}

and

pub fn something(cursor: std.os.windows.HCURSOR) void {
    foo.fooIcon(cursor);
    bar.barIcon(cursor);
}

would give a compile error.

However, if the definition is not in the standard library, then usage might look something like this instead (assuming foo and bar each had their own HICON = *opaque {}):

const foo = @import("foo");
const bar = @import("bar");
// contains its own `HICON = *opaque {};` definition
const windows_extra = @import("windows_extra.zig");

pub fn something(icon: windows_extra.HICON) void {
    foo.fooIcon(@ptrCast(icon));
    bar.barIcon(@ptrCast(icon));
}

which loses the type safety of opaque {}.

Since the maintenance burden of these HANDLE definitions in the standard library is effectively zero (they'll never need to change), I think it might be worth keeping them around (and potentially expanding them to all common HANDLE types if there are any missing) for nicer interoperability.

Just my 2 cents though. #4426 might take precedence.

@squeek502 squeek502 added standard library This issue involves writing Zig code for the standard library. os-windows labels Jun 6, 2024
@expikr
Copy link
Contributor Author

expikr commented Jun 6, 2024

In that case, perhaps opaque types should be maintained in its own namespace std.os.windows.types instead?

The following resource handles are listed in https://learn.microsoft.com/en-us/windows/win32/winprog/windows-data-types:

HACCEL
HBITMAP
HBRUSH
HCOLORSPACE
HCONV
HCONVLIST
HCURSOR
HDC
HDDEDATA
HDESK
HDROP
HDWP
HENHMETAFILE
// HFILE is legacy 32bit handle
HFONT
HGDIOBJ
HGLOBAL
HHOOK
HICON
HINSTANCE
HKEY
HKL
HLOCAL
HMENU
HMETAFILE
HMODULE
HMONITOR
HPALETTE
HPEN
HRGN
HRSRC
HSZ
HWINSTA
HWND

SC_HANDLE
SC_LOCK
SERVICE_STATUS_HANDLE

@Vexu Vexu added this to the 0.14.0 milestone Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os-windows standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

3 participants