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

[Pekka Paalanen]

On Mon, 20 Apr 2015 15:54:59 +0000
Jeroen Bollen <jbinero at gmail.com> wrote:

> On Mon, 20 Apr 2015 at 09:03 Pekka Paalanen <ppaalanen at gmail.com> wrote:
> >
> > Also, adding the strict type information to the XML spec has no benefit
> > for C, which is the de facto language for Wayland core developers. A C
> > compiler also does not raise errors if you violate the rules. This and
> > all the above are the likely reasons why adding the strict type
> > information is not interesting, at least for me.
> >
> > Making the enum rules more strict has a possibility to break existing
> > users, but to me it is unclear if the benefit would outweigh that
> > con or the freedom.
> >
> > On the wire, an enum or bitfield is still just an uint (or int), and a
> > buggy client or server may cause the other to receive illegal values.
> > Do the strongly typed languages have checks against that? Can you
> > define what happens if the value is illegal for an enum? Or do you have
> > to write that check manually in any case?
> >
> > So, the big question here is: do we even want to have strict enum types?
> >
> 
> It does indeed not provide any benefit for C, but if C is the only language
> to be targeted, there is not much use in there being an XML specification
> at all. It could've just been written out in C. I do agree that any changes
> should be compatible with C.
> 
> I know in Rust, conversion from the `uint` to the `enum` type would have to
> be done manually, and error checking happens here. It does stop the user
> from passing illegal values to other servers however. It also makes it more
> clear what `enum`s are to be used with which arguments, and enforces this.
> 
> I don't really see why adding that to the specification is an issue. If
> some `enum` types aren't complete, as in, they aren't the only valid
> values, then I see little value in having the `enum` in the specification
> in the first place.

The foremost purpose of the <enum> is to let the generator create named
constants in a namespace. This is something that has to stay due to
API stability guarantees.

I had some discussion in IRC, and I'm coming around. It does seem a
useful addition, but you have to be clear in specifying what it all
means. For instance, saying an attribute referencing an <enum> in an
<arg> for purely documentation linkage would be fine. We don't have
docs for the XML language, but at least define it in a commit message.

If such annotation is intended to be useful for code generation on
relevant languages, you need to define clearly what it means. I'm not
familiar with those languages you are interested in, so I cannot judge
that.

What I would want, is patches to wayland-scanner to use these
additional annotations, if only for syntax checking the XML and maybe
generating one more comment for an <arg> or whatever. Unfortunately
wayland-scanner still is the definition of the Wayland XML language.
It should be kept updated in any case.

Two things I came up with in the IRC discussion was that only <arg>
types of int an uint are eligible for enums, and only uint for
bitfields. I think wayland-scanner should enforce that.

One question was about whether bitfield values should be limited to
single bits (powers of two). I think that would be an artificial
restriction, and it would not allow using short-hand names for sets of
bits. I see no reason to deny those.

Another is the question about incomplete enumerations. I don't think we
can deny their existence. Therefore they must be supported. If that
means you cannot put the enum attribute in such an <arg>, then so be
it; but you lose also the doc linkage. Extending value or bit
enumerations is something that happens, and with the expectation that
it doesn't break anything. An interface that expects to see new entries
in an enumeration in the future will specify how unknown values should
be dealt with. You maybe have your code generated according to an old
specification, while the other party is using a new specification, and
you need to obey the interface specification regarding unknown values.
This has to work.

Btw. the wayland.dtd is abandoned. The rationale was that if it catches
errors that wayland-scanner doesn't, then wayland-scanner should be
fixed. We should probably remove that file, and preferably replace it
with documentation that agrees with wayland-scanner. If only had the
time...


Thanks,
pq