search

microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2.07k stars 87 forks source link

Friendly overloads of functions with an HDC parameter should accept `SafeHandle` #1049

Open AArnott opened 1 year ago

AArnott commented 1 year ago

EnumDisplayMonitors could before called this way:

Windows.Win32.PInvoke.EnumDisplayMonitors(null, null, proc, 0);

Now the compiler complains:

Argument 1: cannot convert from '' to 'Windows.Win32.Graphics.Gdi.HDC'

I checked the two version and it seems 0.3.18-beta does not create the SafeHandle version anymore (see below the generated code for the two version). Is this a bug or was this intended?

0.2.46-beta

internal static unsafe winmdroot.Foundation.BOOL EnumDisplayMonitors(SafeHandle hdc, winmdroot.Foundation.RECT? lprcClip, winmdroot.Graphics.Gdi.MONITORENUMPROC lpfnEnum, winmdroot.Foundation.LPARAM dwData)

internal static extern unsafe winmdroot.Foundation.BOOL EnumDisplayMonitors(winmdroot.Graphics.Gdi.HDC hdc, [Optional] winmdroot.Foundation.RECT* lprcClip, winmdroot.Graphics.Gdi.MONITORENUMPROC lpfnEnum, winmdroot.Foundation.LPARAM dwData);

0.3.18-beta

internal static unsafe winmdroot.Foundation.BOOL EnumDisplayMonitors(winmdroot.Graphics.Gdi.HDC hdc, winmdroot.Foundation.RECT? lprcClip, winmdroot.Graphics.Gdi.MONITORENUMPROC lpfnEnum, winmdroot.Foundation.LPARAM dwData)

internal static extern unsafe winmdroot.Foundation.BOOL EnumDisplayMonitors(winmdroot.Graphics.Gdi.HDC hdc, [Optional] winmdroot.Foundation.RECT* lprcClip, winmdroot.Graphics.Gdi.MONITORENUMPROC lpfnEnum, winmdroot.Foundation.LPARAM dwData);

Originally posted by @msedi in https://github.com/microsoft/CsWin32/issues/1022#issuecomment-1722834688

AArnott commented 1 year ago

It looks like this is due to HDC no longer carrying a [RAIIFree("ReleaseDC")] attribute in the metadata. That was a good change, since it serves as something of a 'base class' for typedef handles and the derived types are released with distinct functions.

We should try to recognize that pseudo-derived types of HDC exist and notice that some of them do have RAIIFree on them, as a hint that any function that accepts HDC should have a friendly overload that accepts SafeHandle.

adammw commented 1 month ago

I also notice that the RECT is treat as optional, but the HDC isn't? Is that due to the same reason? Should that be a separate bug report?

AArnott commented 1 month ago

@adammw The HDC is optional. It is omitted at the native layer by just passing in 0. In C# the equivalent is to pass in default(HDC) as the argument. No need to mark it nullable. For the SafeHandle overload, the friendly overload should allow null, and I believe it already has that behavior for SafeHandles where [Optional] appears in the metadata.