Hi, On Mon, Nov 9, 2015 at 2:49 PM, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Mon, 9 Nov 2015 12:54:00 +0100 > "Nils Chr. Brause" <nilschrbra...@gmail.com> wrote: > >> Hi, >> >> On Mon, Nov 9, 2015 at 12:04 PM, Pekka Paalanen <ppaala...@gmail.com> wrote: >> > On Fri, 6 Nov 2015 16:05:10 +0100 >> > "Nils Chr. Brause" <nilschrbra...@gmail.com> wrote: >> > >> >> Hi, >> >> >> >> On Fri, Nov 6, 2015 at 3:48 PM, Auke Booij <a...@tulcod.com> wrote: >> >> > On 6 November 2015 at 13:03, Nils Christopher Brause >> >> > <nilschrbra...@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
While I can see your motivation behind having bitfields only in a unsigned integer, none of your concers are actually relevant here, because the wl_output.transform bitfield is already a signed integer. Therefore a C programmer already has to be aware of the issues you have mentioned. Also I am not changing the C API. This patch improves the documentation as it marks every bitfield as such and it improves the usability of non-C language bindings. :) Cheers, Nils _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
