-
Notifications
You must be signed in to change notification settings - Fork 197
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
Go: Improve the ergonomics of Go bindings #612
Comments
The biggest improvement that I would love to see is the elimination of CGO and c bindings. |
A common representation of "sum types" in Go is a "sealed" interface: type C1 interface {
// The method is unexported so other packages cannot implement this interface.
isC1()
}
type C1A int32
func (C1A) isC1() {}
type C1B int64
func (C1B) isC1() {}
type C1C string
func (C1C) isC1() {}
// Usage:
var c1 C1 = C1A(1)
switch v := c1.(type) {
case C1A:
// v is a C1A here
case C1B:
// ...
case C1C:
// ...
default:
panic("unreachable")
} We could alternatively use single-field type C1A struct {
Value int32
}
c1a := C1A{1} // less obvious construction...
i := c1a.Value // ...but you don't have to name the `int32` |
Is there some inherent limitation that prevents us from putting each interface (or at least package) in its own Go package? |
Assuming we can find a way to turn Another possibility for |
Thanks, I remember I have studied this representation, but eventually decided to take the current approach. The current approach can do pretty much the same thing as you described: var c1 C1 = C1A(1) // constructor
switch kind := c1.Kind() {
case C1KindA:
val := c1.GetA()
case C1KindB:
case C1KindC:
default
} From the ergonomics point of view, do you think the "sealed" representation is better than the current one? @lann |
No, I don't think so. That's why I mentioned it. I believe the |
@Mossaka in the JS bindgen we special case top-level errors as being idomatic (try-catch), as distinct from other Result positions. Perhaps something similar might apply to Go? |
I personally prefer it, and I think it is a more conventional pattern in Go. Protobuf's generated Go uses a similar approach, for example. Another (minor) advantage is that it uses slightly less space as it doesn't need a separate |
That would certainly help with verbosity. I think a nested As an aside, it should be possible and quite nice to support e.g. |
This is what the top post's "most Go functions return more idiomatic value, err" is getting at; the Go equivalent of a function returning
|
In JS, we use the standard tagged error object (as per options and variants) in all the other places -
We had something very similar in JS, and while I'm still not completely happy with where we ended up, it did solve this concern - we create a special |
I spent the last few days messing with the output of Ill need to check, but does the wit grammer have a concrete Assuming something like this
IMO:
I'm not entirely sure of the "rules" on wrapping types ( As a personal preference, I would like to see Generics removed as much as possible in favor of interfaces as well. Something like an Something like
package gen_types
type Optionable interface {
IsNone() bool
IsSome() bool
Unwrap() error
Set(val any) error
Unset() error
}
type derp string
func (d derp) IsNone() {}
func (d derp) IsSome() {}
func (d derp) Unwrap() {}
func (d derp) Set() {}
func (d derp) Unset() {} All that said, I want to thank you @Mossaka ..... 1) for getting this out for the go community and 2) continuing to see it though |
I think the main issue with (not having) generics is that the Example: https://go.dev/play/p/5U2IMy7lesx |
It might be nicer to name the variants after the Go types they represent, e.g.
|
Thank you all for suggestions. I will leave this issue open a bit longer for more discussion, comments and suggestions. Then I will make a task list for the implementation. Have a good weekend everyone! 🥂 |
** Edit simplifying some interfaces to structs ** I stumbled upon this issue after writing the following library that I think could serve as a good example of the "sealed" interface pattern: https://github.com/patrickhuber/go-types The library has Result[T], Option[T], Tuple2[T1,T2], Tuple3[T1,T2,T3] and Tuple4[T1,T2,T3] To show an example, I'll pull code from the repo The result type creates the Result[T] interface which Ok[T] and Error[T] must implement: // Result represents a value or an error
type Result[T any] interface {
// result allows inheritance modeling
result(t T)
// Deconstruct expands the result to its underlying values.
// if the result is an error, T contains the zero value.
Deconstruct() (T, error)
// IsOk returns true for Ok results, false for Error results.
IsOk() bool
// IsError returns false for Ok results, true for Error results.
// IsError accepts a list of errors and errors.Is() matches the internal error, returns false.
// IsError ignores the error list if the result type is Ok.
IsError(err ...error) bool
// Unwrap attempts to unwrap the result to its value.
// If the result is an Error, Unwrap will panic.
// Unwrap is meant to be used with handle.Error(*types.Result)
Unwrap() T
} The Ok[T] struct implements the Result[T] interface type Ok[T any] struct{
Value T
}
func (o Ok[T]) result(t T) {}
func (o Ok[T]) Deconstruct() (T, error) {
return o.Value, nil
}
func (o Ok[T]) IsOk() bool {
return true
}
func (o Ok[T]) IsError(err ...error) bool {
return false
}
func (o Ok[T]) Unwrap() T {
return o.Value
} The Error[T] struct is basically a wrapper around the error. The Result[T] type could be defined as a Result[TOk, TError] if there is a desire to represent any error type instead of the standard error type. type Error[T any] struct {
Value error
}
func (e Error[T]) result(t T) {}
func (e Error[T]) Deconstruct() (T, error) {
var t T
return t, e.Value
}
func (e Error[T]) IsError(err ...error) bool {
// no filters so match any error
if len(err) == 0 {
return true
}
// must match one of the filters
for _, target := range err {
if errors.Is(e.err, target) {
return true
}
}
return false
}
func (e Error[T]) IsOk() bool {
return false
}
func (e Error[T]) Unwrap() T {
panic(fmt.Errorf("unable to match %T", e))
} Notable methods to improve ergonomics for go programmers are the Deconstruct()(T, error) methods on Ok[T] and Error[T]. They basically off-ramp from these types into traditional go code. There are also some onramp functions in the result package that create results for common function return types. There is some experimental error handling with the Result[T].Unwrap() functions that I would advise using with caution as panic in go libraries is generally considered bad. I do like how the Option[T] is implemented with the same pattern and contains Some[T] and None[T] interfaces and corresponding types. type Option[T any] interface {
option(t T)
Deconstruct() (T, bool)
IsSome() bool
IsNone() bool
Unwrap() T
} None[T] inherits private methods from Option[T] and also provides an additional private method none(T). Again, Unwrap is provided for experimental error handling and should be avoided. type None[T any] struct {}
func (None[T]) option(t T) {}
func (None[T]) Deconstruct() (T, bool) {
var t T
return t, false
}
func (None[T]) IsSome() bool { return false }
func (None[T]) IsNone() bool { return true }
func (None[T]) Unwrap() T {
panic(fmt.Errorf("unable to match %T", n))
} Some[T] provides an implementation of Option[T] with a value. type Some[T any] struct{
Value T
}
func (Some[T]) option(t T) {}
func (s Some[T]) Deconstruct() (T, bool) {
return s.Value, true
}
func (Some[T]) IsSome() bool { return true }
func (Some[T]) IsNone() bool { return false }
func (Some[T]) Unwrap() T {
return s.value
} Another advantage of this pattern, is it allows you to utilize rust style matches with go's type switch. As seen here and here So you end up with the ability to do the following: var zero T
switch r := res.(type){
case types.Ok[T]:
return r.Value, nil
case types.Error[T]:
return zero, r.Value
} |
Thanks for raising this issue, the status quo is quite not ergonomic. It would really be best for everyone to separate desire to push component model technology from it also being tech debt. For example, the current PR to add wasi-http to dapr would be a lot less polarizing if it didn't imply some significant guest side tech debt. I don't know why this was raised prior to this issue closing, but anyway I hope this can close so we can conserve time. |
unsolicited 2p is to make sure you get folks who are fulltime in Go on the design review as there are some notable cultural differences between it and rust, especially being more imperative than FP. Even generics aren't widely used sometimes. Ideally someone who contributes directly to go and/or tinygo. |
Respectfully, coming into another project with, as you point out, unsolicited input and telling people what's "best for everyone" is incredibly rude, and I would ask you to refrain from such behavior. You're free to disagree with what people are doing here, but if you provide input here, please do it in a constructive and actionable way.
I can't comment on the PR you linked to because I don't know much about it, but I do not think this is the right place to voice your concerns about the timing of any work on wasm components integration into Dapr. If you think that integration work is premature, it seems like that PR itself is exactly the right place to voice these concerns (as, from a quick glance, you seem to have) and try to convince the project maintainers of your position.
I will reiterate that you're being incredibly disrespectful. This very issue is about improving the ergonomics of the Go bindings, and ensuring that they're idiomatic. We're deeply aware that it's of utmost importance to ensure that the component model is as well integrated as possible into as many language ecosystems as possible, and this issue is a perfect example of people (including people with lots of Go experience) engaging in discussion to ensure we do what we can to pursue this goal. I think the conversation isn't yet even at a point where any specific point in the design space is being honed in on, but if you have specific concerns about the direction, your concrete feedback on that would be welcome. Truisms about how Go and Rust are different—that I can't interpret in any way but to assume (despite the evidence in this very issue) are based on the belief that the people working on this are somehow not aware of there even being difference—are not. |
In hindsight, I phrased things poorly. I should have said something like... This issue is an example of people recognizing areas of improvement on TinyGo. For example, it was mentioned here also that the CGO approach is problematic as well ergonomics. It would be better for some maintainers (like me), if these sorts of issues were resolved before adding guest dependencies on the status quo. This would ack I can't speak for everyone and maybe there is someone who doesn't consider CGO approach for tinygo a problem or tech debt. Such people probably would like this issue to remain unsolved I guess. Anyway I get it and I won't discuss problems anymore here. |
Note that I never said you shouldn't bring up any problems here. I was requesting you do so in a respectful and actionable manner. Based on your last comment it sounds like you think the discussion here is going in the right direction, and there currently aren't any problems to address here? (To be clear, I get that you think there are problems with the dapr PR.) |
Since asked, here's what I think could be improved beyond what's already noticed. As the contributor to dapr is a WASI lead, I didn't split hairs. Maybe I should have to reduce confusion. I think there's holistic sense to couple things like this. Encourage WASI leads to not raise pull requests to cement in the CGO based approach to external projects.Instead, finish this first. It is more empathetic to other maintainers (my opinion not everyone's). Skipping this step adds burden to other people who will have to very tactically teach people how to do CGO, just to later tell them not to. Get an outside review on the API approach by someone active in TinyGo and GoFor example, not all approaches work in Go and this is hard to tell. While also possible to assume ergo is fine, I'm constantly surprised to find different thoughts on the same topic. For example, generics and patterns like Option are polarizing. With someone closer to Go/TinyGo on this, some pitfalls can be avoided. Think about testing and how that will workTinyGo is difficult to test, but that doesn't mean it shouldn't happen. One polarizing thing about the other pull request is that it effectively has no guest tests (testing.T) or benchmarks (testing.B). At least in codegen here, see if you can find a way to build in a way to test it, so that people can at least copy/paste the pattern. Leaving things untested, especially not benchmarked, can result in poor experiences and conflicted feedback. Hope this helps. |
one last thing is that tinygo is actually maintained by people who have stake in wasmtime. Literally, |
Thank you, I appreciate the input.
I think there might be a mistaken assumption here: there isn't some kind of group that coordinates what people try to do with components and where they open PRs. The dapr PR for example isn't by someone who could be construed as a "WASI lead", by whatever reasonable definition of that term. I also think that if the dapr project comes to the conclusion that accepting this PR would introduce too much churn, that's a perfectly fine outcome. But if the conclusion is instead that integration something experimental now and making changes as needed later, that seems fine, too? Since you mention that you yourself are a potentially affected maintainer, it seems like for that part the easiest solution is for you to reject any such PRs with a reference to the expected turn. And for other maintainers I think it makes sense to treat them as adults and let them make their own decisions. |
I misunderstood things and really sounds like I'm not providing helpful feedback. I prefer not to go round-and-round, rather call it a day. Good luck and hope some of what I wrote is useful to someone here. |
(@Mossaka solicited my feedback, as I initially worked with him to define the current sum types codegen.
@lann I see how using a sealed interface could make type-switching a little less verbose that the sealed struct + Kind approach, but using sealed interfaces precludes optimizing for small sized types (i.e. everything that fits into a Following your example, image I have this variable: On the other hand, the sealed struct approach allows for fine grained optimizations. Although they are not implemented yet, they would look like this (skipping some checks): type C1Kind int
const (
C1KindA C1Kind = iota // int32
C1KindB // int64
C1KindC // string
)
type C1 struct {
kind C1Kind
// num holds the value forC1KindA and C1KindB,
// and the string length for C1KindC.
num int64
// any holds the backing array for C1KindC
any any
}
func (c1 C1) Kind() C1Kind {
return c1.kind
}
func (c1 C1) Int32() int32 {
return int32(c1.num)
}
func (c1 C1) Int64() int64{
return c1.num
}
func (c1 C1) String() string {
return unsafe.String(c1.any.([]byte), c1.num)
} Worth noting that I want to clarify that this approach is not novel, it is widely used in performance-critical libraries, such as in the new log/slog package. To summarize, IMO having a potentially alloc-free API compensates making it a little bit more verbose. |
Thanks for the context. I agree that avoiding allocs may be worth slightly worse ergonomics. |
@patrickhuber I really like this representation! It's clean and super easy to use. I've noted that this thread has suggested that generics in Go aren't that common. I've been thinking about a feature flag in the go-bindgen that when it's turned on, go-bindgen generates generics bindings, and when it's off, it generates non-generics bindings, leaving users a choice to choose. |
It seems like there are enough threads in this issue that it may be worth splitting them into separate topic issues, e.g. for generic |
Seems like a good idea! I am happy to create an issue for each topic and link them to here. |
Glad you like it. I was thinking that a non-generic representation should be easy enough, you would just be using I'm not sure what the community sentiment is on generics. I like them but there may be some issues of which I'm unaware. As of go 1.18 generics are supported. I think there is still some caution with their use because of this stance from the go team in the 1.18 release notes. Speeding up to current day, generics are getting more and more presence in the standard library https://tip.golang.org/doc/go1.21#library . |
It is almost an end of another week, and I feel very grateful for all the insightful feedback on the improvement of the ergonomics of the Go bindings. I figured that I should do a succinct summary of the key proposals: Please note that the following points do not have a particular ordering.
To keep discussions focused, I will be creating an issue for each of these points. This should spare everyone the task of navigating through an exceedingly lengthy thread. |
An update: @achille-roussel and I are organizing the first BCA SIG-Go meeting on Aug 29th. Invitations will be sent to all who expressed interests in the group. I will give a presentation on the current state of the wit-bindgen-go, discussing various proposals (e.g. #612, #640 and #614). We will vote and make decisions on what approaches to adopt, plan out the work items and call for actions! |
I am raising this issue to create a place for discussing various ways we can improve the ergonomics of the Go bindings. When I was writing the
wit-bindgen-go
crate, I admit that I didn't pay enough attention to how the developer experience of the Go bindings would look like. Now since there are a few places where people discussed the needs to have a better experience (this, this), I think it's time to start a conversation.What the go bindings looks like today
Variants
,Union
,Result
,Option
typesRoughly speaking, the major suggestion for improving the ergonomics of the Go bindings lies in the
Result
,Option
and generallyVariant
types. Since there is no variant type in Go, what I've used to represent variants is examplified in this gist. Every field in a variant has a corresponding constKind
variable in generated bindings. There is a struct that represents a variant typeThere is a
kind()
function that returns the kind of the variant, and a constructor, a getter, and a setter for each of the variant field.The same applies to the
Union
types except that each field now gets a generated name starting fromF1
,F2
to ...Fn
Enum
type is the same except that enum doesn't have a type in each field. So in this case it's much simplier - each field only has a constructor, but no getter and setter as there is no value stored in each field.Although
Result
andOption
types are just special cases of theVariant
type, I recognized that they are used the most frequently in WITs. So I've added a special support for them.Whenever the Go bindgen sees a
Result
orOption
type in a world, it will generate one special Go source code file named<world>_types.go
. This file contains the definition of theResult
andOption
types and utility functions for working with them.For example, the following is the Result type in Go, and for simplicity, I've omitted the implementations here.
But working with these types tend to create frustrations as most Go functions return more idiomatic
value, err
instead of theResult
orOption
types. Developers have to write a lot of boilerplate code to convert between the two. Below is an example of a much more used pattern in Go grabbed from here:So the question is - how do we generate the option and result types to follow the above pattern?
Naming of the Types / Functions
Another annoying aspect of working with the Go bindings is to deal with long and repeatitive function and type names. Here is an example.
Given the following WIT
The name of the function
my-func
in the Go bindings will beMynamspaceMypackageAMyFunc
, which uses the pattern<namespace><package><interface><func-name>
to generate the name.The name of the type
v1
will beMynamspaceMypackageAV1
which follows the same pattern.Tuples
The naming of the tuple types is a bit different. Given the following WIT
The generated tuple type name is huge and extremely unreadable.
It follows the pattern
<namespace><package><interface>Tuple<num-of-fields><type-of-first-field><type-of-second-field><...>
Package
The generator is not able to generate a Go package for each worlds. Instead, it generates bindings per world. At the top of the file, it names the generated package as
package <world-name>
.What can we do to improve the ergonomics?
The text was updated successfully, but these errors were encountered: