[PATCH V2] wayland.xml: add "enum", "bitfield" and "is_bitfield" attributes

Jason Ekstrand jason at jlekstrand.net
Mon Sep 22 14:47:02 PDT 2014


On Mon, Sep 22, 2014 at 12:48 PM, Jasper St. Pierre <jstpierre at mecheye.net>
wrote:

>
>
> On Mon, Sep 22, 2014 at 12:27 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
>
>>
>>
>> On Mon, Sep 22, 2014 at 9:30 AM, Nils Chr. Brause <
>> nilschrbrause at gmail.com> wrote:
>>
>>> On Mon, Sep 22, 2014 at 11:21 AM, Pekka Paalanen <ppaalanen at gmail.com>
>>> wrote:
>>> >
>>> > On Wed, 3 Sep 2014 19:44:04 +0200
>>> > "Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:
>>> >
>>> > > This replaces "[PATCH wayland] Add "enum" attribute to "arg"
>>> elements".
>>> > > Previous concers have be incorporated and it is meant for 1.7.
>>> > >
>>> > > From 733fb18b163de93276f092a4be100c7e0c0c723f Mon Sep 17 00:00:00
>>> 2001
>>> > > From: "Nils Chr. Brause" <nilschrbrause at googlemail.com>
>>> > > Date: Wed, 3 Sep 2014 19:18:28 +0200
>>> > > Subject: [PATCH] wayland.xml: add "enum", "bitfield" and
>>> "is_bitfield"
>>> > >  attributes
>>> > >
>>> > > There are programming languages, that are more strongly typed than
>>> > > C. People creating Wayland bindings for these languages will want to
>>> > > make use of this strong type system by declaring each enumeration a
>>> > > distinct type and differentiating between enumerations and bit
>>> fields.
>>> > >
>>> > > For code generation to work with these languages, there needs to be
>>> > > information about which enumerations are actually bit fields and
>>> which
>>> > > enumerations or bit fields may be passed/received in which
>>> > > request/event arguments.
>>> > >
>>> > > This is accomplished by adding an "is_bitfield" attribute to "enum"
>>> > > elements, which are actually bitfields and by adding "enum" or
>>> > > "bitfield" attributes to the "arg" elements of requests and
>>> > > events. The values of the "enum" and "bitfield" attributes have the
>>> > > format "$interface.$name", where "$interface" is the name of the
>>> > > interface, where the enumeration or bit field is declared and "$name"
>>> > > is the name of the enumeration or bit field, which is used in this
>>> > > argument.
>>> > >
>>> > > The scanner has been modified to enforce those rules.
>>> > > ---
>>> > >  protocol/wayland.dtd |   3 ++
>>> > >  protocol/wayland.xml |  42 ++++++++++-----------
>>> > >  src/scanner.c        | 101
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> > >  3 files changed, 124 insertions(+), 22 deletions(-)
>>> > >
>>> > > diff --git a/protocol/wayland.dtd b/protocol/wayland.dtd
>>> > > index b8b1573..4506201 100644
>>> > > --- a/protocol/wayland.dtd
>>> > > +++ b/protocol/wayland.dtd
>>> > > @@ -14,6 +14,7 @@
>>> > >  <!ELEMENT enum (description?,entry*)>
>>> > >    <!ATTLIST enum name CDATA #REQUIRED>
>>> > >    <!ATTLIST enum since CDATA #IMPLIED>
>>> > > +  <!ATTLIST enum is_bitfield CDATA #IMPLIED>
>>> > >  <!ELEMENT entry (description?)>
>>> > >    <!ATTLIST entry name CDATA #REQUIRED>
>>> > >    <!ATTLIST entry value CDATA #REQUIRED>
>>> > > @@ -25,5 +26,7 @@
>>> > >    <!ATTLIST arg summary CDATA #IMPLIED>
>>> > >    <!ATTLIST arg interface CDATA #IMPLIED>
>>> > >    <!ATTLIST arg allow-null CDATA #IMPLIED>
>>> > > +  <!ATTLIST arg enum CDATA #IMPLIED>
>>> > > +  <!ATTLIST arg bitfield CDATA #IMPLIED>
>>> > >  <!ELEMENT description (#PCDATA)>
>>> > >    <!ATTLIST description summary CDATA #REQUIRED>
>>> > > diff --git a/protocol/wayland.xml b/protocol/wayland.xml
>>> > > index bb457bc..878c347 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>
>>> > >
>>> > > @@ -723,7 +723,7 @@
>>> > >        <arg name="serial" type="uint" summary="serial of the
>>> implicit grab on the pointer"/>
>>> > >      </request>
>>> > >
>>> > > -    <enum name="resize">
>>> > > +    <enum name="resize" is_bitfield="true">
>>> > >        <description summary="edge values for resizing">
>>> > >       These values are used to indicate which edge of a surface
>>> > >       is being dragged in a resize operation. The server may
>>> > > @@ -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" bitfield="wl_shell_surface.resize"/>
>>> > >      </request>
>>> > >
>>> > >      <request name="set_toplevel">
>>> > > @@ -762,7 +762,7 @@
>>> > >        </description>
>>> > >      </request>
>>> > >
>>> > > -    <enum name="transient">
>>> > > +    <enum name="transient" is_bitfield="true">
>>> > >        <description summary="details of transient behaviour">
>>> > >       These flags specify details of the expected behaviour
>>> > >       of transient surfaces. Used in the set_transient request.
>>> > > @@ -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"
>>> bitfield="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"
>>> bitfield="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"
>>> bitfield="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"
>>> bitfield="wl_output.transform"/>
>>> > >      </request>
>>> > >
>>> > >      <!-- Version 3 additions -->
>>> > > @@ -1285,7 +1285,7 @@
>>> > >        maintains a keyboard focus and a pointer focus.
>>> > >      </description>
>>> > >
>>> > > -    <enum name="capability">
>>> > > +    <enum name="capability" is_bitfield="true">
>>> > >        <description summary="seat capability bitmask">
>>> > >          This is a bitmask of capabilities this seat has; if a
>>> member is
>>> > >          set, then it is present on the seat.
>>> > > @@ -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"
>>> bitfield="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">
>>> > > @@ -1714,7 +1714,7 @@
>>> > >        <entry name="vertical_bgr" value="5"/>
>>> > >      </enum>
>>> > >
>>> > > -    <enum name="transform">
>>> > > +    <enum name="transform" is_bitfield="true">
>>> > >        <description summary="transform from framebuffer to output">
>>> > >       This describes the transform that a compositor will apply to a
>>> > >       surface to compensate for the rotation or mirroring of an
>>> > > @@ -1753,17 +1753,17 @@
>>> > >          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"
>>> bitfield="wl_output.transform"
>>> > >          summary="transform that maps framebuffer to output"/>
>>> > >      </event>
>>> > >
>>> > > -    <enum name="mode">
>>> > > +    <enum name="mode" is_bitfield="true">
>>> > >        <description summary="mode information">
>>> > >       These flags describe properties of an output mode.
>>> > >       They are used in the flags bitfield of the mode 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" bitfield="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"/>
>>> > > diff --git a/src/scanner.c b/src/scanner.c
>>> > > index 72fd3e8..b240790 100644
>>> > > --- a/src/scanner.c
>>> > > +++ b/src/scanner.c
>>> > > @@ -112,6 +112,8 @@ struct arg {
>>> > >       enum arg_type type;
>>> > >       int nullable;
>>> > >       char *interface_name;
>>> > > +     char *enum_name;
>>> > > +     char *bitfield_name;
>>> > >       struct wl_list link;
>>> > >       char *summary;
>>> > >  };
>>> > > @@ -119,6 +121,7 @@ struct arg {
>>> > >  struct enumeration {
>>> > >       char *name;
>>> > >       char *uppercase_name;
>>> > > +     int is_bitfield;
>>> > >       struct wl_list entry_list;
>>> > >       struct wl_list link;
>>> > >       struct description *description;
>>> > > @@ -314,7 +317,7 @@ start_element(void *data, const char
>>> *element_name, const char **atts)
>>> > >       struct entry *entry;
>>> > >       struct description *description;
>>> > >       const char *name, *type, *interface_name, *value, *summary,
>>> *since;
>>> > > -     const char *allow_null;
>>> > > +     const char *allow_null, *enum_name, *bitfield_name,
>>> *is_bitfield;
>>> > >       char *end;
>>> > >       int i, version;
>>> > >
>>> > > @@ -323,11 +326,14 @@ start_element(void *data, const char
>>> *element_name, const char **atts)
>>> > >       type = NULL;
>>> > >       version = 0;
>>> > >       interface_name = NULL;
>>> > > +     enum_name = NULL;
>>> > > +     bitfield_name = NULL;
>>> > >       value = NULL;
>>> > >       summary = NULL;
>>> > >       description = NULL;
>>> > >       since = NULL;
>>> > >       allow_null = NULL;
>>> > > +     is_bitfield = NULL;
>>> > >       for (i = 0; atts[i]; i += 2) {
>>> > >               if (strcmp(atts[i], "name") == 0)
>>> > >                       name = atts[i + 1];
>>> > > @@ -339,12 +345,18 @@ start_element(void *data, const char
>>> *element_name, const char **atts)
>>> > >                       value = atts[i + 1];
>>> > >               if (strcmp(atts[i], "interface") == 0)
>>> > >                       interface_name = atts[i + 1];
>>> > > +             if (strcmp(atts[i], "enum") == 0)
>>> > > +                     enum_name = atts[i + 1];
>>> > > +             if (strcmp(atts[i], "bitfield") == 0)
>>> > > +                     bitfield_name = atts[i + 1];
>>> > >               if (strcmp(atts[i], "summary") == 0)
>>> > >                       summary = atts[i + 1];
>>> > >               if (strcmp(atts[i], "since") == 0)
>>> > >                       since = atts[i + 1];
>>> > >               if (strcmp(atts[i], "allow-null") == 0)
>>> > >                       allow_null = atts[i + 1];
>>> > > +             if (strcmp(atts[i], "is_bitfield") == 0)
>>> > > +                     is_bitfield = atts[i + 1];
>>> > >       }
>>> > >
>>> > >       ctx->character_data_length = 0;
>>> > > @@ -461,9 +473,31 @@ start_element(void *data, const char
>>> *element_name, const char **atts)
>>> > >                       else
>>> > >                               arg->interface_name = NULL;
>>> > >                       break;
>>> > > +             case UNSIGNED:
>>> > > +             case INT:
>>> > > +                     if (enum_name) {
>>> > > +                             arg->enum_name = xstrdup(enum_name);
>>> > > +                             arg->bitfield_name = NULL;
>>> > > +                     }
>>> > > +                     else if (bitfield_name) {
>>> > > +                             arg->bitfield_name =
>>> xstrdup(bitfield_name);
>>> > > +                             arg->enum_name = NULL;
>>> > > +                     }
>>> > > +                     else {
>>> > > +                             arg->bitfield_name = NULL;
>>> > > +                             arg->enum_name = NULL;
>>> > > +                     }
>>> > > +
>>> > > +                     if (enum_name && bitfield_name)
>>> > > +                             fail(&ctx->loc, "bitfield and enum
>>> attribute cannot both be set");
>>> > > +                     break;
>>> > >               default:
>>> > >                       if (interface_name != NULL)
>>> > >                               fail(&ctx->loc, "interface attribute
>>> not allowed for type %s", type);
>>> > > +                     if (enum_name != NULL)
>>> > > +                             fail(&ctx->loc, "enum attribute not
>>> allowed for type %s", type);
>>> > > +                     if (bitfield_name != NULL)
>>> > > +                             fail(&ctx->loc, "bitfield attribute
>>> not allowed for type %s", type);
>>> >
>>> > This looks broken.
>>> >
>>> > What if
>>> > - int or unsigned has "interface" attribute? It should be an error.
>>> > - new_id or object has "enum" or "bitfield" attribute? Should be an
>>> >   error.
>>> >
>>> > You probably want your own switch-clause.
>>>
>>> Wouldn't a separate switch-clause require a new type? I thought,
>>> this is not what we want, since this would break other parsers.
>>>
>>> I would probably just check for additional unwanted combinations.
>>>
>>> >
>>> > >                       break;
>>> > >               }
>>> > >
>>> > > @@ -497,6 +531,13 @@ start_element(void *data, const char
>>> *element_name, const char **atts)
>>> > >                              &enumeration->link);
>>> > >
>>> > >               ctx->enumeration = enumeration;
>>> > > +
>>> > > +             if (is_bitfield == NULL || strcmp(is_bitfield,
>>> "false") == 0)
>>> > > +                     enumeration->is_bitfield = 0;
>>> > > +             else if (strcmp(is_bitfield, "true") == 0)
>>> > > +                     enumeration->is_bitfield = 1;
>>> > > +             else
>>> > > +                     fail(&ctx->loc, "invalid value for is_bitfield
>>> attribute (%s)", is_bitfield);
>>> > >       } else if (strcmp(element_name, "entry") == 0) {
>>> > >               if (name == NULL)
>>> > >                       fail(&ctx->loc, "no entry name given");
>>> > > @@ -1246,6 +1287,60 @@ emit_code(struct protocol *protocol)
>>> > >       }
>>> > >  }
>>> > >
>>> > > +static int
>>> > > +enum_exists(struct protocol *protocol, const char *enum_name, int
>>> is_bitfield)
>>> > > +{
>>> > > +     struct interface *i;
>>> > > +     struct enumeration *e;
>>> > > +     char full_name[64];
>>> > > +
>>> > > +     wl_list_for_each(i, &protocol->interface_list, link) {
>>> > > +             wl_list_for_each(e, &i->enumeration_list, link) {
>>> > > +                     sprintf(full_name, "%s.%s", i->name, e->name);
>>> >
>>> > Use snprintf to avoid buffer overflows.
>>> > Or split enum_name, and match both halves separately.
>>>
>>> I will split enum_name then.
>>>
>>> >
>>> > > +                     if (strcmp(full_name, enum_name) == 0 &&
>>> > > +                         e->is_bitfield == is_bitfield)
>>> > > +                             return 1;
>>> > > +             }
>>> > > +     }
>>> > > +     return 0;
>>> > > +}
>>> > > +
>>> > > +static void
>>> > > +check_message(struct protocol *protocol, struct message *m)
>>> > > +{
>>> > > +     struct arg *a;
>>> > > +
>>> > > +     wl_list_for_each(a, &m->arg_list, link)
>>> > > +             if ((a->type == UNSIGNED || a->type == INT)) {
>>> > > +                     if (a->enum_name != NULL &&
>>> > > +                         !enum_exists(protocol, a->enum_name, 0)) {
>>> > > +                             fprintf(stderr, "enum %s does not
>>> exist\n",
>>> > > +                                     a->enum_name);
>>> > > +                             exit(EXIT_FAILURE);
>>> > > +                     }
>>> > > +                     else if (a->bitfield_name != NULL &&
>>> > > +                              !enum_exists(protocol,
>>> a->bitfield_name, 1)) {
>>> > > +                             fprintf(stderr, "bitfield %s does not
>>> exist\n",
>>> > > +                                     a->enum_name);
>>> > > +                             exit(EXIT_FAILURE);
>>> > > +                     }
>>> > > +             }
>>> > > +}
>>> > > +
>>> > > +static void
>>> > > +check_protocol(struct protocol *protocol)
>>> > > +{
>>> > > +     struct interface *i;
>>> > > +     struct message *m;
>>> > > +
>>> > > +     wl_list_for_each(i, &protocol->interface_list, link) {
>>> > > +             wl_list_for_each(m, &i->request_list, link)
>>> > > +                     check_message(protocol, m);
>>> > > +             wl_list_for_each(m, &i->event_list, link)
>>> > > +                     check_message(protocol, m);
>>> > > +     }
>>> > > +}
>>> > > +
>>> > >  int main(int argc, char *argv[])
>>> > >  {
>>> > >       struct parse_context ctx;
>>> > > @@ -1302,6 +1397,10 @@ int main(int argc, char *argv[])
>>> > >
>>> > >       XML_ParserFree(ctx.parser);
>>> > >
>>> > > +     /* some checks can only be performed after *
>>> > > +      * the whole protocol has been read in     */
>>> > > +     check_protocol(&protocol);
>>> > > +
>>> > >       switch (mode) {
>>> > >               case CLIENT_HEADER:
>>> > >                       emit_header(&protocol, CLIENT);
>>> >
>>> > Should we also enforce, that only UNSIGNED is valid for bitfields?
>>>
>>> I would certainly like to do this, but wl_output.transform is
>>> always passed as a signed integer and I'm not sure if this is a
>>> bitfield or not. I assumed it is, because you can
>>> reconstruct "270", "flipped_90", "flipped_180" and "flipped_270"
>>> out of "90", "180" and "flipped". Or is this just a coincidence?
>>>
>>
>> It's both... More in a couple of lines
>>
>>
>>> Also, is there a reason why some enumerations are passed as
>>> a signed integer (wl_output.subpixel) and others aren't?
>>>
>>> >
>>> > So, what do other people think of the idea in this patch?
>>>
>>
>> I'm a little unsure.  I think trying to completely solve this problem in
>> a way that will truly make strongly typed languages happy is insanity.
>> That said, I'm cautiously ok with defining bitfields and enums as long as
>> we are very careful in scoping what "bitfield" and "enum" mean.  A
>> "bitfield" should have only power of two values and the result should
>> always be interpreted as an OR of those values.  An enum should have every
>> possible value enumerated.  If anyone has a good example of something that
>> validly doesn't fit into either of these, please speak up.
>>
>
> xdg_shell.resize_edges. It is both a bitfield (top / left / right / bottom
> edges are powers of two) and an enum (top right / bottom left corner
> convenience values, with top / bottom being left out as it is undefined).
>

That's an example of an enum with cleverly chosen values so that you can do
bitfield-like things.  What I'm more concerned about is something where it
would be impractical to actually enumerate all of the possibilities in the
protocol spec but it's not a bitfield either.


>
>
>> The example of wl_output.transform is an enum because every possibility
>> is enumerated.  From C or a similar language, you can do fun stuff like "if
>> (transform & WL_OUTPUT_TRANSFORM_FLIPPED)" to determine if there is a
>> flip.  In a strongly typed language, you can't do this and we shouldn't
>> bend over backwards to make it possible.  If we try and come up with some
>> convoluted system that makes this possible with typed languages, we're
>> going to cause far more pain than it's worth.
>>
>> One other thing that we need to keep in mind here is the primary target
>> audience of Wayland and its libraries.  That audience is compositors and
>> toolkits.  Most of those are written in C and C++.  What we don't want to
>> do is to do a bunch of things for the sake of 1% of the target audience
>> that makes the rest have to bend over backwards.  When I said "cautiously
>> OK", I mean that I don't see that happening yet and I don't see a valid use
>> for an enum that doesn't follow one of those two rules.  However, if we
>> have a plausible case where doing so would make everyone's lives easier,
>> I'm going to not be a big fan.
>>
>> Please note that I'm not trying to insult Haskel or other functional or
>> strongly typed languages or the people who use them.  I'm simply trying to
>> be pragmatic and recognize that people who want to write an app in haskel
>> that manually bangs the Wayland protocol isn't the target audience.
>>
>> --Jason
>>
>> > I want to see some other Wayland developers support this, as I am not
>>> > confident with my own judgement here.
>>> >
>>> >
>>> > Thanks,
>>> > pq
>>>
>>> Cheers,
>>> Nils
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>>
>
>
> --
>   Jasper
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140922/dbe0cb33/attachment-0001.html>


More information about the wayland-devel mailing list