Re: Invalid _Static_assert expanded from HASH_CALLBACKS_CHECK

[Tim Deegan <tim@xxxxxxx>]

Hi,

At 10:58 +0200 on 25 May (1621940330), Jan Beulich wrote:
> On 24.05.2021 06:29, Roberto Bagnara wrote:
> > I stumbled upon parsing errors due to invalid uses of
> > _Static_assert expanded from HASH_CALLBACKS_CHECK where
> > the tested expression is not constant, as mandated by
> > the C standard.
> > 
> > Judging from the following comment, there is partial awareness
> > of the fact this is an issue:
> > 
> > #ifndef __clang__ /* At least some versions dislike some of the uses. */
> > #define HASH_CALLBACKS_CHECK(mask) \
> >      BUILD_BUG_ON((mask) > (1U << ARRAY_SIZE(callbacks)) - 1)
> > 
> > Indeed, this is not a fault of Clang: the point is that some
> > of the expansions of this macro are not C.  Moreover,
> > the fact that GCC sometimes accepts them is not
> > something we can rely upon:

Well, that is unfortunate - especially since the older ad-hoc
compile-time assertion macros handled this kind of thing pretty well.
Why when I were a lad &c &c. :)

> > Finally, I think this can be easily avoided: instead
> > of initializing a static const with a constant expression
> > and then static-asserting the static const, just static-assert
> > the constant initializer.
> 
> Well, yes, but the whole point of constructs like
> 
>     HASH_CALLBACKS_CHECK(callback_mask);
>     hash_domain_foreach(d, callback_mask, callbacks, gmfn);
> 
> is to make very obvious that the checked mask and the used mask
> match. Hence if anything I'd see us eliminate the static const
> callback_mask variables altogether.

That seems like a good approach.

Cheers,

Tim.