[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [PATCH 2 of 5] tools: ocaml: move the nic_info record into a module to the field labels live in a separate namespace


  • To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
  • From: Dave Scott <Dave.Scott@xxxxxxxxxxxxx>
  • Date: Wed, 30 Mar 2011 18:18:15 +0100
  • Accept-language: en-US
  • Acceptlanguage: en-US
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 30 Mar 2011 10:19:12 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: Acvt7UFZSq0ikzgeQxe6eqI9yEwfFQBDDxYg
  • Thread-topic: [Xen-devel] [PATCH 2 of 5] tools: ocaml: move the nic_info record into a module to the field labels live in a separate namespace

Hi Ian,

> On Mon, 2011-03-28 at 13:26 +0100, Dave Scott wrote:
> > tools: ocaml: move the nic_info record into a module to the field
> > labels live in a separate namespace.
> >
> > Otherwise the redefinition of fields like "backend_domid" "devid" et
> > al make the records un-instantiable.

Ian Campbell wrote:
> I suppose it is the case that all the defined data types ought to have
> this done? Doing it for all but one of the types which clash would be
> confusing...

Hm, there is nothing worse than seeing code which has been partially rewritten 
N different ways :-) I'll modify the patch to ripple the change through the 
rest of the file to keep it consistent...

> I guess that Foo.t is an ocaml idiom, so I won't comment on it ;-)

I think this is fairly common in ocaml, e.g. in the standard library Set and 
Map modules. For defining records like these there are two fairly common 
approaches: one to do the Foo.t thing; the other to prefix each field with 
"foo_". More recent ocaml compilers have a nice syntax hack where you can omit 
all but one of the "Foo." prefixes on fields when constructing values, the rest 
get inferred like this:

let x = { Foo.x = 1; y = 2; z = 3 }

there's no direct equivalent for "foo_" so you have to write

let x = { foo_x = 1; foo_y = 2; foo_z = 3 }

Maybe it just depends what you're used to :-)

> The libxl type which this structure mirrors is called libxl_device_nic
> so perhaps to aid future autogeneratability (something I was working on
> at the hackathon) we could use module Device_nic? I'm assuming the full
> name as external modules see it would be Xl.Device_nic, if not perhaps
> Xl_device_nic?

Good idea-- Xl.Device_nic would work ok.

> I ran into some wrinkles with the autogenerated bindings since ocaml
> appears to lack a #include style syntax which allow one to easily
> inject
> a glob of generated code into the middle of an otherwise hand-written
> source code file. I got around this with:
>       sed -e '/^(\* @@LIBXL_TYPES@@ \*)$$/r_libxl_types.mli.in <
> xl.mli.in > xl.mli
> (I'm not sure if I am proud of this or not).

It does lack a general #include directive but there is one for *whole* modules 
and module interfaces:

$ ocaml
        Objective Caml version 3.11.2

# module M = struct
    let x = 1
  end
  module N = struct
    include M
    let y = 2
  end
  ;;
module M : sig val x : int end
module N : sig val x : int val y : int end

So perhaps you could put a "include Xl_autogen" in the middle of your 
hand-written bit of code?

> Does putting the datatypes into a module (or modules) ease this at all?
> It looks like we might end up with device_nic.ml, device_disk.ml etc
> for
> each generated type, which is not ideal, and probably not better than
> using sed.
> 
> Having moved the data type into a module does it also make sense to
> move
> the relevant functions too e.g. Xl.Device_nic.add : Nic_info.t -> domid
> -> unit etc?

That would make the generated ocamldoc look a bit nicer. If the plan is to 
stick to this kind of naming convention in libxl i.e. with a "struct xl_thing" 
and functions with names "xl_thing_do" then it's probably worth it.

> Unless you can use the "module Foo" syntax multiple times to extend the
> contents of the module

Although you can't extend an existing module M you can define a new module N 
which includes M and other stuff, as above. Perhaps that'll be good enough?

Cheers,
Dave

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.