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

[George Dunlap <george.dunlap@citrix.com>]

On 9/12/19 6:35 PM, Nicholas Rosbrook wrote:
> I'm not strongly opposed to the struct duplication, but I do prefer the ability
> to perform type assertions as a way to determine which field is "valid."

Fair enough.

>> So the advantage of this is that you can just call:
>>
>>     fromC(&di, &cdi)
>>
>> Rather than:
>>
>>     di.fromC(&cdi)
>>
>> ?
>>
>> But the cost for this is that we're switching from static type-checking
>> to dynamic type-checking.  If in the first example, cdi is the wrong
>> type (for instance, if we forget the & at the front), everything will
>> compile, and we won't notice unless the function actually gets called.
>> In the second example, if we're not trying to implement a generic
>> "marshaler" method, we can define the function signature to specify
>> exactly what pointer we need.
> 
> The advantage is it simplifies the generated code's error handling. However,
> I was re-thinking this portion as well, because giving up the static type
> checking is not worth "cleaner" generated code. I'll make that change.

Ah, right the main purpose was to have the single place at top to catch
 exceptions^W^W recover from panics, not so much because one form of the
function was nicer than the other one.  Still, I think static type
checking is a big thing to give up to make generated code cleaner.  Thanks.

>> I'd be tempted to say just do something like:
>>
>> type CpuidPolicyList struct {
>>     val C.libxl_cpuid_policy_list
>> };
>>
>> A part of me thinks even something like this wouldn't be terrible:
>>
>> type CpuidPolicyList C.libxl_cpuid_policy_list
>>
>> It "leaks" the internals out to the callers, but it also means you don't
>> have to do all this faff of marshalling / unmarshalling what's
>> essentially just a pointer.
> 
> I don't think it's a good idea to expose the C type. Besides the fact that [2]
> explicitly states not to do this, exporting this type gives the false idea that
> this type is somehow portable.

Ack.

>> It sort of looks like this is an entirely internal thing that libxl
>> uses.  I think to begin with we can just declare this as an empty
>> struct, and figure out what to put in it once it becomes more clear how
>> it needs to be used.
> 
> Okay, will do.

FWIW checked with Ian after I wrote this mail, and he confirmed that
that field (`link` in `libxl_event`) was only meant to be used
internally, and ideally we wouldn't even have that available in the Go
version of the struct (since it's not actually part of the public
interface).

Unfortunately we can't actually get rid of the element it without
special-casing it (which I don't think is a good idea), or adding a new
"PRIVATE()" annotation to the IDL or something (which would be nice to
have, but not something I expect anyone to have much time to do).  For
now, I think defining it as an empty struct will be good enough.

Great, thanks!  Look forward to the next iteration!

 -George

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