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

[Pekka Paalanen]

On Wed, 3 Sep 2014 15:03:55 +0200
"Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:

> Another possibility would of course be to add an 'is_bitfield="true"'
> attribute to enum elements that are actually bit fields instead of using
> 'bitfield' elements. Every sanely written scanner should be able to cope
> with that, shouldn't it?

I believe so, yes.

Btw. the enum size issue could bite you years from now, if you use enum
in a public library API. If the enum happens to be 16-bit now, then
someone adds a value that does not fit in 16 bits and the enum grows
implicitly to 32 bits, that will break your library ABI in a very
surprising way. That someone may not even know about your library, as
he might be editing just some XML file in another project.

If you can force enums to be always (at least these particular enums)
32 bits, you should be safe even when using them in public API. I'm
still not sure how good idea that is, though.


Thanks,
pq

> On Wed, Sep 3, 2014 at 1:09 PM, Nils Chr. Brause <nilschrbrause at gmail.com>
> wrote:
> 
> > I see.
> >
> > In that case, I'll have to maintain my own xml file for my C++ bindings.
> >
> > Thanks,
> > Nils
> >
> >
> >
> > On Wed, Sep 3, 2014 at 10:20 AM, Pekka Paalanen <ppaalanen at gmail.com>
> > wrote:
> >
> >> On Wed, 3 Sep 2014 10:28:30 +0300
> >> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> >>
> >> > 2014-09-03 10:15 GMT+03:00 Pekka Paalanen <ppaalanen at gmail.com>:
> >> > > On Mon, 1 Sep 2014 22:07:44 +0200
> >> > > "Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:
> >> > >
> >> > >> On Mon, Sep 1, 2014 at 10:45 AM, Pekka Paalanen <ppaalanen at gmail.com>
> >> wrote:
> >> > >
> >> > >> > 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.
> >> > >>
> >> > >> I will look into the scanner code then. That is probably the easiest
> >> > >> possibility
> >> > >> to prevent bit-rot, since I never did anything with Publican.
> >> > >>
> >> > >> While I'm at that, I would also like to make use of the enum names
> >> in the
> >> > >> generated C code. As far as I can see, this would not break any
> >> existing
> >> > >> code, would it? Also, that would require the enum definitions to be
> >> at the
> >> > >> top
> >> > >> of the header files (just below the struct forward declarations),
> >> because
> >> > >> enums cannot be forward declared. Would that be acceptable?
> >> > >
> >> > > Perhaps, if it does not cause problems on C++. I'm not sure, but I
> >> > > recall C++ being more strict than C wrt. enums.
> >> >
> >> > It is, in the sense that it doesn't automatically cast int to enums,
> >> > so that could break the API. Also, wouldn't it possibily be an ABI
> >> > break? Afaik enums don't have a very defined size, so what is now a
> >> > int32 could become e.g. a 16 bits enum field.
> >>
> >> Yeah, I think that is another valid concern.
> >>
> >>
> >> Thanks,
> >> pq
> >>
> >
> >