[xkbcommon] Use an integer type for modifiers bit mask.

Daniel Stone daniel at fooishbar.org
Fri Oct 4 05:34:42 PDT 2013


On 4 October 2013 13:09, Wander Lairson Costa <wander.lairson at gmail.com> wrote:
> The issued raised when I took code from window.c in the weston clients:
>         mask = xkb_state_serialize_mods(input->xkb.state,
>                                         XKB_STATE_DEPRESSED |
>                                         XKB_STATE_LATCHED);
> g++ gave me a build error because I was passing an integer to enum
> parameter. C++ is a bit more nit-picky than C regarding implicit
> conversions. Therefore I had to use a cast.

One of so very many reasons I hate C++.  But, unfortunately in the
real world, we have to support people building with C++, so ...

>> With the ABI that GCC uses, it's always at least 4 bytes.
> Personally, I don't like enum's in the ABI at all, because according
> to C/C++ standards, the compiler is free to choose whatever type it
> likes, and indeed I had problems with that in the past. C++11 fixed
> that [1]. But nevermind.

Well sure, but we took the added benefit of type safety (at least with
C compilers) over the the small potential downside.  I was fully aware
of the technical point there, but don't see it as much of a risk at

>> The only thing is that you need to cast it from integer back to the enum type.
> That's what the patch is about: avoid casts. Whenever you use a cast,
> you are giving up the help the compiler may give to you regarding
> invalid type conversions. So, I always use the rule of thumb of
> avoiding casts whenever I can. IMO, this is not a harmful patch and
> will make the C++ programmers a little bit easier.

I can see where it's going, but on the other hand xkb_mod_mask_t is
_so_ not the right type.  That's a bitmask of actual modifiers (i.e.
the return value), not a bitmask of components.  So it avoids the
cast, but also makes the API look extremely misleading, as we've
documented exactly what xkb_mod_mask_t is.

I really like the type safety aspect, so to be honest I'm just
inclined to make C++ wear the cast for now.


More information about the wayland-devel mailing list