[PATCH] Declare enumeration wl_output.transform as bitfield.

[Pekka Paalanen]

On Mon, 9 Nov 2015 12:54:00 +0100
"Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:

> Hi,
> 
> On Mon, Nov 9, 2015 at 12:04 PM, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Fri, 6 Nov 2015 16:05:10 +0100
> > "Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:
> >
> >> Hi,
> >>
> >> On Fri, Nov 6, 2015 at 3:48 PM, Auke Booij <auke at tulcod.com> wrote:
> >> > On 6 November 2015 at 13:03, Nils Christopher Brause
> >> > <nilschrbrause at gmail.com> wrote:
> >> >> The enumeration wl_output.transform is clearly a bitfield.
> >> >> The definition of a bitfield is that each bit has a distinct
> >> >> meaning. This is clearly the case in the enumeration
> >> >> wl_output.transform:
> >> >>
> >> >> - bit 0: rotate by 90 degree
> >> >> - bit 1: rotate by 180 degree
> >> >> - bit 2: flip around vertical axis

> >> >> Therefore the bitfield="true" attribute has been added to
> >> >> the enumeration declaration. Since this bitfield is
> >> >> transferred as signed integer, the scanner had to be
> >> >> modified to accept that behaviour. This was also noted in
> >> >> the documentation.
> >> >
> >> > As I made clear in our previous discusions, I don't think that it is a
> >> > safe idea to allow signed integers to be bitfields. Requiring
> >> > unsignedness is a sanity check. So I would not like this patch, if
> >> > only for the reason that it invalidates this check.
> >>
> >> I don't see why the signedness should even matter. A bitfield doesn't
> >> have a numerical value after all. It is just a collection of bits.
> >
> > I totally agree with Auke here.
> >
> > Let's reject this patch. It's just one more historical accident we have
> > to live with.
> 
> I still got no explaination why the signdness of a bitfield that has no
> numerical meaning even matters. (Rasons like "it feels natural" aside.)
> It is totally irrelavant whether a collection of bits is marked as signed or
> as unsigned. Therefore there is no sensible reason to reject this patch.

Hi,

my motivation comes from C.

A single-bit signed bitfield can have values 0 and -1, not 1, which is
somewhat surprising. This is a small reason that promotes the
convention that all bitfields should be unsigned. Of course, this is
the C bit field feature, which is not very relevant here, except just
having the same name.

Another reason I can think of is conversions between different sized
types. Signed values usually undergo sign extension, which can be a
surprise when you are only thinking about bits.

Maybe the most relevant ones are this:

	"Right shift of a negative signed number has
	implementation-defined behaviour."

and:

	"A left shift, if the number either starts out negative, or the
	shift operation would shift a 1 either to or beyond the sign
	bit, has undefined behaviour (as do most operations on signed
	values which cause an overflow)."

- http://stackoverflow.com/questions/4009885/arithmetic-bit-shift-on-a-signed-integer

I wasn't really even aware of these two until I looked them up.

For me all this belongs in the same category as the convention to
always use signed types when you are going to do arithmetics, rather
than using unsigned types only sometimes when you know you cannot have
negative values for certain input variables but do have negatives for
others. Mixing signed and unsigned types in arithmetic is error prone,
so the convention is there to let programmers think less and also avoid
compiler-specific or undefined behaviour.


Thanks,
pq
-------------- 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/20151109/00cfd24a/attachment.sig>