Code contribution "modern" c++

Mbed OS
1

Hello,
i made some minor pull request lately and during the process i stumbled uppon a question.
i read the Styleguide here and it was helpful. i try to use astyle for every commit.
But it does not say anything about what featues of c++14 i can use.
for example i saw that a lot of preprocessor defines used in mbed, i myself tent to use constexpr.
same for ifdef and if constexpr, or auto.
so my question is what is wanted by the maintainers. is it allowed to use more modern c++ or is it better to stick to a more classic approach and orient myself on the exitsting code so it does not get to messy.
cheers

Edit: another question is: since C++14 is nearly 6 years old should we go on and update to c++17?

2 Likes
2

Reading your PR’s and the reviews, I was asking myself the same questions.

I hope mbed’s team will lean on the side of modern C++.

3

I think it is advisable to use current C++ features, especially when the compiler can produce better code.
The release of C++14 for Mbed is not so long ago, so most parts of Mbed are using more conservative coding. One reason is that Mbed supports several compilers and all must be on the same level, and it introduces more testing again.
So I would not say its a matter of the Mbed team is too conservative, e.g. the chrono features are used now and look how much code is affected.

I worry more about the different styles that are used now, before camel case was used most, now lower case with underscore is used also. Event BufferedSerial / UnbufferedSerial differ: e.g. format() vs. set_format().

4

regarding:

One reason is that Mbed supports several compilers and all must be on the same level, and it introduces more testing again.

the styleguide states here https://os.mbed.com/docs/mbed-os/v6.2/contributing/style.html#compiler-settings

  • Uses the GNU11 standard for C.
  • Uses the GNU++14 standard for C++.

so at least c++14 features should be ok.

regarding:

I worry more about the different styles that are used now, before camel case was used most, now lower case with underscore is used also. Event BufferedSerial / UnbufferedSerial differ: e.g. format() vs. set_format().

The question is what do the maintainers want. when we want to migrate to a modern approach we have to start somwhere. for example all new PR are allowed to use C++14 featues, or it is encuraged to create PRs where we refactor the existing code. but if this is not wanted creating such PRs could cause a lot discussions and troubles

5

I guess that improvements in speed or code size are welcome. My experience is, that Mbed was constantly growing, but newer releases with usage of constexpr or reducing virtual interfaces has reduced code size now.

And I wish that the Mbed team publishes a roadmap (at least rough) with priority of some topics.

6

This does seem quite limiting as we have almost have C++20.

Defaulting to C++17 shouldn’t break anything, but offers lots of potential performance improvements (especially copy mandatory elision).

Alternatively, at least let me set std=c++17 for my project and I’ll take the hit?

1 Like
7

mbed is always going to be constrained why what standards the three supported compilers can handle (iar, gcc, and armcc). As per a comment on this PR https://github.com/ARMmbed/mbed-os/pull/13881 you can see that C++17 support is nearing possibility. After that , it’s about generated code size, ram, and speed.

8

You can edit the profiles or add your own with the c++17 flag. That’s what I do.

1 Like
9

Thanks, I couldn’t see how to do that and couldn’t find any documentation.

Could you describe how/where you make the change?

10

Here is the documentation:

https://os.mbed.com/docs/mbed-os/v6.4/program-setup/build-profiles-and-rules.html

Here are the profiles:

You can create your own profile like this:

And then when compiling, you can point to it:

mbed compile --profile=debug --profile=path/to/your_profile.json

:warning: Unless you’ve copied one of mbed’s profiles to add your arguments, don’t forget to also use the --profile=debug/develop/release

1 Like
11

Thank you that’s perfect.

Really appreciate you help.

12

@ladislas how’s C++17 working for your team? We wanna try using it if possible but don’t want anything to break, ideally - I know the gcc-arm v9 toolchain is supposed to support it but I wanna ask someone with experience to be safer.

13

When I tried switching to -std=gnu++17, I started getting stack overflow issues from the cellular driver:

++ MbedOS Error Info ++
Error Status: 0x80020125 Code: 293 Module: 2
Error Message: CMSIS-RTOS error: Stack overflow
Location: 0x80267E7
File: mbed_rtx_handlers.c+60
Error Value: 0x1
Current Thread: cellular_queue Id: 0x20003A88 Entry: 0x80774C7 StackSize: 0x800 StackMem: 0x20009B08 SP: 0x2003FEB8

Increasing the default thread stack size won’t help. Did anybody have similar issues?

I also had to suspend warnings with the -Wno-register flag, as STM32Cube still uses keyword deprecated in C++17.

14

This will be fixed in the next CubeMX release, it’s safe to ignore as you did as the register keyword has zero effect.

Regarding your stack overflow, we haven’t had the problem, but we are not using the cellular driver.

Have you tried comparing binaries or assembly?

15

Sorry I missed your message!

It’s going well, we are not using anything really funky. To be fair, what we are using the most is “Selection statements with initializer”

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0305r1.html

This really makes our lives easier when writing drivers with long initialization sequences.

16

Yeah that’s one of the more prominent features we wanted to use, too. I just wanted to confirm which toolchain you are using to compile? Is it version 9 of arm-none-eabi-***?

17

We’re using v10.2.0 with no issues.

1 Like
18

Hi,

it should be noted that Mbed OS is only tested with the compiler versions specified in the release note of the last feature release. For 6.8.0 these are:

ARM compiler 6.15.0
GCC_ARM 9-2019-q4-major

Using other versions of the compilers is not recommended as they will not have been tested to ensure Mbed OS works with them.

Regards
Anna

19

Quick update a few weeks later about the C++17 features that we’ve been using with great success.
Each time, the impact on the code generated and the bin size has been measured to make sure it was worth using. We also have a 100% code coverage on our code to help us in the refactoring (though only running on the host platform). We have also been using clang-tidy with modernize-* and are very happy with the results. In particular, we have almost completely removed the use of const char [] variables for null terminated strings with std::string_view or std::array<char, N> and the use of c-arrays with std::array<T, N>

We also started using trailing return types and we really like it. As we work a lot in Swift on iOS at the same time, it makes both code bases closer in look which is a good thing for us

2 Likes
20

interesting stuff. I like also the shorthand writing for iterators in a while loop:

typedef std::map<uint32_t, BaseFeature*> FeatureDescriptorMap;
FeatureDescriptorMap fdm;

// use iterator
    for (const auto& [key, value] : _fdm) {
        if (strcmp((*value).getName(), name) == 0) {
            addr = key;
            break;
        }
    }

this is C++17, but even that gcc complains in C++14 setting, the correct code is generated. Recently, there were some updates to Cube drivers for STM targets, so hopefully C++17 will be enabled soon. I have seen also no bad impact on using gcc 10, the compiler version check wants < 10.0.