Emit handles as void* not IntPtr

[riverar]

Rust currently emits isize/usize integers as a result of decomposing our synthetic handle types and encountering IntPtr/UIntPtr. Rust would now like to emit pointers to better align with its core handle type, but it cannot differentiate between legitimate pointer-sized integers (e.g. LRESULT) and handles (e.g. HMODULE).

The proposed change is to emit all handles with their original pointer type.

// Before
public struct HMODULE
{
   public IntPtr Value;
}

// After
public struct HMODULE
{
   public unsafe void* Value;
}

See also: https://github.com/microsoft/windows-rs/issues/3093

[kennykerr]

This is substantiated by the fact that wtypes.h, and many others, actually defines these as pointers so this is just asking that the metadata is faithful to the original definitions, for example:

typedef void * HANDLE;
typedef void *HMODULE;

typedef void *HINSTANCE;

typedef void *HTASK;

typedef void *HKEY;

typedef void *HDESK;

typedef void *HMF;

typedef void *HEMF;

typedef void *HPEN;

typedef void *HRSRC;

typedef void *HSTR;

typedef void *HWINSTA;

typedef void *HKL;

typedef void *HGDIOBJ;

[mikebattista]

These and others are declared with the DECLARE_HANDLE macro. Should all of these be void*?

[kennykerr]

Yes, the underlying handle type for DECLARE_HANDLE are all pointers and void* is the generic underlying type for such handles.

[AArnott]

This feels wrong to me because handles are not pointers. They are opaque values, and though most (not all) of them are pointer-sized values, there is absolutely no guarantee generally that they point to a memory address. They could just as easily point to a row in a table somewhere. Declaring them as pointers is forcing a particular interpretation on handles that is an assumption that is likely inaccurate at least sometimes.

This change also broke CsWin32.

If this somehow helps rust, can we annotate typedef structs with an attribute instead of changing the field type to a pointer?

[kennykerr]

My understanding is that the Win32 metadata has always strived to faithfully capture the original API definitions, so this change seems consistent with that philosophy.

While handles may or may not be pointers, some definitely are, the Win32 metadata should faithfully capture the original API definitions, as illustrated here https://github.com/microsoft/win32metadata/issues/1924#issuecomment-2165757800, and languages can then decide to either preserve those original definitions or diverge in some way.

There is obviously no question that the Windows SDK defines these handles as pointers.

[Nuklon]

This change also broke CsWin32.

What broke? The code seems to be generated OK here.

[AArnott]

@kennykerr Ok, if the headers themselves defined them as pointers, I guess fine. I'll fix CsWin32

@Nuklon You're the one who filed the bug in the first place: https://github.com/microsoft/CsWin32/issues/1219

[mikebattista]

Yes the feedback was what we had was inconsistent with the SDK headers and it was causing pain, so we made the change.

[riverar]

This feels wrong to me because handles are not pointers. They are opaque values, and though most (not all) of them are pointer-sized values, there is absolutely no guarantee generally that they point to a memory address. They could just as easily point to a row in a table somewhere. Declaring them as pointers is forcing a particular interpretation on handles that is an assumption that is likely inaccurate at least sometimes.

Agreed. That's my stance too, but the SDK ultimately uses a pointer type (due to lack of pointer-sized integers at the time) and there's no appetite to fix this at any level of the supply chain.

You could potentially key off InvalidHandleValue to identify handles and ignore the void* there and continue to emit native integers if you wish.

[Nuklon]

@Nuklon You're the one who filed the bug in the first place: microsoft/CsWin32#1219

I'm not sure if that's related to this change? It works with the latest Win32 metadata, but when you add the new WDK metadata it breaks. WDK metadata didn't have any DEVPKEY before (DEVPKEY is located in Win32 metadata).