Re: [Xen-devel] [PATCH] tools: ocaml: Handle anonymous struct members of structs in libxl IDL

[Dave Scott <Dave.Scott@xxxxxxxxxx>]

> On 9 Jul 2015, at 11:59, Rob Hoes <Rob.Hoes@xxxxxxxxxx> wrote:
> 
>> 
>> On 8 Jul 2015, at 15:29, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
>> 
>> Julien has a patch "arm: Allow the user to specify the GIC version"
>> which adds an anonymous struct to libxl_domain_build_info:
>> 
>>   @ -480,6 +486,11 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>                                          ])),
>>                     ("invalid", None),
>>                     ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
>>   +
>>   +
>>   +    ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
>>   +                              ])),
>>   +
>>        ], dir=DIR_IN
>>    )
>> 
>> (libxl_gic_version is an enum). This is not currently supported by the
>> ocaml genwrap.py. Add a simple pass which handles simple anonymous
>> unions as top level members of a struct type, but not more deeply
>> nested since that would be a much more complex change and is not
>> currently required.
>> 
>> With Juliens patch applied the relevant resulting change to the .mli
>> is:
>> 
>>   --- tools/ocaml/libs/xl/_libxl_BACKUP_types.mli.in 2015-07-08 
>> 11:22:35.000000000 +0100
>>   +++ tools/ocaml/libs/xl/_libxl_types.mli.in        2015-07-08 
>> 12:25:56.000000000 +0100
>>   @@ -469,6 +477,10 @@ module Domain_build_info : sig
>> 
>>      type type__union = Hvm of type_hvm | Pv of type_pv | Invalid
>> 
>>   +  type arch_arm__anon = {
>>   +          gic_version : gic_version;
>>   +  }
>>   +
>>      type t =
>>      {
>>              max_vcpus : int;
>>   @@ -510,6 +522,7 @@ module Domain_build_info : sig
>>              ramdisk : string option;
>>              device_tree : string option;
>>              xl_type : type__union;
>>   +          arch_arm : arch_arm__anon;
> 
> As said on another thread, Iâm not a fan of the __anon suffix. But I wonât 
> object either, because in practice it doesnât really matter. So:
> 
> Acked-by: Rob Hoes <rob.hoes@xxxxxxxxxx>

This is also ok by me, so

Acked-by: David Scott <dave.scott@xxxxxxxxxx>


> 
>>      }
>>      val default : ctx -> ?xl_type:domain_type -> unit -> t
>>    end
>> 
>> The .ml differs similarly. Without Julien's patch there is no change.
>> 
>> gen_struct is refactored slightly to take the indent level as an
>> argument, since it is now used at a different level.
>> 
>> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> Cc: Julien Grall <julien.grall@xxxxxxxxxx>
>> Cc: Dave Scott <Dave.Scott@xxxxxxxxxx>
>> Cc: Rob Hoes <Rob.Hoes@xxxxxxxxxx>
>> ---
>> tools/ocaml/libs/xl/genwrap.py | 39 +++++++++++++++++++++++++++++++++------
>> 1 file changed, 33 insertions(+), 6 deletions(-)
>> 
>> diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
>> index 402e489..1c8ad81 100644
>> --- a/tools/ocaml/libs/xl/genwrap.py
>> +++ b/tools/ocaml/libs/xl/genwrap.py
>> @@ -87,7 +87,10 @@ def ocaml_type_of(ty):
>>    elif isinstance(ty,idl.KeyedUnion):
>>        return ty.union_name
>>    elif isinstance(ty,idl.Aggregate):
>> -        return ty.rawname.capitalize() + ".t"
>> +        if ty.rawname is None:
>> +            return ty.anon_struct
>> +        else:
>> +            return ty.rawname.capitalize() + ".t"
>>    else:
>>        return ty.rawname
>> 
>> @@ -111,14 +114,14 @@ def ocaml_instance_of_field(f):
>>        name = f.name
>>    return "%s : %s" % (munge_name(name), ocaml_type_of(f.type))
>> 
>> -def gen_struct(ty):
>> +def gen_struct(ty, indent):
>>    s = ""
>>    for f in ty.fields:
>>        if f.type.private:
>>            continue
>>        x = ocaml_instance_of_field(f)
>> -        x = x.replace("\n", "\n\t\t")
>> -        s += "\t\t" + x + ";\n"
>> +        x = x.replace("\n", "\n"+indent)
>> +        s += indent + x + ";\n"
>>    return s
>> 
>> def gen_ocaml_keyedunions(ty, interface, indent, parent = None):
>> @@ -140,7 +143,7 @@ def gen_ocaml_keyedunions(ty, interface, indent, parent 
>> = None):
>>            if isinstance(f.type, idl.Struct) and not f.type.has_fields(): 
>> continue
>>            s += "\ntype %s_%s =\n" % (nparent,f.name)
>>            s += "{\n"
>> -            s += gen_struct(f.type)
>> +            s += gen_struct(f.type, indent + "\t")
>>            s += "}\n"
>> 
>>        name = "%s__union" % ty.keyvar.name
>> @@ -169,6 +172,23 @@ def gen_ocaml_keyedunions(ty, interface, indent, parent 
>> = None):
>>        return None, None
>>    return s.replace("\n", "\n%s" % indent), union_type
>> 
>> +def gen_ocaml_anonstruct(ty, interface, indent, parent = None):
>> +    s= ""
>> +
>> +    if ty.rawname is not None:
>> +        # Non-anonymous types need no special handling
>> +        pass
>> +    elif isinstance(ty, idl.Struct):
>> +        name = "%s__anon" % parent
>> +        s += "type %s = {\n" % name
>> +        s += gen_struct(ty, indent)
>> +        s += "}\n"
>> +        ty.anon_struct = name
>> +    if s == "":
>> +        return None
>> +    s = indent + s
>> +    return s.replace("\n", "\n%s" % indent)
>> +
>> def gen_ocaml_ml(ty, interface, indent=""):
>> 
>>    if interface:
>> @@ -212,9 +232,16 @@ def gen_ocaml_ml(ty, interface, indent=""):
>>            if union_type is not None:
>>                union_types.append(union_type)
>> 
>> +        # Handle anonymous structs...
>> +        for f in ty.fields:
>> +            anon = gen_ocaml_anonstruct(f.type, interface, "\t", f.name)
>> +            if anon is not None:
>> +                s += anon
>> +                s += "\n"
>> +
>>        s += "\ttype t =\n"
>>        s += "\t{\n"
>> -        s += gen_struct(ty)
>> +        s += gen_struct(ty, "\t\t")
>>        s += "\t}\n"
>> 
>>        if ty.init_fn is not None:
>> -- 
>> 2.1.4

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel