[PATCH wayland] Add "enum" attribute to "arg" elements

[Pekka Paalanen]

On Sat, 30 Aug 2014 17:20:45 +0200
"Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:

> There are programming languages, that are more strongly typed than
> C. People creating Wayland bindings for these languages probably want
> to make use of this strong type system by declaring each enumeration a
> distict type.
> 
> For code generation to work with these languages, there needs to be
> information about which enumerations may be passed/received in which
> request/event arguments.
> 
> This is accomplished by adding "enum" attributes to the "arg" elements
> of requests and events. The values of these attributes have the format
> "$interface.$enum", where "$interface" is the name of the interface,
> where the enumeration is declared and "$enum" is the name of the
> enumeration, which is used in this argument.
> 
> This might also be of documentary value.

A nice idea, but did you check that all the things you tagged actually
are enumerations and not bitfields?

Everything named "flags" is usually a bitfield, which means you can
'or' different enum values together to form new legal values that are
not listed in the enum. This will be a problem for languages which are
strict with enums, and may even differentiate vs. bitfields.

Another problem with this patch is that while it adds new syntax to the
protocol XML, it does not add anything that would either explain nor
validate/enforce it. (We do not actually use the DTD for anything
anymore.)

Yes, we do not have a document describing the XML format, and that is a
problem. Would be nice to start one, if anyone can work with Publican.

The very least, wayland-scanner should be reading the enum attribute
and check that the referenced enum exists. I'm not sure if it can
generate a nice doc snippet into the generated code, but if there is a
way to include that, it would be useful. We need *something* in the
Wayland repository actually using these new attributes, so that they do
not bit-rot immediately.

If you do this addition because of strictly types languages, you'll
probably also need the concept of "bitfield" in addition to "enum", and
use it where appropriate.

I'd also like to punt this to the 1.7 development cycle, it is a bit
late for 1.6.


Thanks,
pq

> 
> Signed-off-by: Nils Chr. Brause <nilschrbrause at googlemail.com>
> ---
>  protocol/wayland.dtd |  1 +
>  protocol/wayland.xml | 32 ++++++++++++++++----------------
>  2 files changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
> index b8b1573..1d8b9c7 100644
> --- a/protocol/wayland.dtd
> +++ b/protocol/wayland.dtd
> @@ -25,5 +25,6 @@
>    <!ATTLIST arg summary CDATA #IMPLIED>
>    <!ATTLIST arg interface CDATA #IMPLIED>
>    <!ATTLIST arg allow-null CDATA #IMPLIED>
> +  <!ATTLIST arg enum CDATA #IMPLIED>
>  <!ELEMENT description (#PCDATA)>
>    <!ATTLIST description summary CDATA #REQUIRED>
> diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> index bb457bc..a632d2d 100644
> --- a/protocol/wayland.xml
> +++ b/protocol/wayland.xml
> @@ -229,7 +229,7 @@
>        <arg name="width" type="int"/>
>        <arg name="height" type="int"/>
>        <arg name="stride" type="int"/>
> -      <arg name="format" type="uint"/>
> +      <arg name="format" type="uint" enum="wl_shm.format"/>
>      </request>
>  
>      <request name="destroy" type="destructor">
> @@ -367,7 +367,7 @@
>  	can be used for buffers. Known formats include
>  	argb8888 and xrgb8888.
>        </description>
> -      <arg name="format" type="uint"/>
> +      <arg name="format" type="uint" enum="wl_shm.format"/>
>      </event>
>    </interface>
>  
> @@ -751,7 +751,7 @@
>        </description>
>        <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat whose pointer is used"/>
>        <arg name="serial" type="uint" summary="serial of the implicit grab on the pointer"/>
> -      <arg name="edges" type="uint" summary="which edge or corner is being dragged"/>
> +      <arg name="edges" type="uint" summary="which edge or corner is being dragged" enum="wl_shell_surface.resize"/>
>      </request>
>  
>      <request name="set_toplevel">
> @@ -784,7 +784,7 @@
>        <arg name="parent" type="object" interface="wl_surface"/>
>        <arg name="x" type="int"/>
>        <arg name="y" type="int"/>
> -      <arg name="flags" type="uint"/>
> +      <arg name="flags" type="uint" enum="wl_shell_surface.transient"/>
>      </request>
>  
>      <enum name="fullscreen_method">
> @@ -835,7 +835,7 @@
>  	with the dimensions for the output on which the surface will
>  	be made fullscreen.
>        </description>
> -      <arg name="method" type="uint"/>
> +      <arg name="method" type="uint" enum="wl_shell_surface.fullscreen_method"/>
>        <arg name="framerate" type="uint"/>
>        <arg name="output" type="object" interface="wl_output" allow-null="true"/>
>      </request>
> @@ -868,7 +868,7 @@
>        <arg name="parent" type="object" interface="wl_surface"/>
>        <arg name="x" type="int"/>
>        <arg name="y" type="int"/>
> -      <arg name="flags" type="uint"/>
> +      <arg name="flags" type="uint" enum="wl_shell_surface.transient"/>
>      </request>
>  
>      <request name="set_maximized">
> @@ -949,7 +949,7 @@
>  	in surface local coordinates.
>        </description>
>  
> -      <arg name="edges" type="uint"/>
> +      <arg name="edges" type="uint" enum="wl_shell_surface.resize"/>
>        <arg name="width" type="int"/>
>        <arg name="height" type="int"/>
>      </event>
> @@ -1242,7 +1242,7 @@
>  	wl_output.transform enum the invalid_transform protocol error
>  	is raised.
>        </description>
> -      <arg name="transform" type="int"/>
> +      <arg name="transform" type="int" enum="wl_output.transform"/>
>      </request>
>  
>      <!-- Version 3 additions -->
> @@ -1301,7 +1301,7 @@
>  	keyboard or touch capabilities.  The argument is a capability
>  	enum containing the complete set of capabilities this seat has.
>        </description>
> -      <arg name="capabilities" type="uint"/>
> +      <arg name="capabilities" type="uint" enum="wl_seat.capability"/>
>      </event>
>  
>      <request name="get_pointer">
> @@ -1461,7 +1461,7 @@
>        <arg name="serial" type="uint"/>
>        <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>        <arg name="button" type="uint"/>
> -      <arg name="state" type="uint"/>
> +      <arg name="state" type="uint" enum="wl_pointer.button_state"/>
>      </event>
>  
>      <enum name="axis">
> @@ -1493,7 +1493,7 @@
>        </description>
>  
>        <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
> -      <arg name="axis" type="uint"/>
> +      <arg name="axis" type="uint" enum="wl_pointer.axis"/>
>        <arg name="value" type="fixed"/>
>      </event>
>  
> @@ -1527,7 +1527,7 @@
>  	This event provides a file descriptor to the client which can be
>  	memory-mapped to provide a keyboard mapping description.
>        </description>
> -      <arg name="format" type="uint"/>
> +      <arg name="format" type="uint" enum="wl_keyboard.keymap_format"/>
>        <arg name="fd" type="fd"/>
>        <arg name="size" type="uint"/>
>      </event>
> @@ -1572,7 +1572,7 @@
>        <arg name="serial" type="uint"/>
>        <arg name="time" type="uint" summary="timestamp with millisecond granularity"/>
>        <arg name="key" type="uint"/>
> -      <arg name="state" type="uint"/>
> +      <arg name="state" type="uint" enum="wl_keyboard.key_state"/>
>      </event>
>  
>      <event name="modifiers">
> @@ -1753,13 +1753,13 @@
>  	   summary="width in millimeters of the output"/>
>        <arg name="physical_height" type="int"
>  	   summary="height in millimeters of the output"/>
> -      <arg name="subpixel" type="int"
> +      <arg name="subpixel" type="int" enum="wl_output.subpixel"
>  	   summary="subpixel orientation of the output"/>
>        <arg name="make" type="string"
>  	   summary="textual description of the manufacturer"/>
>        <arg name="model" type="string"
>  	   summary="textual description of the model"/>
> -      <arg name="transform" type="int"
> +      <arg name="transform" type="int" enum="wl_output.transform"
>  	   summary="transform that maps framebuffer to output"/>
>      </event>
>  
> @@ -1790,7 +1790,7 @@
>          the output may be scaled, as described in wl_output.scale,
>          or transformed , as described in wl_output.transform.
>        </description>
> -      <arg name="flags" type="uint" summary="bitfield of mode flags"/>
> +      <arg name="flags" type="uint" summary="bitfield of mode flags" enum="wl_output.mode"/>
>        <arg name="width" type="int" summary="width of the mode in hardware units"/>
>        <arg name="height" type="int" summary="height of the mode in hardware units"/>
>        <arg name="refresh" type="int" summary="vertical refresh rate in mHz"/>