-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
functions: Add support for functions #193
base: main
Are you sure you want to change the base?
Conversation
Based on current state:
|
I have only really implemented it for RefType -- I defaulted to the objc impl but I can put in "not implemented" instead.
Ha, no are accidental.
Suggestion for the short term?
Yeah I have that rule in there constraining what is generated just for testing purposes, probably better to add in skipping when we know we'll fail but I need to explore more, any input on approach welcomed. |
Use Since functions are simple symbol names, they could be added to the blacklist in Also maybe for now just have a mapping for C types or rules for what C type to use in your codegen or in ToFunction. Until you can see how many special cases there are that a CName function would help enough for. |
9df8d62
to
2a74281
Compare
OK I think this is ready for a review pass. This isn't complete work yet but I think might have enough of a bite for us to figure out what might go in. Severely needs more testing. |
generate/codegen/modulewriter.go
Outdated
return true | ||
} | ||
if _, ok := map[string]bool{ | ||
"CGDirectDisplayCopyCurrentMetalDevice": true, |
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.
can we have reasons for why these are skipped in comments?
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.
good call, largely an artifact of development and these having edge cases, I'll pull this out for now as I'm preferring to flag things off but I agree we should include comments with any exceptions.
Tried running generate on appkit and with a few fixes it generates but then compiling a program gives a lot of these warnings now:
But more importantly, these errors:
Is this a known part of the process? Like is this something you know how to handle? |
Sorry missed this comment, not a known part of the process, my hope/intent was that once we have the structs appropriately defined with the same memory layout that this is just a cast. |
I rebased all of this post-darwinkit rename and put in the ability to flag on/off struct and function support on a per-module basis. This is still introducing some changes to non-flagged-on modules so it's not complete but it's getting there. |
@@ -0,0 +1,19 @@ | |||
package codegen |
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 since you originally submitted this i added a struct generator and we should be generating all of them for the frameworks we have: https://github.com/progrium/darwinkit/blob/main/generate/tools/structs.go
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.
Oh great, thanks, happy to cull this.
if m.Module.Flags.GenerateStructs { | ||
m.WriteStructs() | ||
} | ||
if m.Module.Flags.GenerateFunctions { |
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.
whats the reason for putting it behind a flag?
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.
The rationale here is that we can add capabilities here incrementally by flagging in/on particular packages. When I was hacking on this previously I kept finding esoteric corners that were adding work.
Basic idea is that we start opt-in, get a few key packages going, and once we feel like it's ready to turn on everywhere I'd just remove the flag and press it all out.
// this keeps track of types that we know we can't handle. | ||
var unhandledStructTypes = map[string]bool{ | ||
"CFAllocatorContext": true, | ||
"CFArrayCallBacks": true, |
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 we have most of these, for example:
https://github.com/progrium/darwinkit/blob/main/macos/corefoundation/corefoundation_structs.go#L232
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.
Gotcha, I should have cleaned this list up, will do.
There's a lot here that I think unfortunately we can do without if we use purego. Also, it would be nice to see an example output. I'm pretty sure function generation with purego should be simple enough it could be an out-of-band (of generate) generation like for structs. If it took an argument to do it for a particular framework, we don't need module flags. A good first pass would be generating unexported function vars for purego to populate, since the "hard" part is translating DarwinKit types to the simpler purego types (often uintptr). Here are the purego tests I originally did with Apple APIs. This one is with basic types: package main
import (
"fmt"
"github.com/ebitengine/purego"
)
func main() {
// corefoundation function test using primitive types
var cfStringCreateWithCString func(uintptr, string, uint32) uintptr
var cfRelease func(uintptr)
var cfShow func(uintptr)
var UTF8 uint32 = 134217984
cf, err := purego.Dlopen("/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation", purego.RTLD_GLOBAL)
if err != nil {
panic(fmt.Errorf("objc: %w", err))
}
purego.RegisterLibFunc(&cfStringCreateWithCString, cf, "CFStringCreateWithCString")
purego.RegisterLibFunc(&cfRelease, cf, "CFRelease")
purego.RegisterLibFunc(&cfShow, cf, "CFShow")
s := cfStringCreateWithCString(0, "Hello world", UTF8)
cfShow(s)
cfRelease(s)
} And then here's one that uses structs (which we have now!): package main
import (
"fmt"
"github.com/ebitengine/purego"
)
type CGRect struct {
Origin CGPoint
Size CGSize
}
type CGPoint struct {
X float64
Y float64
}
type CGSize struct {
Width float64
Height float64
}
func main() {
// coregraphics function test using structs
var CGRectContainsPoint func(CGRect, CGPoint) bool
var CGMainDisplayID func() uint32
var CGDisplayBounds func(uint32) CGRect
cg, err := purego.Dlopen("/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics", purego.RTLD_GLOBAL)
if err != nil {
panic(fmt.Errorf("objc: %w", err))
}
purego.RegisterLibFunc(&CGMainDisplayID, cg, "CGMainDisplayID")
purego.RegisterLibFunc(&CGRectContainsPoint, cg, "CGRectContainsPoint")
purego.RegisterLibFunc(&CGDisplayBounds, cg, "CGDisplayBounds")
fmt.Println(CGRectContainsPoint(CGRect{
Origin: CGPoint{X: 0.0, Y: 0.0},
Size: CGSize{Width: 10.0, Height: 10.0},
}, CGPoint{X: 5.0, Y: 12.0}))
disp := CGMainDisplayID()
fmt.Println(CGDisplayBounds(disp))
} |
Thanks for taking the time to review. I'm flexible on approach here and have zero qualms if this is just scrapped -- I just want to call mps and screencapturekit without doing triple backflips. Perhaps zooming out a sec and being clear on how we want functions and types to work from a user's perspective would be good. Do you want to make a call on purego usage and define the sort of api shape we want to expose to darwinkit users? |
Yeah to start we can just wrap the purego funcs with the exact args/return in an exported version of the function. Just to see what we get and make sure they work at all with the primitive types. And then after that we can work to change the types of the exported functions to something that makes more sense with the rest of DarwinKit (like taking IObject or any of our binding types and converting to uintptr). |
Also quick tip: most of the Make* functions (like NSMakeRect) are inlined, so their symbols won't exist and cant be called via purego. We'd have to just generate native Go functions for those. They're usually just simple constructors. We might even just rename them (NewRect) too |
This adds support for functions and structs which should largely complete the relevant API surface area that this project addresses.