Skip to content

Commit

Permalink
C++20 mode support and pipeline runs (C++17 remains the main and mini…
Browse files Browse the repository at this point in the history
…mal mode, but we now ensure code supports C++20 mode).
  • Loading branch information
ygoldfeld committed Jan 8, 2025
1 parent 32f0806 commit 928e6c2
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 8 deletions.
54 changes: 52 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,30 @@ jobs:
strategy:
fail-fast: false

# Our basic matrix is compiler-version x build-test-config. The latter is the usual possibilities namely
# Debug, Release, ..., plus a few runtime-sanitizer-oriented variations. Generally we want to run the entire
# cross-product AxB of possibilities; but for various reasons we exclude some specific combinations from the
# matrix. So far, then: specify full matrix, specify certain combos to remove from it.
#
# There is however another variable: the C++ standard to specify. In actual practice -- as explained in
# FlowLikeCodeGenerate.cmake, but we *very* briefly recap -- the min required standard is 17, and it is indeed
# *the* standard we use, never taking advantage (via conditional compilation, etc.) of 20-or-higher. So,
# the main matrix shall built with std=17. In addition, though, we *allow* C++20 or higher. In practice, 99.999%
# C++17 code shall build equivalently with C++20 (as for C++23, we haven't looked into it as of this writing);
# but not 100%, and indeed for business reasons we found it is worthwhile to *test* with C++20 as well, so
# as to ensure code will (at least) build (and thus not mess-over C++20 users; it is now 2025 as of this writing).
# However, loosely speaking, we need only hit a few basic setups likeliest to trigger problems (usually warnings)
# which we'd then fix in the code. Long story short then: in addition to the aforementioned matrix (w/ 17),
# minus exclusions, we would *add* a few specific configs with std=20.
#
# Delightfully that's just how GitHub Actions matrix specs work: The matrix, then remove `exclude`s, then
# add `include`s.
matrix:
# Just one value for this C++-version axis of the basic 3D matrix: C++17. I.e., the basic matrix is 2D really.
# For simplicity the Conan-profile-configuring script step simply keys off the `id` instead of our defining
# other properties as for the other 2 more complex axes below (compiler, build config).
cxx-std: { id: cxx17 },
compiler:
- id: gcc-9
name: gcc
Expand Down Expand Up @@ -245,7 +268,8 @@ jobs:
sanitizer-name: tsan
no-lto: True

# Again, these exclusions are explained in FLow-IPC workflow counterpart.
# Remove certain values from the 2D matrix so far.
# Again, these exclusions are explained in Flow-IPC workflow counterpart.
exclude:
- compiler: { id: gcc-9 }
build-test-cfg: { id: relwithdebinfo-asan }
Expand Down Expand Up @@ -278,6 +302,28 @@ jobs:
- compiler: { id: clang-13 }
build-test-cfg: { id: relwithdebinfo-tsan }

# Lastly add certain specific values on top of the result so far. Namely we want to also add some C++20 x AxB
# combos. The basic strategy is noted above; but specifically release-oriented builds tend to go down different
# paths/produce different warnings vs debug-oriented builds -- so we want to try both; and as for which
# compilers to use, on one hand we'll definitely want at least 1 of gcc and clang, but beyond that we suspect
# just the highest version of which is sufficient. TODO: Revisit, especially if users report C++20 problems
# in other environments, and therefore we had to fix them; test coverage should increase.
include:
# gcc-<highest> x (2 build-configs).
- cxx-std: { id: cxx20 }
compiler: { id: gcc-13 }
build-test-cfg: { id: debug }
- cxx-std: { id: cxx20 }
compiler: { id: gcc-13 }
build-test-cfg: { id: relwithdebinfo }
# clang-<highest> x (2 build-configs).
- cxx-std: { id: cxx20 }
compiler: { id: clang-17 }
build-test-cfg: { id: debug }
- cxx-std: { id: cxx20 }
compiler: { id: clang-17 }
build-test-cfg: { id: relwithdebinfo }

# Not using ubuntu-latest, so as to avoid surprises with OS upgrades and such.
runs-on: ubuntu-22.04

Expand Down Expand Up @@ -383,7 +429,7 @@ jobs:
[settings]
compiler = ${{ matrix.compiler.name }}
compiler.version = ${{ matrix.compiler.version }}
compiler.cppstd = 17
compiler.cppstd = ${{ ((matrix.cxx-std.id == 'cxx20') && '20') || '17' }}
# TODO: Consider testing with LLVM-libc++ also (with clang anyway).
compiler.libcxx = libstdc++11
arch = x86_64
Expand All @@ -403,6 +449,10 @@ jobs:
[options]
flow:build = True
flow:doc = False
# `0` here as of this writing would effectively mean `17` too. Specifying `17` explicitly has the same
# effect in the end but goes down a slightly different code-path in FlowLikeCodeGenerate.cmake. It's not
# a huge deal... but it's a little nice to cover more paths, so let's do the `0` path when applicable.
flow:build_cxx_std = ${{ ((matrix.cxx-std.id == 'cxx20') && '20') || '0' }}
EOF
- name: Install Flow dependencies with Conan using the profile
Expand Down
7 changes: 7 additions & 0 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,17 @@ class FlowRecipe(ConanFile):
options = {
"build": [True, False],
"build_no_lto": [True, False],

# 0 => default (let build script decide, as of this writing 17 meaning C++17) or a #, probably `20` as of
# this writing.
"build_cxx_std": ["Any"],
"doc": [True, False]
}

default_options = {
"build": True,
"build_no_lto": False,
"build_cxx_std": 0,
"doc": False
}

Expand Down Expand Up @@ -83,6 +88,8 @@ def generate(self):
if self.options.build:
if self.options.build_no_lto:
toolchain.variables["CFG_NO_LTO"] = "ON"
if self.options.build_cxx_std != 0:
toolchain.variables["CMAKE_CXX_STANDARD"] = self.options.build_cxx_std
else:
toolchain.variables["CFG_SKIP_CODE_GEN"] = "ON"
if self.options.doc:
Expand Down
22 changes: 16 additions & 6 deletions tools/cmake/FlowLikeCodeGenerate.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,23 @@ message(VERBOSE "Environment checks passed. Configuring build environment.")

# Set CMake global stuff.

# Compile our own source files in C++17 mode; and refuse to proceed, if the compiler does not support it.
# Note that any `#include`r will also need to follow this, as our headers are not shy about using C++17 features;
# but that's enforced by a check in common.hpp (at least); not by us here somehow.
set(CXX_STD 17)
set(CMAKE_CXX_STANDARD ${CXX_STD})
# CMAKE_CXX_STANDARD controls the C++ standard version with which items are compiled.
# We require C++17 at the lowest for compilation (and for any `#include`r -- but that is enforced via compile-time
# check in universally-included common.hpp, not by us here somehow).
# So if CMAKE_CXX_STANDARD is not specified by CMake invoker, we in fact build in C++17 mode.
# If it *is* specified, then we don't override it. In practice, as of this writing, that means it should be 17
# or 20. (If a lower one is attempted, we don't fight it here -- but common.hpp check will defeat it anyway.)

# Won't decay to lower standard if compiler does not support CMAKE_CXX_STANDARD (will fail instead).
set(CMAKE_CXX_STANDARD_REQUIRED ON)
message(STATUS "C++${CXX_STD} language and STL required.")

if(NOT DEFINED CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 17)
message(STATUS "C++${CMAKE_CXX_STANDARD} language and STL required: set by this build script.")
else()
message(STATUS "C++${CMAKE_CXX_STANDARD} language and STL requirement "
"inherited from externally set CMAKE_CXX_STANDARD.")
endif()

# When we do find_package(Threads) to link threading library this will cause it to
# prefer -pthread flag where applicable (as of this writing, just Linux, but perhaps any *nix ultimately).
Expand Down

0 comments on commit 928e6c2

Please sign in to comment.