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

Improve C++20 detection with CMake 3.12+ and gcc-9/c++20 support fixes #860

Merged
merged 9 commits into from
Jun 16, 2021

Conversation

maxgolov
Copy link
Contributor

@maxgolov maxgolov commented Jun 15, 2021

Fixes #859

  • CMake v3.12 introduced C++20 support :

image

Our logic was assuming that it's only introduced in 3.18.x , and it would break in Ubuntu-20.04 which has a compiler with C++20, but its CMake version is only 3.16 (less than 3.18).. Thus, it was always falling back to C++17 language standard.

  • now when we actually force C++20 standard, there were two minor compilation issues. One in our nostd::variant projection - it was missing bad_variant_access. And the second one was using std::strlen without #include <cstring>. Both have now been resolved. Interestingly enough, those issues did not affect vs2019 in c++20/c++latest 😄

How to test

Our build system "developer-focused" part for Linux and Mac (/tools/) already has the build scripts to run the build as follows:

nostd:

tools/build.sh

stdlib:

tools/build.sh stdlib -DWITH_STL=ON

That would produce the binaries under out/x86_64-linux-gnu-gcc-9/[nostd|stdlib]. Default build without any parameters - produces nostd flavor. And if custom parameters passed to CMake - these allow to produce the build with any configuration name (1st argument is configuration name). For example, the build with Standard Library types, which in turn always turns on C++20 , if available. If C++20 is not available, then it falls back to C++17.

Build script tools/build.sh - allows to build different configurations side-by-side in one workspace, with build artifacts for different configurations produced side-by-side in a single output directory, as follows:

image

It's also possible to use WSL2 and Windows machine to host multiple docker images with different compilers. All producing the output for subsequent packaging from one single binary artifacts directory a.k.a. multiple docker / single filesystem build farm, side-by-side, one workspace - many containers - separate output directories for artifacts built by different compilers. Neat, isn't it?

@maxgolov maxgolov requested a review from a team June 15, 2021 18:52
@maxgolov maxgolov changed the title Update CMakeLists.txt Improve C++ standard detection with CMake 3.12 and above Jun 15, 2021
@maxgolov maxgolov changed the title Improve C++ standard detection with CMake 3.12 and above Improve C++20 detection with CMake 3.12+ and gcc-9/c++20 support fixes Jun 15, 2021
@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

Merging #860 (d0e9321) into main (b073e8b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #860   +/-   ##
=======================================
  Coverage   95.50%   95.50%           
=======================================
  Files         156      156           
  Lines        6620     6620           
=======================================
  Hits         6322     6322           
  Misses        298      298           
Impacted Files Coverage Δ
api/test/nostd/string_view_test.cc 100.00% <ø> (ø)

@maxgolov maxgolov mentioned this pull request Jun 15, 2021
3 tasks
@maxgolov
Copy link
Contributor Author

@ThomsonTan - if you can have a quick look? I think this PR would be valuable addition to Lalit's CI PR.

Copy link
Contributor

@ThomsonTan ThomsonTan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this.

@@ -107,7 +107,7 @@ if(WITH_STL)
add_definitions(-DHAVE_CPP_STDLIB)
add_definitions(-DHAVE_GSL)
# Require at least C++17. C++20 is needed to avoid gsl::span
if(CMAKE_VERSION VERSION_GREATER 3.18.0)
if(CMAKE_VERSION VERSION_GREATER 3.11.999)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use VERSION_GREATER_EQUAL 3.12.0 to avoid using the crafted version number?

@lalitb
Copy link
Member

lalitb commented Jun 16, 2021

@ThomsonTan - if you can have a quick look? I think this PR would be valuable addition to Lalit's CI PR.

@ThomsonTan - also I have addressed your review comments in #855. Please see if they are fine. Thanks

@lalitb lalitb merged commit 8bf9bbc into main Jun 16, 2021
@lalitb lalitb deleted the maxgolov/fix_cmake_cxx_standard branch June 16, 2021 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails with -DWITH_STL=ON cmake option
3 participants