RE: [PATCH 02/10] compiler.h: add is_const() as a replacement of __is_constexpr()

[David Laight <David.Laight@ACULAB.COM>]

From: Vincent Mailhol
> Sent: 07 December 2024 07:43
...
> > So maybe the slightly long lines:
> > #define const_true(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 1L) : (char *)0, char *: 1, void *: 0)
> > #define const_expr(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 0L) : (char *)0, char *: 1, void *: 0)

Clearly they can be implemented in terms of a common define.
But I don't see a need for a const_zero() and nested expansions make extra
work for the compiler.

> 
> This still throws a -Wnull-pointer-arithmetic on clang on const_expr(NULL):
>   https://godbolt.org/z/vo5W7efdE

I was worried about that one.

> I just do not see a method to silence that one. So three options:
> 
>   1. is_const() does not accept pointers and throws a constraint violation:
>        #define is_const(x) __is_const_zero(0 * (x))
>      This is my current patch.

Is that going to affect things like const_true(x << y)?
Disallowing that seems counter-productive.
(Remember it might be passed into a #define that is then
checking its argument for being constant.)

>   2. is_const() accept pointers but is_const(NULL) returns false:
>        #define is_const(x) __is_const_zero((x) != (x))
>      This keeps the current __is_constexpr() behaviour.

No good - expands everything twice.

>   3. is_const() accepts pointers and is_const(NULL) return true:
> 
>        #define const_expr(x) _Generic(0 ? (void *)((x) + 0 ? 0L : 0L)
> : (char *)0, char *: 1, void *: 0)
> 
>      David's latest proposal, it requires to remove the
>      -Wnull-pointer-arithmetic clang warning.

Only for const_expr(NULL) - and since clang gets that wrong
maybe the warning is a good thing.

You can just add:
#define const_NULL(ptr) const_true(!(ptr))
Probably the only place where you actually want to test for zero.

> 
> I vote for 1. or 2. (with a preference for 1.). IMHO, we are just
> adding an unreasonable level of complexity for making the macro treat
> NULL as an integer. Would someone find a solution for 3. that does not
> yield a warning, then why not. But if we have to remove a compiler
> check for a theoretical use case that does not even exist in the
> kernel, then it is not worth the trade off.
> 
> Concerning is_const(var << 2), the patch I submitted works fine as-is
> with all scalars including that (var << 2):
> 
>   https://godbolt.org/z/xer4aMees
> 
> And can we ignore the case (!(var << 2))? This is not a warning
> because of the macro, but because of the caller! If I do any of:
> 
>           if (!(var << 2)) {}
>           (void)__builtin_constant_p(!(var << 2));
> 
> I also got the warning. The point is that the macro should not
> generate *new* warnings. If the given argument already raises a
> warning, it is the caller's responsibility to fix.

Except it could easily happen way inside some other expansion.
Perhaps someone optimises frobnicate(x) for constant input.
Suddenly frobnicate(!(var << 2)) generates a compile error.

	David

> 
> 
> Yours sincerely,
> Vincent Mailhol

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)