Re: [Xen-devel] [RFC] Generating Go bindings for libxl

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 9/4/19 1:36 AM, Nicholas Rosbrook wrote:
> George,
> 
>> Are you up for taking a stab at something like `gengotypes.py`?
> 
> I have was able to make a bit of progress on this over the weekend. I've 
> started
> `gengotypes.py` in my branch[1]; the portion which generates 
> `xenlight_types.go`
> (the counterpart to _libxl_types.h) is mostly working. 
> 
> The main exception is that I am not certain how the `KeyedUnion` type from 
> the IDL
> should be translated for Go. One option is to do something similar to the 
> `oneof` field 
> in gRPC's protobuf messages[2]. Essentially, we would define a separate 
> struct for each
> of the structs that belong to the union. Then, where a union would be used in 
> C, we use
> an interface type which the previously defined structs implement.

Yes, I think that's really the only option.  Poking around, it looks
like a lot of different people have recommended it; and the fact that
it's in use by gRPC means it can't be too terrible a solution.

The really annoying thing is that with the "interface-as-union", we
can't use anonymous types: we'll have to explicitly define the
{parent-struct} x {union-key} as a distinct type, and the is$TYPE()
method on each one.

> E.g.
> 
> type isDomainTypeStruct interface {
>         isDomainTypeStruct()
> }

The interface type itself will need to be exported, right?  (Obviously
we don't want to export the defining method.)

> type domainTypeHVMStruct struct {
>         ...
> }

So you've named the struct after the name of the key (libxl_domain_type)
and the key value (hvm); but I don't think that's sufficient.  Already
there are two different structures indexed by libxl_channel_connection:

typedef struct libxl_device_channel {
    libxl_domid backend_domid;
    char * backend_domname;
    libxl_devid devid;
    char * name;
    libxl_channel_connection connection;
    union {
        struct {
            char * path;
        } socket;
    } u;
} libxl_device_channel;

typedef struct libxl_channelinfo {
    char * backend;
    uint32_t backend_id;
    char * frontend;
    uint32_t frontend_id;
    libxl_devid devid;
    int state;
    int evtch;
    int rref;
    libxl_channel_connection connection;
    union {
        struct {
            char * path;
        } pty;
    } u;
} libxl_channelinfo;


(Note that in one, `socket` is defined, and in the other, `pty` is
defined.  I'm not sure that's not a bug, but anyway, that's what the IDL
allows.)

And there's no reason, theoretically, we couldn't have the following:

    ("u1", KeyedUnion(None, libxl_channel_connection, "connection",
           [/* One set of types */,
           ])),
    ("u2", KeyedUnion(None, libxl_channel_connection, "connection2",
           [/* A second set of types set of types */,
           ])),

So we need to include the element name as well.  But actually, I suppose
that means we don't actually need to include the type, since the element
name will be unique.

> func (d domainTypeHVMStruct) isDomainTypeStruct() {}
> 
> type DomainBuildInfo struct {
>         ...
>         Type DomainType
>         dts    isDomainTypeStruct
> }

...and then I'm afraid you'd need to have 'Dts' (should be exported,
right?) instead by the element specified by the IDL; so 'U' in all the
current cases.

This gives us:

type DomainBuildInfoU interface {
    isDomainBuildInfoU()
}

type DomainBuildInfoUHvmstruct {
  // ...
}

func (s DomainBuildInfoUHvm) isDomainBuildInfoU() { }

...

struct DomainBuildInfo {
    // ...
    Type DomainType
    U    DomainBuildInfoU
    // ...
}

Alternately, since the "key" element is also unique, we could use that
instead:

type DomainBuildInfoTypeUnion {
 // ...
}

struct DomainBuildInfo {
    // ...
    Type      DomainType
    TypeUnion DomainBuildInfoTypeUnion
    // ...
}

(And in the example given above where there are two keyed unions, one
would be "ConnectionUnion ParentTypeConnectionUnion" and
"Connection2Union ParentTypeConnection2Union".)

I think the second one looks prettier.  (Actually I think using 'u' as
the element name for the union was kind of a bad idea in the first
place.)  But that does mean we're 'overriding' the instructions of the
IDL (which prescribe both the key element name and the union element name).

What do you think?  If like me, you prefer the second one, I'll try to
ping Ian Jackson to make sure he doesn't have any objections to it.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel