Grrr, I just realized that my emails are bouncing… Didn't happen before, I guess this is because my smtp server is unhappy with the many people in CC. Resending using another address (and, while at it, redacted two messages to combine them in one and added more details). My deep apologies for that, really sorry. Also, maybe the bouncing messages will finally find their way out? In that case, please just ignore them, and sorry for the noise. On Fri. 6 Dec. 2024 at 15:14, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Thu, 5 Dec 2024 at 18:26, David Laight <David.Laight@xxxxxxxxxx> wrote: > > > > From: Vincent Mailhol > > > ACK. Would adding a suggested--by Linus tag solve your concern? > > I'm genberally the one person who doesn't need any more credit ;) > > > I actually suspect the first patches to change __is_constexpr() to > > use _Generic were from myself. > > Yes. And David was also I think the one who suggested something else > than "!!" originally too. Ack. Then, I will add a suggested-by tag to credit David! > I may have liked "!!" for being very idiomatic and traditional C, but > there were those pesky compilers that warn about "integer in bool > context" or whatever the annoying warning was when then doing the > "multiply by zero" to turn a constant expression into a constant zero > expression. > > So that > > #define is_const(x) __is_const_zero(0 * (x)) > > causes issues when 'x' is not an integer expression (think > "is_const(NULL)" or "is_const(1 == 2)". But 1 == 2 already has an integer type as proven by: #define is_int(x) _Generic(x, int: 1, default: 0) static_assert(is_int(1 == 2)); And regardless, __is_const_zero() performs a (long) cast, so __is_const_zero() works for any other scalar types. Even the u128 works because after the (0 * (x)) multiplication, you are left with zero, preventing any loss of precision when casting to (long). Here are some tests to prove my claims: https://godbolt.org/z/beGs841sz So, it leaves us with the is_const(pointer) case. To which I would question if we really want to support this. By definition, an expression with a pointer type can not be an *integer* constant expression. So one part of me tells me that it is a sane thing to *not* support this case and throw a warning if the user feeds is_const() with a pointer. By the way, currently, __is_constexpr(NULL) returns false: https://godbolt.org/z/f5P9oP9n5 and the reason this went unnotified in the kernel is because no one is using it that way. So instead of complicating the macro for a use case which currently does not exist (and which goes against the definition of an ICE in the C standard), why not say instead that this is not supported by just leaving the constraint violation in is_const()? > Side note: I think "(x) == 0" will make sparse unhappy when 'x' is a > pointer, because it results that horrid "use integer zero as NULL > without a cast" thing when the plain zero gets implicitly cast to a > pointer. Which is a really nasty and broken C pattern and should never > have been silent. > > I think David suggested using ((x)?0:0) at some point. Silly > nonsensical and complex expression, but maybe that finally gets rid of > all the warnings: > > #define is_const(x) __is_const_zero((x)?0:0) > > might work regardless of the type of 'x'. If we *really* want to go beyond the C standard and extend the meaning of integer constant expressions in the kernel to also include constant pointers, then what about: #define is_const(x) __is_const_zero((x) != (x)) As established above, comparaisons return an int, thus making this pass all tests: https://godbolt.org/z/f5zrzf44h And it is slightly more pretty than the ((x)?0:0). (...) Side question, Linus, what do you think about the __is_const_zero() documentation: https://lore.kernel.org/all/20241203-is_constexpr-refactor-v1-2-4e4cbaecc216@xxxxxxxxxx/ Do you think I am too verbose as pointed out by David? Some people (including me and Yuri) like it that way. But if you also think this is too much, I will make it shorter. Thanks, Yours sincerely, Vincent Mailhol
