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

capnpc-go: deal with subtyping/inheritance in capnp interfaces #42

Open
zenhack opened this issue Jul 31, 2016 · 9 comments
Open

capnpc-go: deal with subtyping/inheritance in capnp interfaces #42

zenhack opened this issue Jul 31, 2016 · 9 comments

Comments

@zenhack
Copy link
Contributor

zenhack commented Jul 31, 2016

So, I just got a basic program running that responds to a GET / via the normal net/http package API, but talking to the sandstorm api server. woo! but it involved this hack:

zenhack/go.sandstorm@1d5b98e

In the capnp schema, WebSession inherits from UiSession, but the generated go packages don't reflect this in a useful way.

@zombiezen
Copy link
Contributor

For servers, I copy the methods into the _Server interface to avoid https://golang.org/issue/6977. Cap'n Proto interfaces allow multiple inheritance, so you can quickly get into the diamond problem.

For clients, I don't have a better answer than the "hack" that you used. Go doesn't have an equivalent idea about subtyping for structs. While the generated code could instead use interfaces, this would introduce inefficiency, and would be at odds with the fact that capabilities are loosely typed anyway: you don't whether a capability actually implements an interface until you call the methods. Think of the generated client types as a weak hint.

That said, I'm sorry this is clunky. This just seemed like the least clunky thing in the design space while I was looking at it six months ago. If you think of a better way to do it, I'd love to hear it!

@zenhack
Copy link
Contributor Author

zenhack commented Jul 31, 2016

I'll mull it over.

@zenhack
Copy link
Contributor Author

zenhack commented Aug 21, 2016

One thing that might make this a little nicer: we could add convience methods for the parent interfaces, to do the conversion. e.g. you'd have:

myWebsession.ToUiSession()

which would basically do what my patch above is doing. These would be defined only for the parent types declared in the schema. You're still doing the upcast manually, but it's a bit less clumsy, and it adds some typesafety, since you can't use it to make an invalid conversion.

Thoughts?

@zombiezen
Copy link
Contributor

I like that idea. One thought: what should happen if an interface inherits from multiple interfaces of the same name? Certainly not a showstopper, as capnpc-go has generally punted on this, but I'd like that to get better over time.

@zenhack
Copy link
Contributor Author

zenhack commented Aug 23, 2016

I think I'm going to mull over #46 for a bit, keeping this in mind as well. Any solution to #46 would have to account for this as well.

@zombiezen
Copy link
Contributor

Acknowledged. golang/protobuf solves this better; you should take a look there for inspiration.

@zombiezen
Copy link
Contributor

In the context of this bug, we could use the interface IDs to differentiate multiple parents of the same name.

@zombiezen zombiezen changed the title Deal with subtyping/inheritance in capnp interfaces. capnpc-go: deal with subtyping/inheritance in capnp interfaces May 18, 2017
@zenhack
Copy link
Contributor Author

zenhack commented May 18, 2017

Not sure I see how that would work? My brain immediately goes to something like websession.To(UiSession_Type), but the types don't work at all. You could tack the actual number on the end of the To_ method, but that kinda sucks from an API perspective, and to get something readable you'd have to use a name, which puts you back at square one.

It might improve ergonomics in general to generate more concise names where a lack of collisions permits it. For example, Sandstorm's web-session schema defines a Response struct in the scope of WebSession, but there's no other type in the schema called Response. Right now the type gets translated to WebSession_Response, so you end up with the very verbose and redundant websession.WebSession_Response. It might make things nicer if the code generator could work out that it won't cause collisions to chop of the WebSession_ and do so, so you'd just have websession.Response.

With that change in place, we could maybe shove package names on the front of colliding interface names in the To* methods, relying on the fact that this is unusual to make it not suck in the general case.

@zombiezen
Copy link
Contributor

My idea was more to make ToUiSession789abcdef methods, but as I go to write it out, it sounds awful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants