diff mbox series

[next,v2,02/11] minmax: Use _Static_assert() instead of static_assert()

Message ID 8059bc04da1a45bc810ac339a1129a4c@AcuMS.aculab.com (mailing list archive)
State Changes Requested
Headers show
Series minmax: Optimise to reduce .i line length | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch, async

Commit Message

David Laight Feb. 25, 2024, 4:49 p.m. UTC
The wrapper just adds two more lines of error output when the test fails.

Signed-off-by: David Laight <david.laight@aculab.com>
---
 include/linux/minmax.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Changes for v2:
- Typographical and spelling corrections to the commit messages.
  Patches unchanged.

Comments

Jani Nikula Feb. 26, 2024, 9:28 a.m. UTC | #1
On Sun, 25 Feb 2024, David Laight <David.Laight@ACULAB.COM> wrote:
> The wrapper just adds two more lines of error output when the test fails.

There are only a handful of places in kernel code that use
_Static_assert() directly. Nearly 900 instances of static_assert().

Are we now saying it's fine to use _Static_assert() directly all over
the place? People will copy-paste and cargo cult.

BR,
Jani.

>
> Signed-off-by: David Laight <david.laight@aculab.com>
> ---
>  include/linux/minmax.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> Changes for v2:
> - Typographical and spelling corrections to the commit messages.
>   Patches unchanged.
>
> diff --git a/include/linux/minmax.h b/include/linux/minmax.h
> index 63c45865b48a..900eec7a28e5 100644
> --- a/include/linux/minmax.h
> +++ b/include/linux/minmax.h
> @@ -48,7 +48,7 @@
>  #define __cmp_once(op, x, y, unique_x, unique_y) ({	\
>  	typeof(x) unique_x = (x);			\
>  	typeof(y) unique_y = (y);			\
> -	static_assert(__types_ok(x, y),			\
> +	_Static_assert(__types_ok(x, y),			\
>  		#op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
>  	__cmp(op, unique_x, unique_y); })
>  
> @@ -137,11 +137,11 @@
>  	typeof(val) unique_val = (val);						\
>  	typeof(lo) unique_lo = (lo);						\
>  	typeof(hi) unique_hi = (hi);						\
> -	static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),	\
> +	_Static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),	\
>  			(lo) <= (hi), true),					\
>  		"clamp() low limit " #lo " greater than high limit " #hi);	\
> -	static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");	\
> -	static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");	\
> +	_Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");	\
> +	_Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");	\
>  	__clamp(unique_val, unique_lo, unique_hi); })
>  
>  #define __careful_clamp(val, lo, hi) ({					\
David Laight Feb. 26, 2024, 10:07 a.m. UTC | #2
From: Jani Nikula
> Sent: 26 February 2024 09:28
> 
> On Sun, 25 Feb 2024, David Laight <David.Laight@ACULAB.COM> wrote:
> > The wrapper just adds two more lines of error output when the test fails.
> 
> There are only a handful of places in kernel code that use
> _Static_assert() directly. Nearly 900 instances of static_assert().

How many of those supply an error message?

> Are we now saying it's fine to use _Static_assert() directly all over
> the place? People will copy-paste and cargo cult.

Is that actually a problem?
The wrapper allows the error message to be omitted and substitutes
the text of the conditional.
But it isn't 'free'.
As well as slightly slowing down the compilation, the error messages
from the compiler get more difficult to interpret.

Most of the static_assert() will probably never generate an error.
But the ones in min()/max() will so it is best to make them as
readable as possible.
(Don't even look as the mess clang makes....)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Jani Nikula Feb. 26, 2024, 10:27 a.m. UTC | #3
On Mon, 26 Feb 2024, David Laight <David.Laight@ACULAB.COM> wrote:
> From: Jani Nikula
>> Sent: 26 February 2024 09:28
>> 
>> On Sun, 25 Feb 2024, David Laight <David.Laight@ACULAB.COM> wrote:
>> > The wrapper just adds two more lines of error output when the test fails.
>> 
>> There are only a handful of places in kernel code that use
>> _Static_assert() directly. Nearly 900 instances of static_assert().
>
> How many of those supply an error message?

At a glance, not many.

>> Are we now saying it's fine to use _Static_assert() directly all over
>> the place? People will copy-paste and cargo cult.
>
> Is that actually a problem?

I don't know. I'm asking.

Usually when we have compiler wrappers, they're meant to be used instead
of the thing being wrapped.

This series deviates from that, so it would seem to fair to mention it
slightly more verbosely than just stating what's being done.

> The wrapper allows the error message to be omitted and substitutes
> the text of the conditional.
> But it isn't 'free'.
> As well as slightly slowing down the compilation, the error messages
> from the compiler get more difficult to interpret.
>
> Most of the static_assert() will probably never generate an error.
> But the ones in min()/max() will so it is best to make them as
> readable as possible.
> (Don't even look as the mess clang makes....)

I'm not arguing any of this. :)


BR,
Jani.
diff mbox series

Patch

diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 63c45865b48a..900eec7a28e5 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -48,7 +48,7 @@ 
 #define __cmp_once(op, x, y, unique_x, unique_y) ({	\
 	typeof(x) unique_x = (x);			\
 	typeof(y) unique_y = (y);			\
-	static_assert(__types_ok(x, y),			\
+	_Static_assert(__types_ok(x, y),			\
 		#op "(" #x ", " #y ") signedness error, fix types or consider u" #op "() before " #op "_t()"); \
 	__cmp(op, unique_x, unique_y); })
 
@@ -137,11 +137,11 @@ 
 	typeof(val) unique_val = (val);						\
 	typeof(lo) unique_lo = (lo);						\
 	typeof(hi) unique_hi = (hi);						\
-	static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),	\
+	_Static_assert(__builtin_choose_expr(__is_constexpr((lo) > (hi)),	\
 			(lo) <= (hi), true),					\
 		"clamp() low limit " #lo " greater than high limit " #hi);	\
-	static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");	\
-	static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");	\
+	_Static_assert(__types_ok(val, lo), "clamp() 'lo' signedness error");	\
+	_Static_assert(__types_ok(val, hi), "clamp() 'hi' signedness error");	\
 	__clamp(unique_val, unique_lo, unique_hi); })
 
 #define __careful_clamp(val, lo, hi) ({					\