-
-
Notifications
You must be signed in to change notification settings - Fork 602
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
Gcc12 related fixes #1138
Gcc12 related fixes #1138
Conversation
On gcc12 we need to use -std=c++20 since constexpr .c_str() on std::string is only officially available since c++20 and the new gcc wouldnt let us use it with older standard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could also compile correctly with gcc-arm-...version 10.3 .
So , no compile errors , and those changes for next gcc v 12.2 are also compatible with mine gcc v 10.3.
All seems working perfectly .
But looking some code changes on TPMS App, I do not know , maybe it would be better if someone , could also confirm that this App has not any side effect.
So to me , it is approved , but better if someone can also in parallel validate correct TPMS App functionality of that binary ,
Thanks for that future preparation to an smooth migration to next gcc-arm -... v 12.2
Note : Prior to that gcc version 12.2 related fixes ,
There were issued another PR #1135 , the first gcc-arm-none-eabi common gcc 10.3 and 12.2 related fixes without that PR , persistent memory UI was not working from 10.3 onwards, now solved )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the docker containers work, or do you need to update those?
Hy, all I've done is move the assignment operator to the header, inside the class. Somehow the compiler didnt use it otherwise. This solved it. The class already had members defined there, so I'm not too afraid of runtime sideeffects, if it compiles it should work ;) Especially with the old compiler. What potentially can have sideeffects is using a newer compiler.. We've caught the obvious one with pmem, but there are potentially more sneaky ones. So this PR is to enable easier testing and development using newer compiler to gain more experience with it, test bins with new compilers more :) Next step could be having nightlies with both, test them extensively, then later make it default for nightlies test, test, test and then eventually make a first release with it :) But before that happens we probably need to look at the binary size issue too.. |
This still fully compatible with gcc9.2 so no need to immediately upgrade the official toolchain, I would not rush that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah gotcha. This is just getting it ready for 12
Make the code compile with gcc12
Switch to c++20 was necessary due to the usage of std::string.c_str() inside constexpr code. This only became standard in c++20 and the new gcc complains about it when trying to compile with -std=c++17 :(
To preserve the ability of compiling with older gcc I used check_cxx_compiler_flag and only enable c++20 if the compiler can handle it. Tested and works with gcc9.2 and gcc12.2, would be great if you could test it with in-between versions!