-
Notifications
You must be signed in to change notification settings - Fork 6.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Zephyr does not define minimal C++ language standard requirement and does not track it #55204
Comments
For example MSVC compiler has /permissive- flag. Pity gcc does not something similar. I mean there is |
@aborisovich - documentation only goes so far. Why not contribute a PR? Perhaps a test so you are not inconvenienced the same way in the future. It will probably amount to only a few lines of code. |
Subject is certainly not easy and we need some tool that would run as part of Zephyr CI. And it will certainly not be only a few lines of code. I'm looking for potential solutions.
(referencing my minimal reproduction example).
|
@aborisovich - can you extract just the relevant portion of your error from the log and put it into a markdown code block, so that the error is a bit more obvious / searchable? It's a bit difficult to pinpoint where the actual error is and it would help if the issue were more concise. This would help others try to fix your issue without having access to your compiler. Can you provide a way to reproduce using Your issue can likely be solved relatively easily with a simple code change. Additional static analysis would not solve any aspect of the actual problem. It would just describe it differently. And please, again, critique the code, not the author :-) You're not here to discredit me, right? You're here to hopefully get help to solve your problem. |
@cfriedt I never, ever meant to discredit anyone. I assume that as obvious thing, that we developers work with mutual respect. |
@aborisovich - no harsh feelings. I'm glad that we can work together to make things better. Please let me know if you need any help. |
Using |
Zephyr docs don't explicitly say C11 or even C99 is required. C11 is still on the table as a pending requirement in RFC |
FWIW: as of #55290 the current functional status of the tree is something like: Zephyr supports building C++ applications (provided toolchain support) with any issued C++ standard equal to or later than C++98. Support for specific standard versions needs to be configured via one of the CONFIG_STD_CPPxx choice values. That sounds about right to me. We aren't a C++ project. We shouldn't be relying on C++ features to build our OS. I don't know necessarily that ancient standards are important per se, as out of tree apps move on (and in particular as SOF moves to xt-clang) I don't see why we can't bump that version in the future. But for now it's not a big hardship to maintain C++98 support. |
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
(and having said that it's clear I should just put that text in the docs and mark it as fixing this bug) |
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
That is not necessarily true. The documentation says that most parts of Zephyr only make use of the features available in C99 with some additional components requiring C11 -- i.e. the "minimum" compiler C standard support requirement varies depending on which component(s) you require. The documentation does not designate a specific "minimum supported" (or "maximum allowed" for that matter) C standard version because some components may benefit from the features available in a more recent version of the standard; nevertheless, the recommendation is to stick with C99 in general and C11 at most.
For the same reason described above, I would refrain from defining a hard limit; especially for C++ because newer C++ standards have many appealing features. The documentation should only provide the general recommendation (and the de facto requirement for the core components of Zephyr) for the C++ standard conformance, which would be to stick with C++11 in general. It would be preferable to allow a more recent version of the C++ standard for specific components and modules. |
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
I reopen this issue because it looks to me that you may missed the key point I am trying to show here.
Is not a resolution to problem with using C++. I agree that new language feature are appealing, but please note what I wrote in the issue description:
What if the zephyr application that uses some custom toolchain X to build embedded projects does not have a compiler supporting C++11 or newer? You basically block them from using your services (Zephyr APIs). |
Hm... as of right now we guarantee that all the OS headers build correctly with C++98 compilers (specifically gcc and clang with -std=c++98), and we're testing this along with all the other . It's true it was a hole, but I'm pretty sure it's been plugged. Obviously the discussion as to when to deprecate C++98 will be contentious, but for now at least that's the requirement. Which is to say: if you push a PR right now using (Note that there are other issues with C++ support at the toolchain level, like "how to link in the standard library" that are quite a bit farther behind. So while the OS has support lots of platforms won't quite build C++98 correctly even if their compilers technically support it; xcc and xt-clang are both in this category for now.) |
Maybe I need to make it simple and more concise.
|
Yeah, those problems are fixed as of #55290 Please open a new bug if you find a way to add non-C++98 code to an OS header without doing proper version detection. |
Thanks @andyross . Did not notice a whole PR just a single commit modifying docs. Sorry for the hussle and thank you for addressing the issue.
Indeed adding a |
We weren't explicit about what we mean by "C++ support" as it relates to standards versions, and it's been causing some friction in the tree vs. some of our cruftier toolchains. Add a paragraph to clarify things. Fixes zephyrproject-rtos#55204 Signed-off-by: Andy Ross <andyross@google.com>
Describe the bug
Zephyr defines C11 as minimal supported standard for C language in C language support - language standards.
Similar documentation article: C++ language support does not have "language standard" section. Roadmap #31281 also does not say a word about minimal requirements. Thus, minimal C++ version supported is unknown. This is a serious problem since:
Concrete problem encountered:
SOF project uses Cadence Xtensa toolchain "xt-xcc" that is based on GNU 4.2.0 compiler for Intel Meteorlake audio firmware.
Meteorlake board has some C++ code that uses Zephyr
#include <zephyr/sys/util.h>
that provides C/C++ interropt helpers at the time of writing. "xt-xcc" is obsolete for some time and latest support language is C++98 standard.Everything was working fine in our CI until PR #53405 , where author added
constexpr
keyword usage in commit deab09d .Author @cfriedt in that PR claims (selective quote):
Proof of wrong:
Fuction template definition
__z_log2_impl
frominclude/zephyr/sys/util.h
in some conditions returns value A and in other conditions value B. According to C++ standard ISO/IEC 14882:2011 §7.1.5.3:Also, cppreference.com - constexpr confirms this:
Quote marked "(until C++14)":
To Reproduce
Steps to reproduce the behavior:
https://godbolt.org/z/78sWY65v4
Note: compiler x86-64 gcc 12.2 compiles minimal example using C++11 without a warning!
Change compiler to: x86-64 gcc 11.3 to receive an error (related to multiple return statements in constexpr function):
set_property(TARGET output.s PROPERTY CXX_STANDARD 11)
toset_property(TARGET output.s PROPERTY CXX_STANDARD 14)
and watch how everything compiles successfully.
Expected behavior
Right now it is C++14 if we do not roll back PR lib: os: PoT utilities, hash function, hash map (hash table) #53405 (probably - remaining C++ code needs verification too!)
If the compiler supports C++17 or C++20 it is possible that it will compile the code marked -std=c++11 that should not compile with that standard due to compiler extensions they provide! We need to watch the conformance carefully, maybe some static analysis tools?
Impact
SOF project has to urgently upgrade toolchain to new Cadence Xtensa "xt-clang" based on clang, that allows to use up to C++17 standard. If we do not do this, we are unable upgrade Zephyr revisions since our code will not build!
Luckily, we had been preparing for toolchain change in thesofproject/sof#7027 for some time and have only 2 .cpp source files at the time of writing.
Logs and console output
Compiling with xt-xcc: xt-xcc.log
Compiling with xt-clang -std=c++11: xt-clang C++11.log
Environment (please complete the following information):
Additional context
N/A
The text was updated successfully, but these errors were encountered: