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

SetWindowLongPtr is not generated #528

Closed
rick-de-water opened this issue Apr 9, 2022 · 6 comments
Closed

SetWindowLongPtr is not generated #528

rick-de-water opened this issue Apr 9, 2022 · 6 comments
Labels
bug Something isn't working

Comments

@rick-de-water
Copy link

Actual behavior

SetWindowLong is generated, but SetWindowLongPtr is not.

Expected behavior

SetWindowLongPtr should be generated to be able to set a pointer value on window.

Repro steps

  1. NativeMethods.txt content:
Windows.Win32.UI.WindowsAndMessaging
  1. NativeMethods.json content (if present):
    N/A

  2. Any of your own code that should be shared?

unsafe private static LRESULT WindowProcedure(HWND window, uint message, WPARAM wparam, LPARAM lparam)
{
    if (message == PInvoke.WM_CREATE)
    {
        var createStruct = (CREATESTRUCTW*)lparam.Value;
        PInvoke.SetWindowLongPtr(window, WINDOW_LONG_PTR_INDEX.GWLP_USERDATA, (IntPtr)(*createStruct).lpCreateParams);
    }

    return PInvoke.DefWindowProc(window, message, wparam, lparam);
}

Context

  • CsWin32 version: 0.1.635-beta
  • Win32Metadata version (if explicitly set by project): N/A
  • Target Framework: net6.0
  • LangVersion (if explicitly set by project): 10.0
@rick-de-water rick-de-water added the bug Something isn't working label Apr 9, 2022
@AArnott
Copy link
Member

AArnott commented Apr 12, 2022

Possibly related to #343

@AArnott
Copy link
Member

AArnott commented Jun 16, 2022

This works as you expect when you target x64. AnyCPU or x86 targeted compilations don't get SetWindowLongPtr because that function only exists for 64-bit processes.
CsWin32 only generates arch-specific APIs when your compilation commits to those architectures, so this is by design.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 2022
@rick-de-water
Copy link
Author

If it is by design then that honestly sounds like bad design. When you include windows.h you get SetWindowLongPtr, even on 32 bit processes. The documentation even recommend you to use it to be compatible with both 32 bit and 64 bit systems:

Note To write code that is compatible with both 32-bit and 64-bit versions of Windows, use SetWindowLongPtr. When compiling for 32-bit Windows, SetWindowLongPtr is defined as a call to the SetWindowLong function.

The Win32 api docs say that this function is available. This library is a wrapper around the Win32 api. The function is missing from the wrapper. This should be considered a defect, not "design".

@AArnott
Copy link
Member

AArnott commented Jun 17, 2022

This library is a wrapper around the Win32 api. The function is missing from the wrapper.

Not exactly. This is a source generator that transforms a metadata file into C#. It doesn't read header files or try to cover any more than the metadata does in general. In some cases we go slightly beyond it just to make the existing APIs more usable, but we don't add extra APIs. CsWin32 is used with other metadata inputs than Win32Metadata, so in fact adding win32 APIs to other libraries' C# projections would be wholly inappropriate.

But I want to give you what you're asking for here. If we can get the codebase to a point where it can be flexible enough to do this, and after we get the defective code generation bugs fixed, we can circle back and look at adding conveniences like this.

It's easy for C++ to support what they do because they use macros in the header files. But in C# we can't invoke macros. We can only invoke real exported functions. And sure, we can emit totally custom code to make up for that, but users can write that code themselves too, so we need to prioritize the defect bugs.

Thanks for understanding.

@jnm2
Copy link
Contributor

jnm2 commented Jun 18, 2022

I'm also hearing: contributions potentially welcome.

@AArnott
Copy link
Member

AArnott commented Jun 20, 2022

Yes, in general they are. But anything more than a trivial change should be presented in an issue with a design proposal before sending a PR to help avoid wasted effort in case it's not a PR we could accept or at least not in a particular form.
And just as fair warning for this issue, if the solution added general complexity to the overall system, we may turn it down at least for now since we don't want the added complexity to slow down our fixing of the defect issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants