doc: languages: add new section "headers and C/C++ compatibility" #74047
Conversation
Writing headers compatible with both C and C++ _and_ a range of toolchains can be very challenging, see for instance long discussions in commit c15f029 ("init.h: restore designated initializers in SYS_INIT_NAMED()"), PR zephyrproject-rtos#69411 and a few others linked from there. Signed-off-by: Marc Herbert <[email protected]>
marc-hb
requested review from
andyross,
ceolin,
keith-packard,
cfriedt,
tejlmand,
stephanosio,
dcpleung,
kv2019i,
fabiobaltieri and
gmarull
| style. In any case, both styles must never be mixed in the same | ||
| initializer. | ||
|
|
||
| Warning: successful compilation is not the end of the incompatibility |
There was a problem hiding this comment.
Should this be an actual .. warning: block, or is it just to make the text less dry?
There was a problem hiding this comment.
The latter. I don't think this is important enough to deserve the highlight of a warning block. Maybe make the "Warning" word bold?
| Recommendation: to maximize compatibility across different C and C++ | ||
| toolchains and standards, designated initializers in Zephyr header files | ||
| should follow all C++20 rules and restrictions. Non-designated, pre-C99 | ||
| initialization offers more compatibility and is also allowed but | ||
| designated initialization is the more readable and preferred code | ||
| style. In any case, both styles must never be mixed in the same | ||
| initializer. |
There was a problem hiding this comment.
I think an example may help make this stick a bit better. That said I love the extensive explanation, if there's something I find irritating on our existing rules is how the misra links often give ZERO explanations of WHY one should follow those guidelines so good job on that front, it's just that I have a hard time visualizing the issue.
Same for the one below.
There was a problem hiding this comment.
I thought about examples but (as you seem to have noted) I was concerned about length: there are many rules and potential examples of what not to do. You could show only the one "correct" example but then it's not obvious what not to do. I actually gave one example: it's the git commit referenced at the bottom! It's short and the diff shows by essence both what to do and not. I could add more references to commits?
BTW there are some related fixes coming in #70403, so this will provide a couple more sample commits. This doc should be merged first though because it was requested in #70403 - circular dependency :-) I can add these commit examples to the doc later.
I didn't find many references to zephyr commits in the documentation so I'm not sure whether such references is a recommended documentation practice or not. I added the one at the bottom because it was "worth a thousand words" and saving as many. In general I like referencing commits because it's short, precise and 100% immutable which completely solves the big problem of documentation being often out of sync.
There was a problem hiding this comment.
Ok fair enough. I'll be totally honest, I think that at this point in the project any rule that is applied by humans rather than CI is not going to work. Too many guidelines, too many people, too many things to consider. If I look at myself for an example, I don't regularly work with C++ and haven't been bitten by any of these regularly by either projects or CI and just for that reason don't see myself remembering to check these two days from now. On the other side, stuff like the explicit casting for the ->api void pointer is caught by CI and I find that that one managed to stick. Maybe we can have more combinations of C++ build? For different language versions or something. Anyway this is as good as it can be I suppose.
There was a problem hiding this comment.
Ok fair enough. I'll be totally honest, I think that at this point in the project any rule that is applied by humans rather than CI is not going to work.
Exactly why #70403 is adding more test coverage.
There was a problem hiding this comment.
Love the content. One extra gotcha, if you're in the mood for more writing, is empty structs. C allows struct foo {}; as a legal type which has zero size, C++ does not and demands it have at least one data member. To this day a struct k_spinlock has a needless dummy variable on uniprocessor builds when CONFIG_CPP=y, and I hate it.
| boilerplate code. C99 added initialization of ``struct`` and ``union`` | ||
| types by "designated" member names instead of a list of *bare* | ||
| expressions. Some GCC versions support designated initializers even in | ||
| their C90 mode. |
There was a problem hiding this comment.
FWIW, in point of fact what happened is that the standards committee simply adopted the pervasively-used gcc extension unmodified, which is the best kind of standards-making.
| allowed a surprisingly large and lax set of possibilities: designated | ||
| initializers can be out of order, duplicated, "nested" (``.a.x =``), | ||
| various other braces can be omitted, designated initializers and not can | ||
| be mixed, etc. |
There was a problem hiding this comment.
If there's concern for concision, this paragraph could be dropped. Users coming to Zephyr with these initializers already know how they work and why they're good, and if they don't they won't care. We don't need to document C, just our own environment.
There was a problem hiding this comment.
I considered this as a relatively short introduction to the main topic... I agree we don't need to document C but I think having a few examples of incompatibilities upfront helps.
There was a problem hiding this comment.
I think it would be helpful to have a couple of snippets with "wrong" and "correct" examples.
One extra gotcha, if you're in the mood for more writing, is empty structs. C allows struct foo {}; as a legal type which has zero size, C++ does not...
I agree these two would be valuable additions but between testing for this and writing it and learning enough about Device Tree to add relevant test coverage (#70403, hopefully final version just pushed), my "mood for writing" has really run out of steam. Sorry about that.
| allowed a surprisingly large and lax set of possibilities: designated | ||
| initializers can be out of order, duplicated, "nested" (``.a.x =``), | ||
| various other braces can be omitted, designated initializers and not can | ||
| be mixed, etc. |
There was a problem hiding this comment.
I considered this as a relatively short introduction to the main topic... I agree we don't need to document C but I think having a few examples of incompatibilities upfront helps.
Writing headers compatible with both C and C++ and a range of toolchains can be very challenging, see for instance long discussions in commit c15f029 ("init.h: restore designated initializers in SYS_INIT_NAMED()"), PR #69411 and a few others linked from there.