-
Notifications
You must be signed in to change notification settings - Fork 89
Windows Struct Support: Organization + Tests can build a DLL #334
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
base: main
Are you sure you want to change the base?
Conversation
My first implementation failed, but this was the first step to get things building/testable. I'll give it a second go tomorrow with fresh eyes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you're still working on this but wanted to leave some comments to help
"testing" | ||
"unsafe" | ||
|
||
"github.com/ebitengine/purego" | ||
"github.com/ebitengine/purego/internal/load" | ||
) | ||
|
||
func getSystemLibrary() (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can stay here instead of moving it to a separate file
@@ -186,41 +162,3 @@ func TestABI(t *testing.T) { | |||
} | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also keep this here and just modify it to support windows. The reason is it is also used by Linux so if it's pulled out it'd create another file which isn't necessary
@@ -3,43 +3,52 @@ | |||
|
|||
#include "stdint.h" | |||
|
|||
#if defined(__x86_64__) || defined(__aarch64__) | |||
#if defined(__x86_64__) || defined(__aarch64__) || defined(_WIN32) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why specifically add the windows check is it not x86 or arm?
// Empty is empty | ||
struct Empty {}; | ||
struct Empty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the braces?
What issue is this addressing?
Closes #237
What type of issue is this addressing?
Feature
What this PR does | solves
Adds struct parameter and return support for Windows (amd64)