Skip to content
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

Enable support for C++23 in esp-idf (IDFGH-9684) #11025

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

0xFEEDC0DE64
Copy link
Contributor

std::expected, std::to_underlying, modules and concepts were missing in the esp-idf and this change enables all of them.

@0xFEEDC0DE64
Copy link
Contributor Author

I tried putting the following statements in my CMakeLists.txt:

set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS ON)

But esp-idf keeps appending a -std=gnu20 at all compiler calls which destroys my switch to c++23.

This has to be changed in idf itself.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Mar 20, 2023
@github-actions github-actions bot changed the title Enable support for C++23 in esp-idf Enable support for C++23 in esp-idf (IDFGH-9684) Mar 20, 2023
@0xjakob
Copy link
Contributor

0xjakob commented Apr 7, 2023

@0xFEEDC0DE64 Thanks for the suggestion! We are considering this PR but we first need to run tests on it and make sure that it won't affect anyone else.

@0xFEEDC0DE64
Copy link
Contributor Author

Can we make it a sdkconfig instead so you dont have to do testing? With defaults set to cpp20

@igrr
Copy link
Member

igrr commented Apr 9, 2023

@0xFEEDC0DE64 To set a custom value of C++ standard for your component (without having to change the default value used for other components) you can use set_property CMake command on your component library target. For example, in component CMakeLists.txt file:

idf_component_register(
    SRCS main.cpp
)

set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 23)

Then the component library will be compiled with an additional -std=gnu++23 argument, which will override the -std=gnu++20 argument added by default.

I've tried to compile the following example main.cpp file with the above CMakeLists.txt and it worked as expected:

#include <expected>

using std::expected;

extern "C" void app_main(void)
{
}

(Not saying that we won't update the default C++ language standard to C++23; just giving you a suggestion how to solve this problem without having to patch IDF or having to wait for us to do this update.)

@0xjakob
Copy link
Contributor

0xjakob commented Apr 11, 2023

sha=a96f5bd5b098feb0e3ac39180bd8b19183f80907

@0xjakob 0xjakob added the PR-Sync-Merge Pull request sync as merge commit label Apr 11, 2023
@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed and removed Status: Opened Issue is new Status: In Progress Work is in progress labels Apr 11, 2023
@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Reviewing Issue is being reviewed Resolution: NA Issue resolution is unavailable labels Apr 19, 2023
@CLAassistant
Copy link

CLAassistant commented Apr 20, 2023

CLA assistant check
All committers have signed the CLA.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Done Issue is done internally Resolution: Done Issue is done internally labels Apr 25, 2023
@espressif-bot espressif-bot merged commit a96f5bd into espressif:master Apr 25, 2023
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress labels Apr 26, 2023
@0xFEEDC0DE64
Copy link
Contributor Author

@igrr we just rebased to the latest esp-idf again, and the compiler is called again with -std=gnu++23 and -std=gnu++20. which results in compiler errors.

We are setting the c++ version with

set(CMAKE_CXX_STANDARD 23)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS ON)

in the topmost CMakeLists.txt and I added your set_property() to each component in our firmware project

set_property(TARGET ${COMPONENT_LIB} PROPERTY CXX_STANDARD 23)

But still I have to patch build.cmake in esp-idf to finally make it work. What has been merged here?

@0xFEEDC0DE64
Copy link
Contributor Author

Maybe COMPONENT_LIB has to be replaced with COMPONENT_TARGET ?

@igrr
Copy link
Member

igrr commented Apr 27, 2023

I think build.cmake has been updated in a96f5bd already. Could you please verify you have this commit in your fork?

@0xFEEDC0DE64
Copy link
Contributor Author

Yes we have that commit, but for some reason the if branch for clang was needed to patch to get it to compile again. I dont understand why this happens.

0xFEEDC0DE64@8e9e12a

@0xjakob
Copy link
Contributor

0xjakob commented Apr 28, 2023

@0xFEEDC0DE64 Sorry for this. We tried to fix an incompatibility with our version of clang, but the code was faulty. We'll take a look ASAP.

@0xjakob
Copy link
Contributor

0xjakob commented Apr 28, 2023

@0xFEEDC0DE64 Meanwhile, could you try the following in your component's CMakeLists.txt?

target_compile_options(${COMPONENT_LIB} PRIVATE -std=gnu++23)

When I use this, the standard is set twice

  • First to gnu++20, by IDF general build commands,
  • The second time to gnu++23, from the component's CMakeLists.txt, which is the one prevailing.

I can then compile C++23 code (which fails compiling without setting the option in the component's CMakeLists.txt).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Sync-Merge Pull request sync as merge commit Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants