search

electro-smith / DaisySP

A Powerful DSP Library in C++
https://www.electro-smith.com/daisy
Other
891 stars 140 forks source link

What's the C++ version used for DaisySP? #32

Closed TheSlowGrowth closed 4 years ago

TheSlowGrowth commented 4 years ago

The makefile doesn't explicitly state the C++ standard to be used, as far as I can tell. Is there officially a standard to use? I would suggest to use at least c++17, so that things like if constexpr () become available. This would be really nice for an embedded project, as it allows to do a lot of computation at compile time and save precious cycles without cryptic lookup tables and other unreadable code. There are more advantages, of course, but this would be the most useful IMO.

stephenhensley commented 4 years ago

I'm a fairly new migrant to C++ from C -- So with that, I don't have a strong basis for an opinion.

As of now, I don't think we're using any features beyond C++11.

Unless there are any reasons not to use c++17 over 14 or 11, then I think that's fine.

stephenhensley commented 4 years ago

Worth mentioning that I just checked on the STM32 Arduino Core (I made a wrapper of this library for Arduino) and it uses gnu++14 for its builds.

TheSlowGrowth commented 4 years ago

Hmm, that's a shame. I'm not sure if this can be changed on the Arduino side, but it seems not trivial. C++17 has really neat features for compile time optimization. But I guess C++14 is better than nothing :-)

stephenhensley commented 4 years ago

Just commenting so this doesn't seem like its stagnating. I think, at least for now, we're going to go with c++14 so that compatibility with Arduino doesn't become an immediate issue.

I set this on my WSL build yesterday, and had a few warnings/errors that I didn't have the time to dig into. Should be able to get to this before the weekend.

stephenhensley commented 4 years ago

Set to 14 for now, but open to moving to 17 at some point.

I'm still not totally familiar with the arduino-related build process. In my very brief attempt to build libdaisy with c++17 I realized the HAL was throwing a lot of register related warnings that did not happen in c++14. Definitely worth investigating at some point.