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

functions: Add support for functions #193

Draft
wants to merge 32 commits into
base: main
Choose a base branch
from
Draft

Conversation

tmc
Copy link
Collaborator

@tmc tmc commented Aug 21, 2023

This adds support for functions and structs which should largely complete the relevant API surface area that this project addresses.

@progrium
Copy link
Owner

Based on current state:

  • nice that ref structs are added too, I imagine they are maybe a common type used by functions
  • is CName ever not ObjcName?
  • those dot files intended to be included?
  • still a lot of framework code for frameworks we don't have in main yet. tests will fail with this
  • are the only examples of successfully generated functions the two coregraphics ones?

@tmc
Copy link
Collaborator Author

tmc commented Aug 22, 2023

Based on current state:

  • nice that ref structs are added too, I imagine they are maybe a common type used by functions
  • is CName ever not ObjcName?

I have only really implemented it for RefType -- I defaulted to the objc impl but I can put in "not implemented" instead.

  • those dot files intended to be included?

Ha, no are accidental.

  • still a lot of framework code for frameworks we don't have in main yet. tests will fail with this

Suggestion for the short term?

  • are the only examples of successfully generated functions the two coregraphics ones?

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.

@progrium
Copy link
Owner

Use ./generate/tools/clobbergen.go on the frameworks that were generated by accident. Or, git reset the macos dir against upstream and then run ./generate/tools/regen.sh to only generate for the existing frameworks. This is all temporary because normally there wouldn't be so many framework stubs, but I started them all and haven't finished getting them working. Would have put them in a new branch but I thought I'd finish them sooner. Whatever ones I don't get working in this next pass I can take out and then this won't be a problem.

Since functions are simple symbol names, they could be added to the blacklist in ./generate/symbols.go to skip any that aren't working yet. For now, maybe make the function case in generate see if the framework is in an inline whitelist. Then try to get functions to mostly work in one framework at a time. You could use the lookup tool with a framework prefix and jq to just get the functions and see which frameworks we have have the fewest functions and work framework to framework taking care of new cases as you run into them. By the time you get to appkit or foundation, maybe it'll all just work!

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.

@tmc tmc force-pushed the functions branch 2 times, most recently from 9df8d62 to 2a74281 Compare September 3, 2023 02:59
@tmc tmc changed the title Functions functions: Add support for functions Sep 3, 2023
@tmc
Copy link
Collaborator Author

tmc commented Sep 7, 2023

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.

return true
}
if _, ok := map[string]bool{
"CGDirectDisplayCopyCurrentMetalDevice": true,
Copy link
Owner

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?

Copy link
Collaborator Author

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.

@progrium
Copy link
Owner

Tried running generate on appkit and with a few fixes it generates but then compiling a program gives a lot of these warnings now:

# github.com/progrium/macdriver/macos/appkit
cgo-gcc-prolog:52:66: warning: incompatible pointer types passing 'struct NSString *' to parameter of type 'NSAccessibilityActionName' (aka 'NSString *') [-Wincompatible-pointer-types]
macos/appkit/functions.gen.go:60:69: note: passing argument to parameter 'action' here
cgo-gcc-prolog:106:44: warning: incompatible pointer types passing 'struct NSString *' to parameter of type 'NSAccessibilityNotificationName' (aka 'NSString *') [-Wincompatible-pointer-types]
macos/appkit/functions.gen.go:19:85: note: passing argument to parameter 'notification' here
cgo-gcc-prolog:120:56: warning: incompatible pointer types passing 'struct NSString *' to parameter of type 'NSAccessibilityNotificationName' (aka 'NSString *') [-Wincompatible-pointer-types]
...

But more importantly, these errors:

macos/appkit/functions.gen.go:88:14: cannot convert boundsRect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:91:14: cannot convert clipRect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:110:14: cannot convert rect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:132:27: cannot convert role (variable of type AccessibilityRole) to type _Ctype_NSAccessibilityRole
macos/appkit/functions.gen.go:135:30: cannot convert subrole (variable of type AccessibilitySubrole) to type _Ctype_NSAccessibilitySubrole
macos/appkit/functions.gen.go:138:16: cannot convert rv (variable of type *_Ctype_struct_NSString) to type string
macos/appkit/functions.gen.go:148:14: cannot convert rect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:193:14: cannot convert rect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:209:39: cannot convert notification (variable of type AccessibilityNotificationName) to type _Ctype_NSAccessibilityNotificationName
macos/appkit/functions.gen.go:220:14: cannot convert rect (variable of type coregraphics.Rect) to type _Ctype_struct_CGRect
macos/appkit/functions.gen.go:220:14: too many errors

Is this a known part of the process? Like is this something you know how to handle?

@tmc
Copy link
Collaborator Author

tmc commented Nov 13, 2023

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.

@tmc
Copy link
Collaborator Author

tmc commented Jun 22, 2024

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.

@tmc tmc changed the title functions: Add support for functions functions: Add support for functions and structs Jun 22, 2024
@@ -0,0 +1,19 @@
package codegen
Copy link
Owner

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

Copy link
Collaborator Author

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 {
Copy link
Owner

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?

Copy link
Collaborator Author

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,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

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.

@progrium
Copy link
Owner

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))
}

@tmc
Copy link
Collaborator Author

tmc commented Jun 27, 2024

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?

@progrium
Copy link
Owner

progrium commented Jun 27, 2024

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).

@progrium
Copy link
Owner

progrium commented Jun 27, 2024

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

@tmc tmc changed the title functions: Add support for functions and structs functions: Add support for functions Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants