init.h: restore designated initializers in SYS_INIT_NAMED()

[marc-hb]

As seen in the PR #68125 discussion, commit 19a33c7 ("init: adjust the SYS_INIT dev field init to play nice with older compilers") entirely threw away designated initializers in SYS_INIT_NAMED() to avoid compatibility issues across toolchains.

One key aspect that was probably missed at the time: C and C++ are two different languages and this is especially true with respect to designated initializers.

Designated initializers provide safer and more readable code, especially in their much stricter C++ version. So use #ifdef __cplusplus to restore them in SYS_INIT_NAMED() thanks to a small braces difference between C and C++.

cc:

• Error compiling C++ with flag (-Wmissing-field-initializers) #67396

[cfriedt]

@marc-hb - wow! Thanks for the deep dive on this.

I believe this bit of code has been inadvertently broken a couple of times.

Is there a unit test that can be added or modified to compile this code with various C++ standard versions?

I also like the level of documentation, but I wonder if some of it could be localized to the test, with a reference made here.

[keith-packard]

I think we're all asking what the minimum supported GCC version should be and whether that could be something that doesn't require this hack?

[keith-packard]

Generally yes, designated initializers should always be used. Wondering if we can skip the C/C++ variance and expect people to use a compiler released in the last decade?

[marc-hb]

Thanks for the prompt reviews! Answering both in a single comment.

I believe this bit of code has been inadvertently broken a couple of times.

This one yes (see #68125 and friends) and one or two other places similar to it. I'm focusing this PR on one single example for now but I plan to use any conclusions here as a "template" for similar situations.

Is there a unit test that can be added or modified to compile this code with various C++ standard versions?

tests/lib/cpp/ which I already mentioned in the source is very useful in this case.

but I wonder if some of it could be localized to the test, with a reference made here.

I chose to keep everything in one place for this first version but I very much agree that some of the source comments are not specific to this .h file at all and would be better located in some "common" location elsewhere. I'm not convinced about tests/lib/cpp/ though... any other idea?

Wondering if we can skip the C/C++ variance and expect people to use a compiler released in the last decade?

This is a good question and it keeps coming back for C and now C++ (be scared?) Maybe this PR was an attempt to discuss this topic yet again :-) This time in the context of one specific, "real-world" example.

From https://docs.zephyrproject.org/latest/develop/languages/c/index.html

... extensive use of the features newly introduced in the 1999 release of the ISO C (C99)
...
Some Zephyr components make use of the features newly introduced in the 2011 release [...] For example, the cbprintf() component, used as the default formatted output processor for Zephyr, makes use of the C11 type-generic expressions [...]
In summary, it is recommended to use a compiler toolchain that supports at least the C11 standard for developing with Zephyr.

How I interpret this:

• C99: mandatory
• C11: recommended

gcc 4.5 which requires this PR was released in 2010 and 4.5.4 in 2012. So it's technically not "in this decade" but not far from it.

https://gcc.gnu.org/onlinedocs/gcc-4.5.4/gcc/Standards.html#Standards mentions neither C11 nor C1X

So gcc 4.5 falls between "mandatory" and "recommended"; just a tiny bit short of the latter.

https://gcc.gnu.org/onlinedocs/gcc-4.6.4/gcc/Standards.html#Standards (2011-2013) starts mentioning C1X but not C11 yet. 4.7 finally does.

BTW SOF has been successfully using cbprintf() for a long time so it looks like the C11 __Generic__ is actually optional for now: https://docs.zephyrproject.org/latest/services/formatted_output.html

As it should be pretty obvious now from #68125 and its many friends, the SOF project (like most XTensa projects?) depends on generated Cadence toolchains for XTensa which are more difficult to upgrade than `apt-get do-release-upgrade && apt-get install gcc". In the case of SOF, the same audio DSP spans multiple generations (e.g. Tiger Lake -> Raptor Lake, 2022) which makes this particular situation worse. Cadence toolchains also transitioned from gcc to clang recently which could explain why gcc was being a bit "neglected".

XTensa and Cadence aside, any toolchain upgrade obviously requires a much bigger validation effort than some random and small code change.

Wondering if we can skip the C/C++ variance and expect people to use a compiler released in the last decade?

Designated Initializers (and other features) are quite different between C and C++ so I think a reasonable amount of reasonably simple #ifdef __cpluplus conditionals is really not outrageous in .h files. If not for Designated Initializers, there will be some for other features eventually; I think it's a given considering the language differences. If any amount of "divide and conquer" is required at all to stretch compatibility, I think #ifdef __cpluplus is the obvious first choice. For instance, someone could want to use in the future some unusual C++ < C++20 compiler that rejects Designated Initializers entirely.

While the "standards deep dive" in the comments is admittedly a bit complex, this particular #ifdef code difference is very simple. I've been debugging much, much worse C macros! I think just searching the Internet for Zephyr+macrobatics can return some extreme example(s) :-)

PS: the compiler explorer at https://godbolt.org/ is great. Don't forget to switch between C and C++

[keith-packard]

How I interpret this:

* C99: mandatory
* C11: recommended

I don't think the standard version is what we want to base this particular decision on -- C99 didn't have anonymous structs or unions. GCC 4.5 appears to have had support for them, but it is not standards conforming as it seems to require the additional level of braces, which (if I read the C11 spec correctly) are not allowed, just like C++. Here's the relevant paragraph from the spec (ISO/IEC 9899:201x):

An unnamed member of structure type with no tag is called an anonymous structure; an
unnamed member of union type with no tag is called an anonymous union. The members
of an anonymous structure or union are considered to be members of the containing
structure or union. This applies recursively if the containing structure or union is also
anonymous.

So gcc 4.5 falls between "mandatory" and "recommended"; just a tiny bit short of the latter.

I suspect figuring out what C standard version Zephyr requires is a much longer conversation.

If GCC 4.5 support is required, then we need to adopt the conditional initializer as proposed, but I'd suggest making it conditional on using GCC of a suitable vintage rather than switching between C++ and C -- neither C11 nor C++ allow the braces.

[marc-hb]

I took anonymous unions for granted and missed they were new in C11, thanks for correcting me.

In that case how about simply de-anonymizing the problematic unions like this one? It would be a simple, mechanical change and would completely avoid the type of recurring issue this PR is trying to address. Setting all standards aside for a moment, the combination of anonymous unions with C++ restrictions on designated initializers is inherently brittle.

Designated Initializers are new to C+20 too but:

• Zephyr is mostly C, little C++
• Designated Initializers have some value, anonymous unions have... close to none?

I suspect figuring out what C standard version Zephyr requires is a much longer conversation.

... unless we stick to the current "C99 mandatory, C11 recommended" status currently documented in
https://docs.zephyrproject.org/latest/develop/languages/c/index.html

[cfriedt]

de-anonymizing sounds like a good approach.

[keith-packard]

Taking Zephyr back to C99 would be a really big regression in language support for so much code..

I think all we need for designated initializers is to follow the C++ restrictions which disallow mixing designated initializers and require that they be in the same order as the struct definition. What you've hit here is a mis-match between the GCC 4.5 implementation (which preceded C11) and the eventual C11 specification. All that I'm suggesting here is that the condition in the #ifdef be switched from#ifdef __cplusplus to an expression which catches only use of GCC before version 4.6. There's an existing check for this case in include/zephyr/toolchain/gcc.h which could be copied:

/*
 * GCC 4.6 and higher have the C11 _Static_assert built in and its
 * output is easier to understand than the common BUILD_ASSERT macros.
 * Don't use this in C++98 mode though (which we can hit, as
 * static_assert() is not available)
 */
#elif !defined(__cplusplus) && \
	((__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)) ||	\
	 (__STDC_VERSION__) >= 201100)
#define BUILD_ASSERT(EXPR, MSG...) _Static_assert(EXPR, "" MSG)
#else
#define BUILD_ASSERT(EXPR, MSG...)
#endif

And, sadly, given that this check exists, it looks like Zephyr was used with GCC 4.5 at some point...

[marc-hb]

Taking Zephyr back to C99 would be a really big regression in language support for so much code..

De-anonymizing a couple anonymous unions where they cause some minor issue with gcc 2010 sounds quite different from "taking Zephyr back to C99" to me.

I suspect figuring out what C standard version Zephyr requires is a much longer conversation.

Yet you just seemed quite confident that C99 is not the current minimum. So it must be C11, I don't think there's anything in between? So please update https://docs.zephyrproject.org/latest/develop/languages/c/index.html to clarify that a C11 toolchain is now mandatory. This is not what is stated there right now. Right now it says C11 is required for "some features" but include/zephyr/init.h does not look like some optional feature.

The global C/C++ toolchains and standards landscape is a giant and very complicated mess that very few people besides you can make sense of and that's even before you add MISRA on top (#68118). So, some clear rules like "A C11-compatible toolchain is required" (for anonymous unions and maybe others) is the bare minimum information Zephyr developers need to navigate this mess. We can't have a never ending series of regressions, reverts, and re-re-verts (#68118), etc. every time something like this happens. Spending hours and hours on compatibility issues like this every week is not productive. If include/zephyr/init.h requires C11 then so be it; we'll branch or add some #ifdefs as you suggested or something but we need first of all some clarity, some clear line between what is guaranteed to work versus what works by luck or not at all, not "the minimum baseline is some undefined gcc place between C99 and C11 until we have a longer discussion".

Incoming C++ rules ("profiles") trying to ban unsafe C code is going to make things even more ... fun https://thenewstack.io/can-c-be-saved-bjarne-stroustrup-on-ensuring-memory-safety/

[keith-packard]

first of all some clarity, some clear line between what is guaranteed to work versus what works by luck or not at all, not "the minimum baseline is some undefined gcc place between C99 and C11 until we have a longer discussion".

Unless we select some published C standard version, we're always going to be stuck in defining our own standard. And it seems like that's where we are today -- C99 is the base required version, but some components use a vaguely specified subset of the additions C11 made to the language.

If GCC 4.5 is the only non-C11 compiler we need to support (admittedly, this is a big "if"), then we could re-specify this as "C11 or GCC 4.5" instead of "C99 + random C11 mix-ins". And if this is true, then we can provide compatibility hacks as needed to patch up the differences between GCC 4.5 and C11. There is already one such hack in gcc.h for BUILD_ASSERT; adding more seems reasonable.

I think that would provide clarity here, although it would probably place the burden of maintaining the set of GCC 4.5 work-arounds on the users of that compiler. At least you'd have the backing of having that compiler version explicitly named as required-to-be-supported? Have you identified additional gaps between GCC 4.5 and C11?

But, that all depends on whether there are other pre-C11 compilers still in use today. If so, we're stuck with either requiring some subset of the Zephyr be limited to C99 or continuing to play whac-a-mole when we hit compiler compatibility issues like this. And even the "c99 is required" is going to be an ongoing challenge unless we actually set the compiler option to enforce that.

[marc-hb]

All that I'm suggesting here is that the condition in the #ifdef be switched from#ifdef __cplusplus to an expression which catches only use of GCC before version 4.6.

Submitted in #69486. Only the #ifdef (and comments) differ.

[marc-hb]

Alternative #69486 merged, closing.