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
1.99k stars 84 forks source link

Functions like GetDeviceCaps, SetBkMode had a unnecessary additional LocalExternFunction #1075

Closed harborsiem closed 8 months ago

harborsiem commented 8 months ago

Actual behavior

Actual generated code:

public static int GetDeviceCaps(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.GET_DEVICE_CAPS_INDEX index)
{
    int __retVal = LocalExternFunction(hdc, (winmdroot.Graphics.Gdi.GET_DEVICE_CAPS_INDEX)index);
    return __retVal;

    [DllImport("GDI32.dll", ExactSpelling = true, EntryPoint = "GetDeviceCaps")]
    static extern int LocalExternFunction(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.GET_DEVICE_CAPS_INDEX index);
}

public static int SetBkMode(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.BACKGROUND_MODE mode)
{
    int __retVal = LocalExternFunction(hdc, (winmdroot.Graphics.Gdi.BACKGROUND_MODE)mode);
    return __retVal;

    [DllImport("GDI32.dll", ExactSpelling = true, EntryPoint = "SetBkMode")]
    static extern int LocalExternFunction(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.BACKGROUND_MODE mode);
}

Expected behavior

Expected generated code:

[DllImport("GDI32.dll", ExactSpelling = true, EntryPoint = "GetDeviceCaps")]
public static extern int GetDeviceCaps(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.GET_DEVICE_CAPS_INDEX index);

[DllImport("GDI32.dll", ExactSpelling = true, EntryPoint = "SetBkMode")]
public static extern int SetBkMode(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.BACKGROUND_MODE mode);

or

[DllImport("GDI32.dll", ExactSpelling = true)]
public static extern int GetDeviceCaps(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.GET_DEVICE_CAPS_INDEX index);

[DllImport("GDI32.dll", ExactSpelling = true)]
public static extern int SetBkMode(winmdroot.Graphics.Gdi.HDC hdc, [MarshalAs(UnmanagedType.I4)] winmdroot.Graphics.Gdi.BACKGROUND_MODE mode);

Repro steps

  1. NativeMethods.txt content:
    
    GetDeviceCaps
    SetBkMode

2. `NativeMethods.json` content (if present):
```json
  1. Any of your own code that should be shared?

Context

AArnott commented 8 months ago

Good catch. In this specific case I think you're right. However with other options set (e.g. allowMarshaling: false), the local function is required. While we could add code to distinguish between these cases, it would make the code and testing more complex. We would need some real value (more than just a couple less lines of generated code) to justify that cost.