Give up supporting Go 1.16 and Go 1.17

[hajimehoshi]

• gccgo12 already supports Go 1.18 (https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgo/VERSION;h=39560f0fcdd63e4bb8eead6a3f849e91636d31eb;hb=2ee5e4300186a92ad73f1a1a64cb918dc76c8d67)
• DebianUbuntu stable already supports Go 1.18 (note that UbuntuDebian stable supports Go 1.15 by default...)
• The Go team no longer supports Go 1.17 and older, as we know

@TotallyGamerJet what do you think? Are there extra tasks for this?

[TotallyGamerJet]

There isn't anything that needs to be done since purego already works for 1.18. Updating to 1.18 would allow us to remove the stack and register hack files though. This is because on Darwin amd64 and arm64 would use register based calling convention by 1.18

[hajimehoshi]

Another reason is that semi-standard libraries like golang.org/x/sys/unix uses unsafe.Slice, which Go 1.16 doesn't support.

There isn't anything that needs to be done since purego already works for 1.18. Updating to 1.18 would allow us to remove the stack and register hack files though. This is because on Darwin amd64 and arm64 would use register based calling convention by 1.18

Sure! As there seems no issue, I'll drop Go 1.16 and 1.17 supports from Ebitengine and Oto first. After that, we can remove the hacks for old Go. It's not hurry anyway.

[TotallyGamerJet]

Oh yeah! unsafe.Slice will cleanup some of the code

[hajimehoshi]

Ebitengine: hajimehoshi/ebiten@606ff77
Oto: ebitengine/oto@ad5f033

OK, it's ready! As I said, it's not hurry.

[hajimehoshi]

By the way, do you think it is possible to remove go-vet warnings?

$ go vet ./...
# github.com/ebitengine/purego/objc_test
objc/objc_runtime_darwin_test.go:31:41: possible misuse of unsafe.Pointer
objc/objc_runtime_darwin_test.go:34:34: possible misuse of unsafe.Pointer

I feel like these are false positives. See also golang/go#56487

[TotallyGamerJet]

Yeah these are false positives. My first inclination is to use the address hack. Like here. But I'll see if there is just a better way to write it that avoids the message

[hajimehoshi]

golang/go#56487 (comment) This might be fixed at Go 1.20!

[TotallyGamerJet]

That would be awesome! I do want to point out that the two test messages are actually examples that should be replaced with unsafe.Add. So a different fix is still likely needed then the one that issue addresses

[TotallyGamerJet]

So I came up with a clever solution. Since Objective-C objects are just pointers to C structs with the first field being type objc.Class why not replicate the struct like we do others (source).

So the ExampleClass_AddIvar would look something like this:

type barT struct {
  isa objc.Class
  bar int
}
var class = objc.AllocateClassPair(objc.GetClass("NSObject"), "BarObject", 0)
class.AddIvar("bar", int(0), "q")
class.AddMethod(objc.RegisterName("bar"), objc.NewIMP(func(self *barT, _cmd objc.SEL) int {
  return self.bar
}), "q@:")
class.AddMethod(objc.RegisterName("setBar:"), objc.NewIMP(func(self *barT, _cmd objc.SEL, bar int) {
  self.bar = bar
}), "v@:q")
class.Register()

And we can verify that the field has the right offset by testing like:

var barOffset = class.InstanceVariable("bar").Offset()
if barOffset != unsafe.Offsetof(barT{}.bar) {
  panic("offsets are different")
}

This requires a small change to support IMPs that take a pointer struct whose structure matches Class:

diff --git a/objc/objc_runtime_darwin.go b/objc/objc_runtime_darwin.go
index a38fc6f..872c4cf 100644
--- a/objc/objc_runtime_darwin.go
+++ b/objc/objc_runtime_darwin.go
@@ -180,7 +180,11 @@ func NewIMP(fn interface{}) IMP {
        switch {
        case ty.NumIn() < 2:
                fallthrough
-       case ty.In(0).Kind() != reflect.Uintptr:
+       case ty.In(0).Kind() != reflect.Uintptr && // checks if it's objc.ID
+               // checks if it's a pointer to a struct
+               (ty.In(0).Kind() != reflect.Pointer || ty.In(0).Elem().Kind() != reflect.Struct ||
+                       // and that the structs first field is an objc.Class
+                       ty.In(0).Elem().Field(0).Type != reflect.TypeOf(Class(0))):

Doing so makes the tests completely type safe.

Also, It might be nice to have an API that creates the objc.Class from the fields and methods of a Go struct. It would also check that each of the methods like AddMethod and AddIvar actually succeed. It would be something similar to:

var class_BarObject objc.Class

type _BarObject struct {
	isa objc.Class
	bar int
}

func (b * _BarObject) getBar(_cmd objc.SEL) int {
	return b.bar
}

func (b * _BarObject) setBar(_cmd objc.SEL, bar int) {
	b.bar = bar
}

func init() {
  // RegisterClass takes a pointer to a struct whose fields matches that of an objective-c object
  // and the name of the superclass. It returns the created class or an error message if it failed to.
  // The created objc.Class will have Ivars created for each field in the struct and methods added
  // for each method that takes a pointer receiver.
  var err error
  class_BarObject, err  = objc.RegisterClass(& _BarObject, "NSObject")
  // ... check error here
}

Thoughts? I think we should create a new issue to track a solution to the go vet issue instead of responding in this closed one.

[hajimehoshi]

Nice idea!

Thoughts? I think we should create a new issue to track a solution to the go vet issue instead of responding in this closed one.

Well, I think tracking this by this issue #54 is fine, but it's also fine to have another issue.

[TotallyGamerJet]

Well, I think tracking this by this issue #54 is fine, but it's also fine to have another issue.

Okay. Is there anything else to track now that #57 was merged? I guess if there are any updates on the use of unsafe.Slice

[hajimehoshi]

Okay. Is there anything else to track now that #57 was merged? I guess if there are any updates on the use of unsafe.Slice

So we can now replace unsafe.Slice usages in Oto and Ebitengine, right?

[TotallyGamerJet]

Yep! :D