[PATCH] tools: ocaml: Handle anonymous struct members of structs in libxl IDL

[PATCH] tools: ocaml: Handle anonymous struct members of structs in libxl IDL

[Ian Campbell]

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;
     	}
     	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@citrix.com>
Cc: Julien Grall <julien.grall@citrix.com>
Cc: Dave Scott <Dave.Scott@citrix.com>
Cc: Rob Hoes <Rob.Hoes@citrix.com>
---
 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

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

[Ian Jackson]

Ian Campbell writes ("[PATCH] tools: ocaml: Handle anonymous struct members of structs in libxl IDL"):
> Julien has a patch "arm: Allow the user to specify the GIC version"
> which adds an anonymous struct to libxl_domain_build_info:

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

for the changes to the generator.  You probably want to get an ack
from an ocaml person on the generated ocaml code.

Ian.

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

[Rob Hoes]

> On 8 Jul 2015, at 15:29, Ian Campbell <Ian.Campbell@citrix.com> 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@citrix.com>

>     	}
>     	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@citrix.com>
> Cc: Julien Grall <julien.grall@citrix.com>
> Cc: Dave Scott <Dave.Scott@citrix.com>
> Cc: Rob Hoes <Rob.Hoes@citrix.com>
> ---
> 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@lists.xen.org
http://lists.xen.org/xen-devel

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

[Dave Scott]

> On 9 Jul 2015, at 11:59, Rob Hoes <Rob.Hoes@citrix.com> wrote:
> 
>> 
>> On 8 Jul 2015, at 15:29, Ian Campbell <Ian.Campbell@citrix.com> 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@citrix.com>

This is also ok by me, so

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


> 
>>    	}
>>    	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@citrix.com>
>> Cc: Julien Grall <julien.grall@citrix.com>
>> Cc: Dave Scott <Dave.Scott@citrix.com>
>> Cc: Rob Hoes <Rob.Hoes@citrix.com>
>> ---
>> 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@lists.xen.org
http://lists.xen.org/xen-devel

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

[Ian Campbell]

On Thu, 2015-07-09 at 12:02 +0100, Dave Scott wrote:
> > 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.

Right, I kept it for consistency with the __union suffix elsewhere, but
perhaps we should consider removing both at some point.

>  So:
> > 
> > Acked-by: Rob Hoes <rob.hoes@citrix.com>
> 
> This is also ok by me, so
> 
> Acked-by: David Scott <dave.scott@citrix.com>

Applied, thanks.

Ian.


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