SCOREC / core

parallel finite element unstructured meshes
Other
183 stars 62 forks source link

Use CMAKE_CXX_STANDARD to set C++14 standard #448

Closed cwsmith closed 1 week ago

cwsmith commented 2 months ago

Replace the -std=c++14 here https://github.com/SCOREC/core/blob/8959c599cc05e21d0fb470f941d9f892da62aa02/cmake/bob.cmake#L98 with set(CMAKE_CXX_STANDARD 14).

Tagging with CGNS as that code requires C++14: https://github.com/SCOREC/core/blob/8959c599cc05e21d0fb470f941d9f892da62aa02/CMakeLists.txt#L40-L42

jacobmerson commented 2 months ago

Here is some suggested code from Professional CMake:

# Require C++20, but let a parent project ask for something higher
if(DEFINED CMAKE_CXX_STANDARD)
  if(CMAKE_CXX_STANDARD EQUAL 98 OR CMAKE_CXX_STANDARD LESS 20)
    message(FATAL_ERROR "This project requires at least C++20")
  endif()
else()
  set(CMAKE_CXX_STANDARD 20)
endif()
# Always enforce the language constraint
set(CMAKE_CXX_STANDARD_REQUIRED ON)
# We don't need compiler extensions, but let a parent ask for them
if(NOT DEFINED CMAKE_CXX_EXTENSIONS)
  set(CMAKE_CXX_EXTENSIONS OFF)
endif()

If we are making a major version bump for the PCU changes, can we consider jumping to c++17? I saw somewhere, but can't track it down at the moment that Kokkos is going to c++20 within the next 6 months.

cwsmith commented 2 months ago

Thank you.

I'm not sure we have a strong reason to increase the standard beyond 14 (pending the discussion here https://github.com/SCOREC/core/pull/447).

jacobmerson commented 2 months ago

It would be nice to push the language standard as much forward as we can on a major version release. C++17 has some performance guarantees related to NRVO/ copy elision. And many usability improvements over 14. But, I can understand the argument for not breaking customers code.

Is there a way for us to find out if any of our users are stuck on 14 and cannot move to 17?

cwsmith commented 2 months ago

I agree, there are nice benefits of the newer standards.

To find out if we will break anyone, we'd have to survey the pumi users (which we have not done before). A couple ideas:

jacobmerson commented 2 months ago

Do we want to wait for the decision on 17 or start a new issue for 17 and make progress on the CMAKE_CXX_STANDARD stuff

cwsmith commented 2 months ago

Let's get this fixed and wait on the use of 17.