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

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

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

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