Messages in this thread Patches in this message | | | Subject | Re: [PATCH] compiler_types: mark __compiletime_assert failure as __noreturn | From | Rasmus Villemoes <> | Date | Fri, 15 Oct 2021 09:55:58 +0200 |
| |
On 14/10/2021 20.55, Nick Desaulniers wrote: > On Thu, Oct 14, 2021 at 11:41 AM Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: >> >> On Thu, Oct 14, 2021 at 8:33 PM Miguel Ojeda >> <miguel.ojeda.sandonis@gmail.com> wrote: >>> >>> That would be a nice to do, but I am not sure about introducing one >>> more macro about this... I think it would be simpler to submit patches >>> for moves into `static_assert` even if we have to "flip" the meaning. > > $ grep -r BUILD_BUG_ON | wc -l > 3405 > >> Actually, what would be ideal is a compiler-backed lint that checks >> whether it could be an `static_assert`, perhaps in clang-tidy? > > Oh, that is a good idea. There is one already for recommending the > use of static_assert instead of assert. That's actually very nice.
So I did
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index b6ff83a714ca..e212220216e8 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -295,12 +295,17 @@ struct ftrace_likely_data { # define __compiletime_object_size(obj) -1 #endif
+#include <linux/const.h> + #ifdef __OPTIMIZE__ # define __compiletime_assert(condition, msg, prefix, suffix) \ do { \ extern void prefix ## suffix(void) __compiletime_error(msg); \ + extern void prefix ## suffix ## ice(void) __compiletime_warning("Ice ice baby"); \ if (!(condition)) \ prefix ## suffix(); \ + if (__is_constexpr(condition)) \ + prefix ## suffix ## ice(); \ } while (0) #else # define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0) and that throws a gazillion warnings. Picking one at random shows that container_of() has a BUILD_BUG_ON. So we could do
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 2776423a587e..0a1969b11619 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -492,9 +492,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } */ #define container_of(ptr, type, member) ({ \ void *__mptr = (void *)(ptr); \ - BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ - !__same_type(*(ptr), void), \ - "pointer type mismatch in container_of()"); \ + static_assert(__same_type(*(ptr), ((type *)0)->member) || \ + __same_type(*(ptr), void), \ + "pointer type mismatch in container_of()"); \ ((type *)(__mptr - offsetof(type, member))); })
/** @@ -507,9 +507,9 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } */ #define container_of_safe(ptr, type, member) ({ \ void *__mptr = (void *)(ptr); \ - BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \ - !__same_type(*(ptr), void), \ - "pointer type mismatch in container_of()"); \ + static_assert(__same_type(*(ptr), ((type *)0)->member) || \ + __same_type(*(ptr), void), \ + "pointer type mismatch in container_of_safe()"); \ IS_ERR_OR_NULL(__mptr) ? ERR_CAST(__mptr) : \ ((type *)(__mptr - offsetof(type, member))); }) [fixing the copy-pasto in container_of_safe while at it].
Basically, all BUILD_BUG_ONs like that that do type checking are very obviously candidates for using static_assert instead (they very naturally go at the end of all the locally declared variables, so one won't hit the declaration-after-statement problem that can otherwise prevent using static_assert).
> I was playing with trying to adapt clang-tidy's C++11 `auto` fixit to > work on GNU C code to automate the replacement of: > > __typeof(x) y = (x); > > with: > > __auto_type y = (x); > > in macros. That's perhaps interesting, too. Given the volume of code > in the kernel, I wouldn't waste time with one off patches;
Well, for the kind of macros that are used _everywhere_ a few one-off patches might be in order. It's also interesting if one could measure any speedup from switching those core macros to static_assert.
Rasmus
| |