diff mbox series

[01/10] compiler.h: add statically_false()

Message ID 20241203-is_constexpr-refactor-v1-1-4e4cbaecc216@wanadoo.fr (mailing list archive)
State New
Headers show
Series compiler.h: refactor __is_constexpr() into is_const{,_true,_false}() | expand

Commit Message

Vincent Mailhol via B4 Relay Dec. 2, 2024, 5:33 p.m. UTC 
From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

For completion, add statically_false() which is the equivalent of
statically_true() except that it will return true only if the input is
known to be false at compile time.

The == operator is used instead of the ! negation to prevent a
-Wint-in-bool-context compiler warning when the argument is not a
boolean. For example:

  statically_false(var * 0)

Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
---
 include/linux/compiler.h | 1 +
 1 file changed, 1 insertion(+)

Comments

David Laight Dec. 4, 2024, 6:30 p.m. UTC | #1 
From: Vincent Mailhol
> Sent: 02 December 2024 17:33
> 
> From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> 
> For completion, add statically_false() which is the equivalent of
> statically_true() except that it will return true only if the input is
> known to be false at compile time.

This is pretty much pointless.
It is just as easy to invert the condition at the call site.

	David

> 
> The == operator is used instead of the ! negation to prevent a
> -Wint-in-bool-context compiler warning when the argument is not a
> boolean. For example:
> 
>   statically_false(var * 0)
> 
> Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> ---
>  include/linux/compiler.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 469a64dd6495fefab2c85ffc279568a657b72660..a2a56a50dd85227a4fdc62236a2710ca37c5ba52 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -314,6 +314,7 @@ static inline void *offset_to_ptr(const int *off)
>   * values to determine that the condition is statically true.
>   */
>  #define statically_true(x) (__builtin_constant_p(x) && (x))
> +#define statically_false(x) (__builtin_constant_p(x) && (x) == 0)
> 
>  /*
>   * This is needed in functions which generate the stack canary, see
> 
> --
> 2.45.2
> 
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Vincent Mailhol Dec. 5, 2024, 3:25 p.m. UTC | #2 
On Thu. 5 Dec 2024 at 03:30, David Laight <David.Laight@aculab.com> wrote:
> From: Vincent Mailhol
> > Sent: 02 December 2024 17:33
> >
> > From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> >
> > For completion, add statically_false() which is the equivalent of
> > statically_true() except that it will return true only if the input is
> > known to be false at compile time.
>
> This is pretty much pointless.
> It is just as easy to invert the condition at the call site.

To start with, I will argue that:

  statically_false(foo)

is more pretty than

  statically_true(!(foo))

In addition, the simple negation !(foo) only works if foo is a
boolean. If it is an integer, you would get the -Wint-in-bool-context
warning. Thus you would have to write:

  statically_true((foo) == 0)

Anyone using this in a global header basically does not know what type
of argument they are getting. So, the (foo) == 0 trick is a must. But
because it is ugly, better to encapsulate it once for all. The
statically_false() is just cleaner and less error prone.

That said, I am not strongly opposed to removing statically_false(),
but in that case, I would also remove is_const_false(). For me, these
come as a pair, either we have both or we have none. Only having one
of these looked unbalanced.


Yours sincerely,
Vincent Mailhol
David Laight Dec. 6, 2024, 3:39 a.m. UTC | #3 
From: Vincent Mailhol
> Sent: 05 December 2024 15:26
> 
> On Thu. 5 Dec 2024 at 03:30, David Laight <David.Laight@aculab.com> wrote:
> > From: Vincent Mailhol
> > > Sent: 02 December 2024 17:33
> > >
> > > From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > >
> > > For completion, add statically_false() which is the equivalent of
> > > statically_true() except that it will return true only if the input is
> > > known to be false at compile time.
> >
> > This is pretty much pointless.
> > It is just as easy to invert the condition at the call site.
> 
> To start with, I will argue that:
> 
>   statically_false(foo)
> 
> is more pretty than
> 
>   statically_true(!(foo))

Except that the test is more likely to be:
	statically_false(x > y)
and the invert is then
	statically_true(x <= y)

No different from C itself, there is no 'ifnot (condition) {...}'
(don't talk to me about perl...)

I suspect you need to pretty much remove all the comments that
cross-refer to statically_true() from the other patches.

So is_const_true() is just 'return true if the expression
is a 'non-zero constant integer expression'.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Vincent Mailhol Dec. 6, 2024, 4:42 a.m. UTC | #4 
On Fri. 6 Dec. 2024 at 12:39, David Laight <David.Laight@aculab.com> wrote:
> From: Vincent Mailhol
> > Sent: 05 December 2024 15:26
> >
> > On Thu. 5 Dec 2024 at 03:30, David Laight <David.Laight@aculab.com> wrote:
> > > From: Vincent Mailhol
> > > > Sent: 02 December 2024 17:33
> > > >
> > > > From: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > > >
> > > > For completion, add statically_false() which is the equivalent of
> > > > statically_true() except that it will return true only if the input is
> > > > known to be false at compile time.
> > >
> > > This is pretty much pointless.
> > > It is just as easy to invert the condition at the call site.
> >
> > To start with, I will argue that:
> >
> >   statically_false(foo)
> >
> > is more pretty than
> >
> >   statically_true(!(foo))
>
> Except that the test is more likely to be:
>         statically_false(x > y)
> and the invert is then
>         statically_true(x <= y)
>
> No different from C itself, there is no 'ifnot (condition) {...}'
> (don't talk to me about perl...)

No need to talk about perl, just staying in C, it has both the #ifdef
and the #ifndef directives (and since C23, it even has the #elifndef).

Regardless, this was just a nitpick. You convinced me, I will remove
both statically_false() and is_const_false() in v2.

> I suspect you need to pretty much remove all the comments that
> cross-refer to statically_true() from the other patches.
>
> So is_const_true() is just 'return true if the expression
> is a 'non-zero constant integer expression'.

No. Linus made it clear in

  https://lore.kernel.org/all/CAHk-=wh5SNYdgx8-X+ggHP+ojbG2F7oyt3TLmMgqejYd5zn0Aw@mail.gmail.com/

that we need an explanation of why statically_true() may not work in
some contexts and I agree with this.

As far as I like using proper nuances between the terms 'integer
constant expression' and 'compile time constant', treating
statically_true() and is_const_true() as completely different things
as you are suggesting is not constructive. At the end, what matters
the most in a comment, is that the final user properly understands how
to use the thing correctly.

Removing the cross reference would increase the risk of people using
is_const_true() in places where it is not needed. I will keep the
cross reference to statically_true().


Yours sincerely,
Vincent Mailhol
diff mbox series

Patch

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 469a64dd6495fefab2c85ffc279568a657b72660..a2a56a50dd85227a4fdc62236a2710ca37c5ba52 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -314,6 +314,7 @@  static inline void *offset_to_ptr(const int *off)
  * values to determine that the condition is statically true.
  */
 #define statically_true(x) (__builtin_constant_p(x) && (x))
+#define statically_false(x) (__builtin_constant_p(x) && (x) == 0)
 
 /*
  * This is needed in functions which generate the stack canary, see