[xkbcommon] Use an integer type for modifiers bit mask.
Daniel Stone
daniel at fooishbar.org
Fri Oct 4 05:34:42 PDT 2013
Hi,
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
all.
>> 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.
Cheers,
Daniel
More information about the wayland-devel
mailing list