On Mon, 9 Nov 2015 15:38:22 +0100 "Nils Chr. Brause" <nilschrbra...@gmail.com> wrote:
> 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. That is the very reason why it should not be marked as a bitfield. If you mark it as a bitfield, it encourages the use of bit-wise logic ops. The data type is still signed. That is like knowingly luring programmers into a trap. You easily avoid the pitfalls by handling it as an enum, not a bitfield. This is also the rationale behind the decision to require that bitfields must be unsigned. The convention goes both ways: bitfield must be unsigned, and signed must not be a bitfield. > This patch improves the documentation as it marks every bitfield as such > and it improves the usability of non-C language bindings. :) We can argue whether it is a bitfield or not. There is certainly no requirement to handle it as a bitfield as all possible "combinations" are easily enumerated: there are only eight, and they are all listed explicitly. Furthermore, all bits are part of the same thing: transformation. There are no bits that would not affect the thing you compute from any of the other bits. If we ever get the open/closed notation, this would be a closed enum. However, all the above argumentation pales when we look at what doing what you suggest would require: either throwing away the requirement for uint bitfields completely, or hardcoding an exception. To me, both are far far too heavy solutions to a problem that is minor at most. Thanks, pq
pgpRiPPZSfiD7.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
