Re: [PATCH v2] linux/bits: simplify GENMASK_INPUT_CHECK()

[Linus Torvalds]

On Mon, 11 Nov 2024 at 08:48, Vincent Mailhol
<mailhol.vincent@xxxxxxxxxx> wrote:
>
>    - introduce _statically_true(), taking inspiration from
>      statically_true() as introduced in commit 22f546873149 ("minmax:
>      improve macro expansion and type checking")

So I really think this needs an explanation of what the difference is
when using __builtin_constant_p() vs using __is_constexpr(), and why
the existing statically_true() didn't work for you.

In my experience, __is_constexpr() is too limited, because it
literally requires a syntactically constant expression.

In contrast, __builtin_constant_p() often works for things that aren't
constant expressions, but that evaluate to constants at build time.

For example, I had a test patch that used statically_true() to do
things like "if the size of a user copy is a multiple of the size of
'long', call a simplified version without the byte copy part".

And sure, __is_constexpr() gets it right for completely constant
arguments. But __builtin_constant_p() will actually trigger not only
those, but also when the argument is something like

        if (copy_to_user(buf, values, n * sizeof(u64)))

because it sees that even if "n * sizeof(u64)" is not a constant, the
"is this a multiple of 'long' size" _is_ constant.

IOW, I think __builtin_constant_p() is preferable, because it not only
doesn't expand to the horror that is __is_constexpr(), it also
generally does better when you have the flexibility to use it.

Of course, I do think that the use in BUILD_BUG_ON_ZERO() requires
something that is more statically reliable, and so __is_constexpr()
that is purely syntactic is probably the right thing to have. So I'm
not objecting to your _statically_true() per se. I just think this
needs a big comment about why we have both versions, and when to use
one over the other.

                     Linus