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 has already an integer type as proven by: #define is_int(x) _Generic(x, int: 1, default: 0) static_assert(is_int(1 == 2)); So, it leaves us with the case is_const(pointer). 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_cont() with a pointer. If we just what to accept pointer arguments but still return false (because those are not integers), one solution is: #define is_const(x) __is_const_zero((long)(x) * 0l) This would be consistent with __is_constexpr(): it does accept NULL (i.e. no warnings), but does not recognize it as an integer constant expression, e.g.: is_const(NULL); returns false with no warnings. > 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'. > > Or does that trigger some odd case too? Following a quick test, this seems to work and to return true if given NULL as an argument (contrary to the current __is_const_expr()). So if we want to go beyond the C standard and extend the meaning of integer constant expression in the kernel to also include constant pointers, I agree that this is the way to go! Side question, Linus, what do you think about the __is_const_zero() documentation in 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
