Enums, bitfields and wl_arrays

Fri Oct 2 04:31:59 PDT 2015

Hi all,

let's add Jeroen, Jason, Boyan to CC.

The fundamental requirement here is that we must not break existing
code or users. All arguments I have can be traced back to this simple
statement.

First, I would rather keep the wl_array discussion in a separate thread
and not mix it up here. What we do for wl_array does not affect enums
or bitfields, but the other way around maybe.

On Thu, 1 Oct 2015 10:49:02 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> The topic of adding better enum/bitfield support to the protocol has
> come up a few[0] times[1] in the past, and again more recently[2].  We
> also have several proposed patches in patchwork, but I'm not sure they
> reflect consensus and none have Reviewed-by's on them yet [3,4,5,6,7].

This might have been first email about the topic:
http://lists.freedesktop.org/archives/wayland-devel/2014-August/016988.html
http://lists.freedesktop.org/archives/wayland-devel/2014-September/thread.html#16994

I suppose since then we have agreed at least on one thing:

Adding these attributes to XML must not change the signatures generated
by wayland-scanner.

Why: these are also used in C++, which does not implicitly covert
between enums and other stuff, which would break the build.
http://lists.freedesktop.org/archives/wayland-devel/2014-September/017057.html

> From what I gather of the discussions, no one is really against this
> feature conceptually, and impementationally the discussions appear to
> have moved further afield.  It feels like we're real close to having
> something that could be landed, but it's not 100% clear to me what
> exactly to land.  Since it's a protocol types change I would prefer to
> make sure it has a strong consensus before landing it.

In September 2014 iteration of the topic we (I, Jason, Boyan) basically
rejected the idea:
http://lists.freedesktop.org/archives/wayland-devel/2014-September/017457.html

Later that thread also brought up two incompatible definitions of a
"bitfield":
- a type where bit-wise logic operations make sense
- a type where all listed values are powers of two (POT), and bit-wise
  logic operations make sense

As a practical example, is wl_output::transform a bitfield or an enum?
- it is used as an int, not uint
- it lists all possible values
- the listed values are not all POT
- bit-wise operations make sense (check for flipped)

Arguably it should have been an uint, but I don't think we can change
it now.

> I know that several people have proposed patches on this - Bill, Nils
> and Auke at least.  Since there's a definite need for this, and since
> agreement appears to be not far off, I would like to get this landed
> this release.  And ideally I'd like to get this landed early in the
> release so we give plenty of time for testing.

The things that have been lacking in my opinion are specifications,
especially answering the questions listed here by Victor after our irc
discussion:
http://lists.freedesktop.org/archives/wayland-devel/2015-September/024382.html

The "do not break existing code" rule applies also proactively, not
just at the time we decide to start using the new attributes, so it is
important to define what things are under the "stable protocol"
definition and cannot change after publishing. IOW, what are we carving
in stone.

There is a paradox: if the new attributes affect codegen for some
languages in a way that will break the build (which is exactly the
reason why they are wanted: to enforce type-safety), we cannot add
these attributes to the existing protocols, most importantly
wayland.xml, because it would "break existing code". You might escape
that paradox once, by claiming that there is no "existing code", after
all the generators for these languages will be new. But this escape
hatch will only work once. When these generators get into use, we can
no longer add the attributes to stable protocols without breaking
things. Not even if the generators had a "legacy mode" that ignores the
new attributes, because they would then regress on arguments that
already had the new attributes set.

The important thing to realize is that once an XML file starts using
these new attributes, the attributes can no longer be added later into
more arguments. We need to get it right on the first go, but hey,
that's how extending already stabilized protocols goes.

A further complication here is that currently, the definition of the
XML language is literally "what wayland-scanner accepts", bugs and
oversights in wayland-scanner notwithstanding. This makes it tricky to
add XML language features that are not really used in the C bindings.

Or, would it be ok to dismiss all non-C languages as second-class
citizens, and say that we can change whatever as long as it does not
break the wire format, wayland-scanner or the bindings generated by it?
Somehow I don't think that would be an acceptable position.

This is actually a fundamental question:

Should we make any stability guarantees towards non-C bindings
generators that exceed what we need for C?

If no, then there's nothing to solve, but like I said, that would be
pretty harsh and inconsiderate.

Since most XML file authors are likely going to test only with
wayland-scanner, wayland-scanner should detect as many problems as
possible. This is why I have asked for generator patches to at least use
the new attributes somehow. Otherwise we would be breaking the non-C
generators all the time.

> Since Auke's patchset proposalis the most recent, let's take that one as
> the candidate for landing.  Gentlemen, I'd like to ask you to review
> these three patches [5,6,7] and either give your Reviewed-by's or flag
> specific improvements needed.  If you have a more conceptual
> disagreement, and don't think the patchset is landable as implemented,
> please raise that issue asap too.

I will be answering there.

Thanks,
pq

> 
> 0: http://lists.freedesktop.org/archives/wayland-devel/2015-April/021438.html
> 1: http://lists.freedesktop.org/archives/wayland-devel/2015-June/023008.html
> 2: http://lists.freedesktop.org/archives/wayland-devel/2015-September/024249.html
> 
> 3: http://patchwork.freedesktop.org/patch/47726/
> 4: http://patchwork.freedesktop.org/patch/47727/
> 5: http://patchwork.freedesktop.org/patch/53018/
> 6: http://patchwork.freedesktop.org/patch/53019/
> 7: http://patchwork.freedesktop.org/patch/53020/
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151002/b83c0443/attachment.sig>