diff --git a/.github/workflows/auto-formatting.yml b/.github/workflows/auto-formatting.yml index 7274f34038..589a626ed4 100644 --- a/.github/workflows/auto-formatting.yml +++ b/.github/workflows/auto-formatting.yml @@ -12,7 +12,8 @@ jobs: # Checkout the pull request branch, also include all history - uses: actions/checkout@v7 with: - ref: ${{ github.head_ref }} + # Use the PR ref in the base repository so fork branches are resolved. + ref: refs/pull/${{ github.event.pull_request.number }}/head fetch-depth: 0 - name: "Set up Python" diff --git a/CMakeLists.txt b/CMakeLists.txt index 90fe822aa3..6737ab00e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -560,10 +560,11 @@ set_target_properties( set(CONFIG_LDFLAGS "${CONFIG_LDFLAGS} -L\$BOUT_LIB_PATH -lbout++") set(BOUT_INCLUDE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/include") set(CONFIG_CFLAGS - "${CONFIG_CFLAGS} -I\${BOUT_INCLUDE_PATH} -I${CMAKE_CURRENT_BINARY_DIR}/include ${CMAKE_CXX_FLAGS} -std=c++20" + "${CONFIG_CFLAGS} -I\${BOUT_INCLUDE_PATH} -I${CMAKE_CURRENT_BINARY_DIR}/include ${CMAKE_CXX_FLAGS} -std=c++${BOUT_CXX_STANDARD}" ) -target_compile_features(bout++ PUBLIC cxx_std_20) +target_compile_features(bout++ PUBLIC cxx_std_${BOUT_CXX_STANDARD}) +bout_target_set_language_standards(bout++) set_target_properties(bout++ PROPERTIES CXX_EXTENSIONS OFF) # Optional compiler features @@ -783,6 +784,7 @@ option( if(BOUT_TESTS) set(CMAKE_CTEST_ARGUMENTS "--output-on-failure" "--timeout" "300") enable_testing() + add_subdirectory(tests/cmake) # Targets for just building the tests # Tests need to add themselves as dependencies to these targets add_custom_target(build-check-unit-tests) @@ -803,7 +805,10 @@ if(BOUT_TESTS) # Targets for running the tests if(BOUT_ENABLE_UNIT_TESTS) add_custom_target( - check-unit-tests COMMAND ctest -R serial_tests --output-on-failure + check-unit-tests + COMMAND ${CMAKE_CTEST_COMMAND} -R serial_tests --output-on-failure + COMMAND ${CMAKE_CTEST_COMMAND} -R cmake-language-standards + --output-on-failure ) add_dependencies(check-unit-tests build-check-unit-tests) endif() diff --git a/cmake/BOUT++functions.cmake b/cmake/BOUT++functions.cmake index be56977996..f8013c21ee 100644 --- a/cmake/BOUT++functions.cmake +++ b/cmake/BOUT++functions.cmake @@ -1,5 +1,27 @@ # Helper functions for building BOUT++ models +set(BOUT_CXX_STANDARD 20) + +# Apply the BOUT++ minimum to each dialect without changing directory defaults +# or downgrading a newer standard selected by the caller. +function(bout_target_set_language_standards TARGET_NAME) + foreach(language IN ITEMS CXX CUDA HIP) + get_target_property(standard ${TARGET_NAME} ${language}_STANDARD) + if(NOT standard + OR standard EQUAL 98 + OR standard LESS BOUT_CXX_STANDARD + ) + set_property( + TARGET ${TARGET_NAME} PROPERTY ${language}_STANDARD + ${BOUT_CXX_STANDARD} + ) + endif() + set_property( + TARGET ${TARGET_NAME} PROPERTY ${language}_STANDARD_REQUIRED ON + ) + endforeach() +endfunction() + # Copy FILENAME from source directory to build directory macro(bout_copy_file FILENAME) configure_file( @@ -37,6 +59,7 @@ macro(bout_handle_requires_conflicts TYPENAME TYPEVAR) endmacro() function(bout_target_enable_hip TARGET_NAME) + bout_target_set_language_standards(${TARGET_NAME}) if(BOUT_HAS_HIP) get_target_property(model_sources ${TARGET_NAME} SOURCES) list(FILTER model_sources INCLUDE REGEX ".*\\.(cxx|cpp|cc)$") @@ -44,11 +67,7 @@ function(bout_target_enable_hip TARGET_NAME) set_source_files_properties(${model_sources} PROPERTIES LANGUAGE HIP) endif() set_target_properties( - ${TARGET_NAME} - PROPERTIES HIP_STANDARD 20 - HIP_STANDARD_REQUIRED ON - HIP_EXTENSIONS OFF - LINKER_LANGUAGE HIP + ${TARGET_NAME} PROPERTIES HIP_EXTENSIONS OFF LINKER_LANGUAGE HIP ) endif() endfunction() diff --git a/cmake/SetupBOUTThirdParty.cmake b/cmake/SetupBOUTThirdParty.cmake index d6d3f18b1e..799b40c9e7 100644 --- a/cmake/SetupBOUTThirdParty.cmake +++ b/cmake/SetupBOUTThirdParty.cmake @@ -22,8 +22,6 @@ if(BOUT_HAS_CUDA) set(BOUT_SOURCES_CXX ${BOUT_SOURCES}) list(FILTER BOUT_SOURCES_CXX INCLUDE REGEX ".*\.cxx") - # NOTE: CUDA inherits the CXX standard setting from the top-level - # compile features, set for the bout++ target. set_source_files_properties(${BOUT_SOURCES_CXX} PROPERTIES LANGUAGE CUDA) # timer.cxx contains no device code, and compiling its C++20 formatting and @@ -43,10 +41,7 @@ if(BOUT_HAS_HIP) list(FILTER BOUT_HIP_SOURCES INCLUDE REGEX ".*\\.cxx$") set_source_files_properties(${BOUT_HIP_SOURCES} PROPERTIES LANGUAGE HIP) set_target_properties( - bout++ - PROPERTIES HIP_STANDARD 20 - HIP_STANDARD_REQUIRED ON - LINKER_LANGUAGE HIP + bout++ PROPERTIES HIP_EXTENSIONS OFF LINKER_LANGUAGE HIP ) find_package(hip REQUIRED CONFIG) target_link_libraries(bout++ PUBLIC hip::host) diff --git a/cmake/SetupCompilers.cmake b/cmake/SetupCompilers.cmake index 8b8256a59c..4d76643ac0 100644 --- a/cmake/SetupCompilers.cmake +++ b/cmake/SetupCompilers.cmake @@ -56,9 +56,6 @@ if(BOUT_ENABLE_HIP) ) endif() enable_language(HIP) - set(CMAKE_HIP_STANDARD 20) - set(CMAKE_HIP_STANDARD_REQUIRED ON) - set(CMAKE_HIP_EXTENSIONS OFF) endif() set(BOUT_HAS_HIP ${BOUT_ENABLE_HIP}) set(CUDA_ARCH diff --git a/tests/README.md b/tests/README.md index 1defe24de5..211dfda42b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -8,6 +8,17 @@ The BOUT++ test suite is split into three parts: +## CMake tests + +The `cmake-language-standards` tests check target-local C++, CUDA and HIP +standard settings and compile a C++23 probe when supported (C++20 otherwise). +The GPU properties are inspected +without enabling GPU languages, so these tests do not require CUDA or HIP +toolchains and do not replace GPU compilation tests. + +Run them from a configured build directory with +`ctest -R cmake-language-standards --output-on-failure`. + ## Unit tests These aim to be a comprehensive set of tests that run *very* fast and diff --git a/tests/cmake/CMakeLists.txt b/tests/cmake/CMakeLists.txt new file mode 100644 index 0000000000..52a711e2a9 --- /dev/null +++ b/tests/cmake/CMakeLists.txt @@ -0,0 +1,14 @@ +add_test( + NAME cmake-language-standards + COMMAND + ${CMAKE_COMMAND} -S ${CMAKE_CURRENT_SOURCE_DIR}/language-standards -B + ${CMAKE_CURRENT_BINARY_DIR}/language-standards + -DBOUT_SOURCE_DIR=${PROJECT_SOURCE_DIR} +) +add_test(NAME cmake-language-standards-build + COMMAND ${CMAKE_COMMAND} --build + ${CMAKE_CURRENT_BINARY_DIR}/language-standards +) +set_tests_properties( + cmake-language-standards-build PROPERTIES DEPENDS cmake-language-standards +) diff --git a/tests/cmake/language-standards/CMakeLists.txt b/tests/cmake/language-standards/CMakeLists.txt new file mode 100644 index 0000000000..6494ee8d85 --- /dev/null +++ b/tests/cmake/language-standards/CMakeLists.txt @@ -0,0 +1,109 @@ +cmake_minimum_required(VERSION 3.17) +project(BoutLanguageStandards LANGUAGES CXX) +include("${BOUT_SOURCE_DIR}/cmake/BOUT++functions.cmake") + +function(expect_property target property expected) + get_target_property(actual ${target} ${property}) + if(NOT actual STREQUAL expected) + message( + FATAL_ERROR "${target}.${property}: expected ${expected}, got ${actual}" + ) + endif() +endfunction() + +# GPU properties can be checked without a GPU compiler. This is a configuration +# regression test, not a CUDA/HIP compilation or runtime test. +add_library(defaults INTERFACE) +bout_target_set_language_standards(defaults) +foreach(language IN ITEMS CXX CUDA HIP) + expect_property(defaults ${language}_STANDARD 20) + expect_property(defaults ${language}_STANDARD_REQUIRED ON) + if(DEFINED CMAKE_${language}_STANDARD) + message(FATAL_ERROR "Helper changed ${language} directory default") + endif() + set(CMAKE_${language}_STANDARD 23) +endforeach() + +add_library(newer INTERFACE) +# Model an explicitly chosen target standard independently of CMake defaults. +foreach(language IN ITEMS CXX CUDA HIP) + set_property(TARGET newer PROPERTY ${language}_STANDARD 23) +endforeach() +bout_target_set_language_standards(newer) +foreach(language IN ITEMS CXX CUDA HIP) + expect_property(newer ${language}_STANDARD 23) + if(NOT CMAKE_${language}_STANDARD EQUAL 23) + message(FATAL_ERROR "Helper changed caller's ${language} default") + endif() +endforeach() + +foreach(old_standard IN ITEMS 98 17) + add_library(older_${old_standard} INTERFACE) + foreach(language IN ITEMS CXX CUDA HIP) + set_property( + TARGET older_${old_standard} PROPERTY ${language}_STANDARD + ${old_standard} + ) + endforeach() + bout_target_set_language_standards(older_${old_standard}) + foreach(language IN ITEMS CXX CUDA HIP) + expect_property(older_${old_standard} ${language}_STANDARD 20) + endforeach() +endforeach() + +add_library(unrelated INTERFACE) +foreach(language IN ITEMS CXX CUDA HIP) + set_property(TARGET unrelated PROPERTY ${language}_STANDARD 17) + expect_property(unrelated ${language}_STANDARD 17) +endforeach() + +set(BOUT_HAS_HIP ON) +bout_target_enable_hip(newer) +expect_property(newer HIP_STANDARD 23) +expect_property(newer HIP_EXTENSIONS OFF) +expect_property(newer LINKER_LANGUAGE HIP) +bout_target_enable_hip(defaults) +expect_property(defaults HIP_STANDARD 20) + +# Exercise SetupCompilers' HIP branch without requiring a ROCm installation. +# Only language detection is stubbed; the production configuration runs. +function(enable_language language) + if(NOT language STREQUAL "HIP") + message(FATAL_ERROR "Unexpected language detection: ${language}") + endif() + set(hip_detection_requested + ON + PARENT_SCOPE + ) +endfunction() +set(BOUT_ENABLE_MPI OFF) +set(BOUT_ENABLE_HIP ON) +set(BOUT_ENABLE_CUDA OFF) +set(BOUT_ENABLE_RAJA ON) +set(BOUT_ENABLE_UMPIRE ON) +set(CMAKE_HIP_STANDARD_REQUIRED OFF) +set(CMAKE_HIP_EXTENSIONS ON) +include("${BOUT_SOURCE_DIR}/cmake/SetupCompilers.cmake") +if(NOT hip_detection_requested + OR NOT CMAKE_HIP_STANDARD EQUAL 23 + OR CMAKE_HIP_STANDARD_REQUIRED + OR NOT CMAKE_HIP_EXTENSIONS +) + message(FATAL_ERROR "HIP setup changed caller's directory defaults") +endif() + +# Do not raise the project's supported compiler minimum just for this probe. +set(BOUT_HAS_HIP OFF) +if("cxx_std_23" IN_LIST CMAKE_CXX_COMPILE_FEATURES) + set(probe_standard 23) +else() + set(probe_standard 20) +endif() +set(CMAKE_CXX_STANDARD ${probe_standard}) +add_executable(cpu_probe probe.cxx) +bout_target_enable_hip(cpu_probe) +expect_property(cpu_probe CXX_STANDARD ${probe_standard}) +if(probe_standard EQUAL 23) + target_compile_definitions(cpu_probe PRIVATE CHECK_CXX23) +endif() +message(STATUS "BOUT language standards regression checks passed") diff --git a/tests/cmake/language-standards/probe.cxx b/tests/cmake/language-standards/probe.cxx new file mode 100644 index 0000000000..22d099fb5b --- /dev/null +++ b/tests/cmake/language-standards/probe.cxx @@ -0,0 +1,13 @@ +#ifdef _MSVC_LANG +#define PROBE_CXX_STANDARD _MSVC_LANG +#else +#define PROBE_CXX_STANDARD __cplusplus +#endif + +#if defined(CHECK_CXX23) && PROBE_CXX_STANDARD < 202100L +#error "The caller's C++23 standard was not preserved" +#endif +#if PROBE_CXX_STANDARD < 202002L +#error "The BOUT++ C++20 minimum was not applied" +#endif +int main() { return 0; } diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index d931239a70..88548aac89 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -158,5 +158,4 @@ bout_target_enable_hip(serial_tests) if(BOUT_HAS_CUDA) set_source_files_properties(${serial_tests_source} PROPERTIES LANGUAGE CUDA) - set_target_properties(serial_tests PROPERTIES CUDA_STANDARD 20) endif()