The goal of adopting gnu17 is not removing gnu extensions but being ready to adopt c23. c23 will be finalized in February 2024 (c23 schedule) and once it is publicly released, gnu23 can be adopted and then the kernel can be migrated to c23.

The reason for this is that there are some cases of using __attributes__(unused) which can be replaced with [[ maybe_unused ]] (explanation on cppreference.com) in c23.

src/sys/contrib/zstd/lib/common/xxhash.h:1354

When __STDC_VERSION__ >= 201112L, we need assert.h. However, header files like stdio.h is not included from src/include but we mock userspace header files by creating an equivalent to src/sys/contrib/zstd/lib/freebsd. This has been done since zstd was first imported to kernel in d3692a4dee4d6ff0926365bcf95bbedb039ea70d.

assert() is a macro function so I though copying and paste it to src/sys/contrib/zstd/lib/freebsd would be better than integrating assert() in zstd_kfreebsd.h

The system compiles and boots when buildworld and buildkernel are bumped to gnu17.

I'm running kyua tests right now which will be done in a few hours.

I'm surprised we're seeing '_Alignof' applied to an expression is a GNU extension with the gnu17 flag? Is this file compiled with other flags or does that warning not get silenced by using the gnu versions of the -std flag.

We use a lot of GNU extensions so using c17 would most likely require adding a bunch of -Wno-foo flags

Clang diagnostic flags
This warning is turned on by default.

I'd prefer this was unchanged and that we define _Alignof in terms of alignof for sufficiently new C standards in sys/cdefs.h. We already come close to doing it in cdefs.h (it defines it to __alignof() which is likely why the error happens. There's about 25 _Alignof in the kernel today and it will be confusing to people to know which to use...

_Alignof is an operator and alignof is already a C11 macro in <stdalign.h>, which will be an operator in C23. Should I replace them with __alignof()?

So <sys/cdefs.h> is already correct I think in that it doesn't define _Alignof at all but lets it use the native language version for C11.

The error here is that _Alignof/alignof can only be used on types. In C23 the way you would spell this is alignof(typeof(*tf)). Instead, I think the way to fix these is to use _Alignof(struct trapframe) which will be valid in C11 and not require typeof. Fixing the various db_trace.c to use struct trapframe instead of *tf just needs to be its own commit to fix my earlier change to be valid C11.

We are years away from requiring gnu23 to build the kernel I think. We support older toolchains (e.g. right now main still builds with clang 12). We can't feasibly get by with requiring the latest version of toolchains. We don't have a formal policy on this currently, and we could talk about that on, say, arch@. c17 is probably the newest thing we could require, and c11 is probably an easier pitch currently.

c17 is probably the newest thing we could require, and c11 is probably an easier pitch currently.

my pitch for c17 is: it's c11, actually (with the most glaring bugs fixed)

I think that we can easily sell c11. If we enumerate the things that are actually fixed, we can likely sell c17. Especially if there's a number of tangible, easy to articulate benefits. It gets dicier if the reasons are 'it's the newest' or 'trust me it's better' and the 'most of the bugs fixed' level of detail is more on the dicier end of the scale. I don't doubt it, but it would be helpful to enumerate what those things are and how it would help us, the FreeBSD project, have a better system.

Yes, GDB just recently went through this where we moved from requiring C++11 to C++17 and in that case there some specific things that GDB had already backported (std::optional<> and array_view come to mind) that helped justify sliding forward. In GDB's case the policy they use is that supported LTS OS versions have to have a compiler that supports the version of the language (or some such).

For FreeBSD I think we'd like to continue supporting cross-builds using external toolchains on supported versions of Linux and macOS, so would have some similar constraints in how quickly we can step our minimum toolchain requirements up.

It appears the list of changes between C11 and C17 is https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2244.htm I haven't looked through all of them but it seems like there are quite a few clarifications/fixes to the atomics specification.

If this is really needed, could this file just`#include "../../../../include/assert.h" instead of copying the file?

I guess the alternative would be to modify the xxhash.h file to use _Static_assert instead of including <assert.h>?

I think that we can easily sell c11. If we enumerate the things that are actually fixed, we can likely sell c17. Especially if there's a number of tangible, easy to articulate benefits. It gets dicier if the reasons are 'it's the newest' or 'trust me it's better' and the 'most of the bugs fixed' level of detail is more on the dicier end of the scale. I don't doubt it, but it would be helpful to enumerate what those things are and how it would help us, the FreeBSD project, have a better system.

I agree with this. The only reason for adopting c23 is to stopping depending on GNU C extensions. Introduction of alignof, alignas, and typeof in c23 can fix most of the problems.

IMO we should separate kernel-specific codes from user space as sys/contrib is separate from contrib.
I'm trying not to modify files fetched from zstd release. In this way, it is much easier to import next releases of zstd.

Actually, what I'd like to do for the kernel is just go ahead and define static_assert() in <sys/systm.h> (really <sys/kassert.h>) instead of requiring the use of _Static_assert in the kernel. The intention of C11 from my reading is that code should really be using static_assert and that _Static_assert is just a shim so that <assert.h> can define the wrapper macro. Honestly I'd prefer it if for the kernel if we eventually too a similar route with defining things like alignof in the kernel always and not having to use the more obfuscated _Alignof type keywords. In the case of static_assert fixing that would probably mean we wouldn't need this hack at all for zstd to compile in the kernel.

_Static_assert() is a compiler extension in clang, so I guess there is no need to redefine it.

sys/cdefs.h

#if !__has_extension(c_static_assert)
#if (defined(__cplusplus) && __cplusplus >= 201103L) || \
    __has_extension(cxx_static_assert)
#define	_Static_assert(x, y)	static_assert(x, y)
#elif __GNUC_PREREQ__(4,6) && !defined(__cplusplus)
/* Nothing, gcc 4.6 and higher has _Static_assert built-in */
#elif defined(__COUNTER__)
...

Yes, but <sys/cdefs.h> is a generic header for more than just the kernel. We define kernel-specific things in <sys/systm.h>. And I think we should just #define static_assert to _Static_assert in <sys/systm.h> under #ifdef KERNEL.