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

Go: Improve the ergonomics of Go bindings #612

Open
Tracked by #499
Mossaka opened this issue Jul 12, 2023 · 34 comments
Open
Tracked by #499

Go: Improve the ergonomics of Go bindings #612

Mossaka opened this issue Jul 12, 2023 · 34 comments
Labels
gen-tinygo Related to the TinyGo bindings generator

Comments

@Mossaka
Copy link
Member

Mossaka commented Jul 12, 2023

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 types

Roughly speaking, the major suggestion for improving the ergonomics of the Go bindings lies in the Result, Option and generally Variant 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 const Kind variable in generated bindings. There is a struct that represents a variant type

variant c1 {
    a(s32),
    b(s64),
    c(string)
}
type FooFooAV1Kind int

const (
FooFooAV1KindA FooFooAV1Kind = iota
FooFooAV1KindB
FooFooAV1KindC
)

type FooFooAC1 struct {
  kind FooFooAC1Kind
  val any
}

There 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 from F1, F2 to ... Fn

union u1 {
    s32, 
    u64, 
    string
}
const (
FooFooAU1KindF0 FooFooAU1Kind = iota
FooFooAU1KindF1
FooFooAU1KindF2
)

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 and Option types are just special cases of the Variant 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 or Option type in a world, it will generate one special Go source code file named <world>_types.go. This file contains the definition of the Result and Option 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.

type ResultKind int

const (
Ok ResultKind = iota
Err
)

type Result[T any, E any] struct {
  Kind ResultKind
  Val  T
  Err  E
}

func (r Result[T, E]) IsOk() bool {}
func (r Result[T, E]) IsErr() bool {}
func (r Result[T, E]) Unwrap() T {}
func (r Result[T, E]) UnwrapErr() E {}
func (r *Result[T, E]) Set(val T) T {}
func (r *Result[T, E]) SetErr(err E) E {}

But working with these types tend to create frustrations as most Go functions return more idiomatic value, err instead of the Result or Option 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:

token, err := scanner.Scan()
if err != nil {
    return err // or maybe break
}
// process token

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

package mynamspace:mypackage

interface a {
    variant v1 {
        a(s32),
        b(s64),
        c(string)
    }

    my-func: func(arg1: v1) -> option<s32>
}

world b {
    import a
}

The name of the function my-func in the Go bindings will be MynamspaceMypackageAMyFunc, which uses the pattern <namespace><package><interface><func-name> to generate the name.

The name of the type v1 will be MynamspaceMypackageAV1 which follows the same pattern.

Tuples

The naming of the tuple types is a bit different. Given the following WIT

package mynamspace:mypackage

interface a {
    variant v1 {
        a(s32),
        b(s64),
        c(string)
    }

    my-func: func(arg1: tuple<string, u32, u64, v1>) -> option<s32>
}

The generated tuple type name is huge and extremely unreadable.

type MynamspaceMypackageATuple4StringU32U64MynamspaceMypackageAV1T struct {
  F0 string
  F1 uint32
  F2 uint64
  F3 MynamspaceMypackageAV1
}

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?

@Mossaka Mossaka mentioned this issue Jul 12, 2023
11 tasks
@Mossaka Mossaka added the gen-tinygo Related to the TinyGo bindings generator label Jul 12, 2023
@Mossaka Mossaka changed the title Go: Improve the ergonomics of Go bindings, including splitting interfaces to different go packages, and the variant, result and option's generated APIs. Go: Improve the ergonomics of Go bindings Jul 12, 2023
@ricochet
Copy link

The biggest improvement that I would love to see is the elimination of CGO and c bindings.

@Mossaka
Copy link
Member Author

Mossaka commented Jul 12, 2023

@ricochet I created an issue specifically for that! #614

I think once //go:wasmexport is supported in the Go compiler, we can get rid of the CGO and C bindings. Note that //go:wasmimport has already landed in Go 1.21.

@lann
Copy link
Contributor

lann commented Jul 13, 2023

variant

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 structs instead of a "new types" for variants, with a tradeoff between verbosity and type inference:

type C1A struct {
  Value int32
}

c1a := C1A{1}   // less obvious construction...
i := c1a.Value  // ...but you don't have to name the `int32`

@lann
Copy link
Contributor

lann commented Jul 13, 2023

long and repetitive function and type names
<namespace><package><interface><func-name>

Is there some inherent limitation that prevents us from putting each interface (or at least package) in its own Go package?

@lann
Copy link
Contributor

lann commented Jul 13, 2023

option

Assuming we can find a way to turn fallible: func() -> result<T, E> into Fallible() (T, E), we could similarly turn optional: func() -> option<T> into Optional() (T, bool).

Another possibility for option in non-return places would be to give it a e.g. Value() *T method that could return nil for none.

@Mossaka
Copy link
Member Author

Mossaka commented Jul 13, 2023

A common representation of "sum types" in Go is a "sealed" interface:

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

@Mossaka
Copy link
Member Author

Mossaka commented Jul 13, 2023

Is there some inherent limitation that prevents us from putting each interface (or at least package) in its own Go package?

No, I don't think so. That's why I mentioned it. I believe the <namespace><package><interface> can all go to the Go package name.

@guybedford
Copy link
Contributor

@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?

@lann
Copy link
Contributor

lann commented Jul 13, 2023

do you think the "sealed" representation is better than the current one?

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 kind field.

@lann
Copy link
Contributor

lann commented Jul 13, 2023

I believe the can all go to the Go package name.

That would certainly help with verbosity. I think a nested <prefix>/<namespace>/<package>/<interface> pattern would work well. Of note here is <prefix>, which in practice probably needs to be a user-specified input to the code generation.

As an aside, it should be possible and quite nice to support e.g. import "preview-registry.bytecodealliance.org/wasi/keyvalue/v1/readwrite" as well.

@lann
Copy link
Contributor

lann commented Jul 13, 2023

special case top-level errors as being idomatic (try-catch), as distinct from other Result positions. Perhaps something similar might apply to Go?

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 result<T, E> is returning (T, E). There are a couple of challenges here:

  • You still need a representation of results that works in places other than the outer-most return type of a function, like a option<result<...>> (which we already have in wasi-http)
  • To really match the Go idiom, the error (E) type should implement the error interface, meaning it needs a method Error() string, which means we need to pick some reasonable "error-like" string representation of arbitrary types.

@guybedford
Copy link
Contributor

You still need a representation of results that works in places other than the outer-most return type of a function

In JS, we use the standard tagged error object (as per options and variants) in all the other places - { tag: 'err', val }, therefore the representation is entirely different at the top-level.

To really match the Go idiom, the error (E) type should implement the error interface

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 ComponentError class for JS (https://github.com/bytecodealliance/jco/blob/main/crates/js-component-bindgen/src/intrinsics.rs#L109), with a payload property for lifting and lowering into.

@jordan-rash
Copy link

I spent the last few days messing with the output of wit-bindgen tiny-go. @lann has pretty much summarized my feedback nicely. I was okay with the variant code gen, but I think result and option could be represented in a more idiomatic Go fashion.

Ill need to check, but does the wit grammer have a concrete error type?

Assuming something like this

interface error {
  error func() -> string
}

IMO:

wit go note
derp func() -> result< string > func derp() string
derp func() -> result<string, error> func derp() (string, error)
derp func() -> option< T > func derp() *T where nil == None and !nil == Some
derp func() -> result<option< T >, error> func derp() (*T, error)

I'm not entirely sure of the "rules" on wrapping types (option<result<option<result<option<T>>>>) but I assume there is some limitations 🤷🏼

As a personal preference, I would like to see Generics removed as much as possible in favor of interfaces as well. Something like an Optionable and Resultable interface we could satisfy

Something like type derp = option<string> could generate

Disclaimer, i dont think we need all these functions

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

@lann
Copy link
Contributor

lann commented Jul 13, 2023

I think the main issue with (not having) generics is that the result and option types can be anonymous in WIT, which means you're stuck with awful type names (like the tuple example). In fact, I wonder if we'd be better off adding generics as a way to handle tuples. They'd have to be given the somewhat ugly Tuple1, Tuple2, etc. names, but it would be better than Tuple4StringU32U64MynamspaceMypackageAV1T...

Example: https://go.dev/play/p/5U2IMy7lesx

@lann
Copy link
Contributor

lann commented Jul 14, 2023

union

It might be nicer to name the variants after the Go types they represent, e.g.

union my-union {
    s32,        // MyUnionInt32
    u64,        // MyUnionUint64
    string,     // MyUnionString
    other-type  // MyUnionOtherType
}

@Mossaka
Copy link
Member Author

Mossaka commented Jul 14, 2023

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! 🥂

@patrickhuber
Copy link

patrickhuber commented Jul 15, 2023

** 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]
Result[T] and Option[T] are implemented as unions through interfaces with private members (you don't need to carry around both fields in a composed struct)

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 defer handle.Error(&res) pattern simplifies go code and mirrors the rust question mark (?) operator, but there may be some performance impacts. More examples of this pattern can be seen here: https://github.com/patrickhuber/go-types/tree/main/examples. This package was the original inspiration and they have some benchmarks and analysis of some of the issues with this approach https://github.com/lainio/err2.

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
}

@codefromthecrypt
Copy link

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.

@codefromthecrypt
Copy link

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.

@tschneidereit
Copy link
Member

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.

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.

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.

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.

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.

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.

@codefromthecrypt
Copy link

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.

@tschneidereit
Copy link
Member

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

@codefromthecrypt
Copy link

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 Go

For 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 work

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

@codefromthecrypt
Copy link

one last thing is that tinygo is actually maintained by people who have stake in wasmtime. Literally, tinygo test uses wasmtime by default. This may not be well known, but I think it can be leveraged for a higher quality process and end product (EOF)

@tschneidereit
Copy link
Member

Thank you, I appreciate the input.

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.

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.

@codefromthecrypt
Copy link

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.

@qmuntal
Copy link

qmuntal commented Jul 18, 2023

(@Mossaka solicited my feedback, as I initially worked with him to define the current sum types codegen.

variant

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

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

Following your example, image I have this variable: var foo []C1 and I want to fill it with 100 C1A and C1B, which under the hood are just small integers. In that case, I would have to allocate 100 times to create them, there is no way around that, and allocating is a performance bottleneck in Go.

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 C1 could memory footprint could be simplified even further if all the sum type would fit in int64, any which case any wouldn't be necessary. And even more if all types could be represented as a int32, in which case num could be a int32, and so on and so forth.

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.

@lann
Copy link
Contributor

lann commented Jul 18, 2023

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.

@Mossaka
Copy link
Member Author

Mossaka commented Jul 18, 2023

I think could serve as a good example of the "sealed" interface pattern: https://github.com/patrickhuber/go-types

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

@lann
Copy link
Contributor

lann commented Jul 18, 2023

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 variant improvements vs specific result/tuple improvements.

@Mossaka
Copy link
Member Author

Mossaka commented Jul 18, 2023

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 variant improvements vs specific result/tuple improvements.

Seems like a good idea! I am happy to create an issue for each topic and link them to here.

@patrickhuber
Copy link

patrickhuber commented Jul 18, 2023

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

Glad you like it. I was thinking that a non-generic representation should be easy enough, you would just be using any instead of generic types. I felt more comfortable with the sealed interface pattern after discovering its use in the standard library https://cs.opensource.google/go/go/+/master:src/go/ast/ast.go;l=38

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 .

@Mossaka
Copy link
Member Author

Mossaka commented Jul 20, 2023

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.

  1. The biggest need is the elmination of CGO and C binidngs. I actually believe that I found a way to do it with TinyGo. please refer to Go: switch CGo implementation to component model canonical ABI in pure Go implementation #614 for more discussion on this topic.
  2. Variants: "sealed" interface proposal provides better ergonomics. The "sealed" struct + Kind approach, which the current codegen is based on, has more potential to be optmized for less memory allocations. Please refer to issue Go: ergonomics improvement for Variants type #624 for more discussion on this topic.
  3. Unions: Better to name each field after the types they represent, instead of generic F0, F1`, etc.
  4. Results and Options types have a few proposals:
    • @lann suggested an optional() function that returns a (T, bool), a seperate implementation for to-level result vs. inner-level results, and that the error E should implement the errro interface.
    • @patrickhuber provides a clean "sealed" interface for Results and Options types
    • @jordan-rash suggested an alternative generics-less proposal for Optionable and Resultable interfaces.
    • @guybedford proposed special case handling for top-level errors.
      Please refer to issue Go: ergonomics improvement for Option and Result type #625 for more discussion on this topic.
  5. Package name: certain parts of the verbose names of the types and functions could be moved to the Go package name. @lann suggested the <prefix>/<namespace>/<package>/<interface> pattern for Go package name. Please refer to issue Go: ergonomics improvement for types, function and package name #626 for more discussion on this topic.
  6. Finally, there were several recommendations centered around explicitly stating the current bindgen is based off CGO and its consequent performance implications, increasing focus on testing, and soliciting external review on the API approaches.

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.

@Mossaka
Copy link
Member Author

Mossaka commented Aug 18, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-tinygo Related to the TinyGo bindings generator
Projects
None yet
Development

No branches or pull requests

9 participants