Skip to content

Commit

Permalink
GH-477: C++20 is now a required standard.
Browse files Browse the repository at this point in the history 
DDS general: Modified: C++20 is now a required standard (GH-477).
  • Loading branch information
@AnarManafov
AnarManafov committed Jan 2, 2024
1 parent 9982552 commit 828c49e
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ string(TOLOWER ${PROJECT_NAME} PROJECT_NAME_LOWER)

set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/modules")

set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard version to use (default is 17)")
set(CMAKE_CXX_STANDARD 20 CACHE STRING "C++ standard version to use (default is 20)")

This comment has been minimized.

Sign in to view

Copy link
@dennisklein

dennisklein Jan 2, 2024 •
edited
Loading

Member

It is not a good practice to set CMAKE_* variables unconditionally. Those are part of the CMake user interface and a project shall leave them open to be defined by the user - historically there are exceptions on rare variables, but CMAKE_CXX_STANDARD is not one them, it is frequently set by packagers and/or users.

Also it is a project-wide setting, in some scenarios, e.g. for compatibility with the ROOT C++ interpreter, global control is required. From the point of view of the DDS project, it should only set the minimally required standard based on the actual code used. CMake provides a per-target facility to express the required language dependency - compile features are also exported with the CMake package making the dependency on the required C++ flag even more explicit and automated.

So, the most idiomatic approach is to set the per-target compile feature and to not modify CMAKE_CXX_STANDARD at all, but only sanity-check it, if set, e.g.: https://github.com/FairRootGroup/FairMQ/blob/41ac755c573a8d214eb90af6159ef90bfc376345/cmake/FairMQProjectSettings.cmake#L21C33-L29
message(STATUS "Requiring C++${CMAKE_CXX_STANDARD}")
set(CMAKE_CXX_STANDARD_REQUIRED ON)

# Arch
execute_process(COMMAND uname -m OUTPUT_VARIABLE arch OUTPUT_STRIP_TRAILING_WHITESPACE)
Expand Down
1 change: 1 addition & 0 deletions ReleaseNotes.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Fixed: Replace std::iterator as it's deprecated (C++17).
* Fixed: Tasks working directory is set to their slot directory instead of $DDS_LOCATION.
* Fixed: Multiple stability issues.
* Modified: C++20 is now a required standard (GH-477).
* Modified: Bump minimum version requirements for cmake (from 3.11.0 to 3.23.1) and boost (from 1.67 to 1.75). (GH-428)
* Modified: C++17 modernization of EnvProp.h/env_prop. (GH-368)
* Added: 3rd party dependency on Protobuf (min v3.15).
Expand Down

0 comments on commit 828c49e

Please sign in to comment.