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

x/mobile: go interfaces become objc protocol and interface with the same name #35003

Closed
kazzmir opened this issue Oct 19, 2019 · 1 comment
Closed
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile
Milestone

Comments

@kazzmir
Copy link

kazzmir commented Oct 19, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.5 darwin/amd64

Does this issue reproduce with the latest release?

The go version is irrelevant, as this is a problem with gomobile.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jon/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jon/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/jon/tmp/go-1.12.5/go"
GOTMPDIR=""
GOTOOLDIR="/Users/jon/tmp/go-1.12.5/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gr/_fggz35x6vj7r2yb0rg8hky40000gq/T/go-build207061710=/tmp/go-build -gno-record-gcc-switches -fno-common"

Report (with a fix)

gomobile bind -target ios does not allow one to declare an interface in go, subclass that interface in swift, and then pass an instance of that subclass to go. When the instance is passed to go a runtime error occurs:

go_seq_go_to_refnum on objective-c objects is not permitted'

Essentially my code is

x.go

package gostuff
type Machine interface {
   Work(data string)
}
func Run(machine Machine){ ... }

x.swift

class MyMachine: gostuffMachine {
   func work(_ data:String!){
   }
}
let m = MyMachine() 
gostuffRun(m) // this line causes the runtime exception

The exception is really thrown in the generated code for Run when the incoming objc object is converted to a refnum.

if ([machine conformsToProtocol:@protocol(goSeqRefInterface)]) {
     id<goSeqRefInterface> machine_proxy = (id<goSeqRefInterface>)(machine);
       _machine = go_seq_go_to_refnum(machine_proxy._ref); // I believe its this line
 } else {
    _machine = go_seq_to_refnum(machine);
}

As far as I can tell this is because gostuffMachine has two declarations, one as a @protocol and one as an @interface

@protocol Machine <NSObject>
- (void) work:(NSString* _Nullable) data;
@end

@interface Machine: NSObject<goSeqRefInterface, Machine> {
}
- (nonnull instancetype)initWithRef:(_Nonnull id)ref;
- (void) work:(NSString* _Nullable) data;
@end

When the swift side goes to implemet Machine it picks up the one declared as an interface rather than the protocol (I'm not an objc/swift expert so I don't understand the subtleties here). In any case, the interface version has an initWithRef constructor that only the go code knows how to call when a Machine instance is constructed from go. When swift creates a subclass of Machine it does not call the initWithRef constructor, so _ref is null, and ultimately when the C code gets to

if ([x conformsToProtocol:@protocol(goSeqInterface)])

This condition is true, due to the interface inheriting from goSeqRefInterface, and then the C code dies at trying to invoke go_seq_go_to_refnum(machine_proxy.ref).

The fix I have made so far is to append the word Protocol to the protocols generated by genobjc.go. Essentially this patch:

diff --git a/bind/genobjc.go b/bind/genobjc.go
index 7595fa0..f99ee40 100644
--- a/bind/genobjc.go
+++ b/bind/genobjc.go
@@ -168,7 +168,7 @@ func (g *ObjcGen) GenH() error {
                g.Printf("@class %s%s;\n", g.namePrefix, s.obj.Name())
        }
        for _, i := range g.interfaces {
-               g.Printf("@protocol %s%s;\n", g.namePrefix, i.obj.Name())
+               g.Printf("@protocol %s%sProtocol;\n", g.namePrefix, i.obj.Name())
                if i.summary.implementable {
                        g.Printf("@class %s%s;\n", g.namePrefix, i.obj.Name())
                        // Forward declaration for other cases will be handled at the beginning of GenM.
@@ -874,7 +874,7 @@ func (g *ObjcGen) genInterfaceInterface(obj *types.TypeName, summary ifaceSummar
        }
        prots := []string{"goSeqRefInterface"}
        if isProtocol {
-               prots = append(prots, fmt.Sprintf("%[1]s%[2]s", g.namePrefix, obj.Name()))
+               prots = append(prots, fmt.Sprintf("%[1]s%[2]sProtocol", g.namePrefix, obj.Name()))
        }
        g.Printf(" <%s>", strings.Join(prots, ", "))
        g.Printf(" {\n}\n")
@@ -899,7 +899,7 @@ func (g *ObjcGen) genInterfaceH(obj *types.TypeName, t *types.Interface) {
                g.genInterfaceInterface(obj, summary, false)
                return
        }
-       g.Printf("@protocol %s%s <NSObject>\n", g.namePrefix, obj.Name())
+       g.Printf("@protocol %s%sProtocol <NSObject>\n", g.namePrefix, obj.Name())
        for _, m := range makeIfaceSummary(t).callable {
                if !g.isSigSupported(m.Type()) {
                        g.Printf("// skipped method %s.%s with unsupported parameter or return types\n\n", obj.Name(), m.Name())

This question was asked on stack overflow: https://stackoverflow.com/questions/50815664/gomobile-binding-callbacks-for-objc
and that question was also added to the end of an old gomobile issue but left unaddressed: #17102 (comment)

Anyway, I was initially quite confused about the default behavior of gomobile. Based on the following blog post I had assumed this would all work out fine: https://medium.com/@matryer/tutorial-calling-go-code-from-swift-on-ios-and-vice-versa-with-gomobile-7925620c17a4

In fact that blog post does exactly what I'm trying to accomplish, and even says the protocol name ends with Protocol, yet clearly the genobjc.go file in gomobile does not name protocols this way. The naming of protocols seems to have been done in bf2ca7a93b02169b6f8c221ba13b22919a728994, from 2015.

I have not tested this change with pure objc projects, but I don't think it breaks any functionality there (except for changing naming, same as in swift).
If you need a small test case I can produce one.

@gopherbot gopherbot added this to the Unreleased milestone Oct 19, 2019
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Oct 19, 2019
@kazzmir
Copy link
Author

kazzmir commented Oct 19, 2019

Ugh nevermind on this, it was my mistake. The objc protocol can be used directly in swift already by appending the Protocol name.

The generated x.h objc header

@protocol Machine
@end
@interface Machine
@end
class MyMachine: MachineProtocol {
}

Now everything works fine, I can pass an instance of MyMachine directly to go.

I also see that the medium blogpost I referenced discuss this exact point, but I glossed over it before. It would be useful if the gomobile documentation referenced this point perhaps.

@kazzmir kazzmir closed this as completed Oct 19, 2019
@golang golang locked and limited conversation to collaborators Oct 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile
Projects
None yet
Development

No branches or pull requests

2 participants