From 1eb5a1034e27c07662ebb9298052ddaa6e304095 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Fri, 24 Oct 2025 14:25:51 +0200 Subject: [PATCH 01/33] make FFTPoissonSolver's destructor = default --- src/fields/fft_poisson_solver/FFTPoissonSolver.H | 2 +- src/fields/fft_poisson_solver/FFTPoissonSolver.cpp | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/fields/fft_poisson_solver/FFTPoissonSolver.H b/src/fields/fft_poisson_solver/FFTPoissonSolver.H index 48106ed27b..842d08ff17 100644 --- a/src/fields/fft_poisson_solver/FFTPoissonSolver.H +++ b/src/fields/fft_poisson_solver/FFTPoissonSolver.H @@ -31,7 +31,7 @@ public: FFTPoissonSolver () = default; /** Abstract class needs a virtual destructor */ - virtual ~FFTPoissonSolver () = 0; + virtual ~FFTPoissonSolver () = default; /** * Solve Poisson equation. The source term must be stored in the staging area m_stagingArea prior to this call. diff --git a/src/fields/fft_poisson_solver/FFTPoissonSolver.cpp b/src/fields/fft_poisson_solver/FFTPoissonSolver.cpp index 57af458411..40effaf0a9 100644 --- a/src/fields/fft_poisson_solver/FFTPoissonSolver.cpp +++ b/src/fields/fft_poisson_solver/FFTPoissonSolver.cpp @@ -7,9 +7,6 @@ */ #include "FFTPoissonSolver.H" -FFTPoissonSolver::~FFTPoissonSolver () -{} - amrex::MultiFab& FFTPoissonSolver::StagingArea () { From 4cad3e604757ff8b6d2e2151df81f8a29e85e477 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 27 Oct 2025 14:51:26 +0100 Subject: [PATCH 02/33] add tool to run clang-tidy locally --- .clang-tidy | 6 +++ tools/runClangTidy.sh | 107 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 .clang-tidy create mode 100755 tools/runClangTidy.sh diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000000..4e7f5394e4 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,6 @@ +Checks: ' + -*, + cppcoreguidelines-avoid-goto + ' + +HeaderFilterRegex: 'src[a-z_A-Z0-9\/]+\.H$' diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh new file mode 100755 index 0000000000..df0068d171 --- /dev/null +++ b/tools/runClangTidy.sh @@ -0,0 +1,107 @@ +#! /usr/bin/env bash + +# Copyright 2025 +# +# This file is part of HiPACE++. +# +# Authors: Luca Fedeli +# License: BSD-3-Clause-LBNL + +# This script is a developer's tool to perform +# checks with clang-tidy. +# +# Note: this script is only tested on Linux + +echo "=============================================" +echo +echo "This script is a developer's tool to perform" +echo "checks with clang-tidy." +echo "_____________________________________________" + +# Check source dir +REPO_DIR=$(cd $(dirname ${BASH_SOURCE})/../ && pwd) +echo +echo "Your current source directory is: ${REPO_DIR}" +echo "_____________________________________________" + +# Set number of jobs to use for compilation +PARALLEL="${WARPX_TOOLS_LINTER_PARALLEL:-4}" +echo +echo "${PARALLEL} jobs will be used for compilation." +echo "This can be overridden by setting the environment" +echo "variable HIPACE_TOOLS_LINTER_PARALLEL, e.g.: " +echo +echo "$ export HIPACE_TOOLS_LINTER_PARALLEL=8" +echo "$ ./tools/runClangTidy.sh" +echo "_____________________________________________" + +# Check clang version +export CC="${CLANG:-"clang"}" +export CXX="${CLANGXX:-"clang++"}" +export CTIDY="${CLANGTIDY:-"clang-tidy"}" +echo +echo "The following versions of the clang compiler and" +echo "of the clang-tidy linter will be used:" +echo +echo "clang version:" +which ${CC} +${CC} --version +echo +echo "clang++ version:" +which ${CXX} +${CXX} --version +echo +echo "clang-tidy version:" +which ${CTIDY} +${CTIDY} --version +echo +echo "This can be overridden by setting the environment" +echo "variables CLANG, CLANGXX, and CLANGTIDY e.g.: " +echo "$ export CLANG=clang-20" +echo "$ export CLANGXX=clang++-20" +echo "$ export CTIDCLANGTIDYY=clang-tidy-20" +echo "$ ./tools/runClangTidy.sh" +echo "_____________________________________________" + +# Prepare clang-tidy wrapper +echo +echo "Prepare clang-tidy wrapper" +echo "The following wrapper ensures that only source files" +echo "in hipace/src/* are actually processed by clang-tidy" +echo +cat > ${REPO_DIR}/clang_tidy_wrapper << EOF +#!/bin/bash +REGEX="[a-z_A-Z0-9\/]*hipace\/src[a-z_A-Z0-9\/]+.cpp" +if [[ \$4 =~ \$REGEX ]];then + ${CTIDY} \$@ +fi +EOF +chmod +x ${REPO_DIR}/clang_tidy_wrapper +echo "clang_tidy_wrapper: " +cat ${REPO_DIR}/clang_tidy_wrapper +echo "_____________________________________________" + +# Compile HiPACE++ using clang-tidy +echo +echo "*******************************************" +echo "* Compile HiPACE++ using clang-tidy *" +echo "* Please ensure that all the dependencies *" +echo "* required to compile HiPACE++ are met *" +echo "*******************************************" +echo + +rm -rf ${REPO_DIR}/build_clang_tidy + +cmake -S ${REPO_DIR} -B ${REPO_DIR}/build_clang_tidy \ + -DCMAKE_CXX_CLANG_TIDY="${REPO_DIR}/clang_tidy_wrapper;--system-headers=0;--config-file=${REPO_DIR}/.clang-tidy" \ + -DCMAKE_VERBOSE_MAKEFILE=ON \ + -DHiPACE_MPI=ON \ + -DHiPACE_COMPUTE=OMP \ + -DHiPACE_OPENPMD=ON \ + -DHiPACE_PRECISION=SINGLE + +cmake --build ${REPO_DIR}/build_clang_tidy -j ${PARALLEL} 2> ${REPO_DIR}/build_clang_tidy/clang-tidy.log + +cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log +echo +echo "=============================================" \ No newline at end of file From 18077c4b674b0a8a6691e65699c2d26acce312cf Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Tue, 28 Oct 2025 11:59:57 +0100 Subject: [PATCH 03/33] Update tools/runClangTidy.sh --- tools/runClangTidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh index df0068d171..ceae658936 100755 --- a/tools/runClangTidy.sh +++ b/tools/runClangTidy.sh @@ -25,7 +25,7 @@ echo "Your current source directory is: ${REPO_DIR}" echo "_____________________________________________" # Set number of jobs to use for compilation -PARALLEL="${WARPX_TOOLS_LINTER_PARALLEL:-4}" +PARALLEL="${HIPACE_TOOLS_LINTER_PARALLEL:-4}" echo echo "${PARALLEL} jobs will be used for compilation." echo "This can be overridden by setting the environment" From a44e221c6a677f04f2dd892e78d7b9aaf75165bc Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 24 Nov 2025 11:21:22 +0100 Subject: [PATCH 04/33] add clang-tidy in CI --- .github/workflows/clangtidy.yml | 23 +++++++++++++++++++++++ .github/workflows/linux.yml | 19 ------------------- .github/workflows/setup/ubuntu_clang.sh | 3 ++- 3 files changed, 25 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/clangtidy.yml diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml new file mode 100644 index 0000000000..7d4fd727dd --- /dev/null +++ b/.github/workflows/clangtidy.yml @@ -0,0 +1,23 @@ + +name: ✨🧹✨ clang-tidy + +on: + push: + branches: + - development + pull_request: + branches: + - development + +jobs: + + linux_clang_tidy: + name: clang-tidy + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Dependencies + run: .github/workflows/setup/ubuntu_clang.sh + - name: Build & Install + run: | + ./tools/runClangTidy.sh diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 7fa33065e2..236f2d615e 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -78,22 +78,3 @@ jobs: cmake --build build -j 2 - name: Run Tests run: ctest --test-dir build --output-on-failure - -# enable again when we open-source (free core-hours for GH Actions) -# -# linux_clang7: -# name: Clang@7 C++17 OMPI -# runs-on: ubuntu-latest -# steps: -# - uses: actions/checkout@v4 -# - name: Dependencies -# run: .github/workflows/setup/ubuntu_clang.sh -# - name: Build & Install -# run: | -# mkdir build -# cd build -# cmake .. \ -# -DCMAKE_C_COMPILER=$(which clang-7) \ -# -DCMAKE_CXX_COMPILER=$(which clang++-7) -# make -j 2 VERBOSE=ON -# ctest --output-on-failure diff --git a/.github/workflows/setup/ubuntu_clang.sh b/.github/workflows/setup/ubuntu_clang.sh index 59ca005186..1513fb075f 100755 --- a/.github/workflows/setup/ubuntu_clang.sh +++ b/.github/workflows/setup/ubuntu_clang.sh @@ -19,6 +19,7 @@ sudo apt-get update sudo apt-get install -y --no-install-recommends \ build-essential \ ccache \ - clang-7 \ + clang \ + clang-tidy \ libopenmpi-dev \ openmpi-bin From 8d816676249bb117ccfcae51db5692664badea5e Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 24 Nov 2025 11:29:53 +0100 Subject: [PATCH 05/33] add missing dependency --- .github/workflows/setup/ubuntu_clang.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/setup/ubuntu_clang.sh b/.github/workflows/setup/ubuntu_clang.sh index 1513fb075f..4b7e91c9a8 100755 --- a/.github/workflows/setup/ubuntu_clang.sh +++ b/.github/workflows/setup/ubuntu_clang.sh @@ -21,5 +21,6 @@ sudo apt-get install -y --no-install-recommends \ ccache \ clang \ clang-tidy \ + libomp-dev \ libopenmpi-dev \ openmpi-bin From 77e124ab80960c61bbc932fccee015bc53dd051e Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 24 Nov 2025 11:38:55 +0100 Subject: [PATCH 06/33] update runClangTidy script --- tools/runClangTidy.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh index ceae658936..66a4ead3cf 100755 --- a/tools/runClangTidy.sh +++ b/tools/runClangTidy.sh @@ -12,6 +12,8 @@ # # Note: this script is only tested on Linux +set -eu -o pipefail + echo "=============================================" echo echo "This script is a developer's tool to perform" @@ -104,4 +106,4 @@ cmake --build ${REPO_DIR}/build_clang_tidy -j ${PARALLEL} 2> ${REPO_DIR}/build_c cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log echo -echo "=============================================" \ No newline at end of file +echo "=============================================" From a169d146c87b6391c52ab480be811fac17ea4b04 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 24 Nov 2025 11:43:14 +0100 Subject: [PATCH 07/33] add missing dependency --- .github/workflows/setup/ubuntu_clang.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/setup/ubuntu_clang.sh b/.github/workflows/setup/ubuntu_clang.sh index 4b7e91c9a8..b96e76ab84 100755 --- a/.github/workflows/setup/ubuntu_clang.sh +++ b/.github/workflows/setup/ubuntu_clang.sh @@ -23,4 +23,5 @@ sudo apt-get install -y --no-install-recommends \ clang-tidy \ libomp-dev \ libopenmpi-dev \ + fftw-dev \ openmpi-bin From cc675d9ce5aebf2afbb980d3914e05bbf46ea2ce Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 24 Nov 2025 11:45:54 +0100 Subject: [PATCH 08/33] add missing dependency --- .github/workflows/setup/ubuntu_clang.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/setup/ubuntu_clang.sh b/.github/workflows/setup/ubuntu_clang.sh index b96e76ab84..77f65e539d 100755 --- a/.github/workflows/setup/ubuntu_clang.sh +++ b/.github/workflows/setup/ubuntu_clang.sh @@ -23,5 +23,6 @@ sudo apt-get install -y --no-install-recommends \ clang-tidy \ libomp-dev \ libopenmpi-dev \ - fftw-dev \ - openmpi-bin + openmpi-bin \ + libfftw3-dev \ + libfftw3-single3 From 8674d768cf9d30d7d58157dfde7b7d5168e157e5 Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 14:57:18 +0100 Subject: [PATCH 09/33] silence two warnings --- src/mg_solver/HpMultiGrid.H | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mg_solver/HpMultiGrid.H b/src/mg_solver/HpMultiGrid.H index 5cf1c67f84..68a5b56964 100644 --- a/src/mg_solver/HpMultiGrid.H +++ b/src/mg_solver/HpMultiGrid.H @@ -230,8 +230,11 @@ private: int m_num_mg_levels; /** Number of single-block-kernel levels */ int m_num_single_block_levels; + +#if defined(AMREX_USE_GPU) /** If the single block kernel should be used */ bool m_use_single_block_kernel = true; +#endif /** Alias to the solution argument passed in solve() */ amrex::FArrayBox m_sol; From 99e0b9a85eba082367ae3960dfd707423c821f6d Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 14:57:49 +0100 Subject: [PATCH 10/33] disable vectorization warnings in CI --- .github/workflows/clangtidy.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml index 7d4fd727dd..d0f58aa385 100644 --- a/.github/workflows/clangtidy.yml +++ b/.github/workflows/clangtidy.yml @@ -14,6 +14,7 @@ jobs: linux_clang_tidy: name: clang-tidy runs-on: ubuntu-latest + env: {CXXFLAGS: "-Wno-error=pass-failed"} steps: - uses: actions/checkout@v4 - name: Dependencies From 949465fe7aad6114fbf6f66132c94ea154e54396 Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:08:22 +0100 Subject: [PATCH 11/33] disable vectorization errors also for the linking step --- .github/workflows/clangtidy.yml | 2 +- src/mg_solver/HpMultiGrid.cpp | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml index d0f58aa385..65e07bc4a2 100644 --- a/.github/workflows/clangtidy.yml +++ b/.github/workflows/clangtidy.yml @@ -14,7 +14,7 @@ jobs: linux_clang_tidy: name: clang-tidy runs-on: ubuntu-latest - env: {CXXFLAGS: "-Wno-error=pass-failed"} + env: {CXXFLAGS: "-Wno-error=pass-failed", LDFLAGS: "-Wno-error=pass-failed"} steps: - uses: actions/checkout@v4 - name: Dependencies diff --git a/src/mg_solver/HpMultiGrid.cpp b/src/mg_solver/HpMultiGrid.cpp index 2ffb2ef215..ba9a52d9de 100644 --- a/src/mg_solver/HpMultiGrid.cpp +++ b/src/mg_solver/HpMultiGrid.cpp @@ -15,7 +15,9 @@ namespace hpmg { namespace { -constexpr int n_cell_single = 32; // switch to single block when box is smaller than this +#if defined(AMREX_USE_GPU) + constexpr int n_cell_single = 32; // switch to single block when box is smaller than this +#endif Box valid_domain_box (Box const& domain) { From dafc96243cfd8f05159beaa8bac77be611414a49 Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:19:07 +0100 Subject: [PATCH 12/33] fix env. variables bug --- .github/workflows/clangtidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml index 65e07bc4a2..2ddc4fabca 100644 --- a/.github/workflows/clangtidy.yml +++ b/.github/workflows/clangtidy.yml @@ -14,7 +14,7 @@ jobs: linux_clang_tidy: name: clang-tidy runs-on: ubuntu-latest - env: {CXXFLAGS: "-Wno-error=pass-failed", LDFLAGS: "-Wno-error=pass-failed"} + env: {CXXFLAGS: "-Wno-pass-failed"} steps: - uses: actions/checkout@v4 - name: Dependencies From 6238b6e2dd243ec6827f4ef232d949b5cb90380a Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:33:40 +0100 Subject: [PATCH 13/33] add clearer messages and exit with code 1 if clang-tidy finds issues --- .github/workflows/clangtidy.yml | 1 + tools/runClangTidy.sh | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml index 2ddc4fabca..c23b6c2298 100644 --- a/.github/workflows/clangtidy.yml +++ b/.github/workflows/clangtidy.yml @@ -22,3 +22,4 @@ jobs: - name: Build & Install run: | ./tools/runClangTidy.sh + diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh index 66a4ead3cf..71b44faac1 100755 --- a/tools/runClangTidy.sh +++ b/tools/runClangTidy.sh @@ -104,6 +104,20 @@ cmake -S ${REPO_DIR} -B ${REPO_DIR}/build_clang_tidy \ cmake --build ${REPO_DIR}/build_clang_tidy -j ${PARALLEL} 2> ${REPO_DIR}/build_clang_tidy/clang-tidy.log -cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log -echo -echo "=============================================" +if [ -f ${REPO_DIR}/build_clang_tidy/clang-tidy.log ]; then + echo + echo "clang-tidy found the following issues:" + echo + cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log + echo + echo "=============================================" + exit 1 +else + echo + echo "clang-tidy has not found any issue." + echo + echo "=============================================" + exit 0 +fi + + From e8b0780b21fd12de2d9a30a85800d891171608fd Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:42:07 +0100 Subject: [PATCH 14/33] check file size --- tools/runClangTidy.sh | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh index 71b44faac1..ba89304a3f 100755 --- a/tools/runClangTidy.sh +++ b/tools/runClangTidy.sh @@ -104,20 +104,18 @@ cmake -S ${REPO_DIR} -B ${REPO_DIR}/build_clang_tidy \ cmake --build ${REPO_DIR}/build_clang_tidy -j ${PARALLEL} 2> ${REPO_DIR}/build_clang_tidy/clang-tidy.log -if [ -f ${REPO_DIR}/build_clang_tidy/clang-tidy.log ]; then +if [ -s ${REPO_DIR}/build_clang_tidy/clang-tidy.log ]; then echo - echo "clang-tidy found the following issues:" - echo - cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log + echo "clang-tidy has not found any issue." echo echo "=============================================" - exit 1 + exit 0 else echo - echo "clang-tidy has not found any issue." + echo "clang-tidy found the following issues:" + echo + cat ${REPO_DIR}/build_clang_tidy/clang-tidy.log echo echo "=============================================" - exit 0 + exit 1 fi - - From 0d5d04d2542b9fa7f0cf1953c5c946a93ee7908c Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:46:35 +0100 Subject: [PATCH 15/33] add checks of the cert-* family --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 4e7f5394e4..5724d2951d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,5 +1,6 @@ Checks: ' -*, + cert-*, cppcoreguidelines-avoid-goto ' From 8555a3dd8b2e9b83ceafb4121ab0b57ff56526b8 Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:52:52 +0100 Subject: [PATCH 16/33] fix bug --- tools/runClangTidy.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/runClangTidy.sh b/tools/runClangTidy.sh index ba89304a3f..4e0c9bcf1e 100755 --- a/tools/runClangTidy.sh +++ b/tools/runClangTidy.sh @@ -105,12 +105,6 @@ cmake -S ${REPO_DIR} -B ${REPO_DIR}/build_clang_tidy \ cmake --build ${REPO_DIR}/build_clang_tidy -j ${PARALLEL} 2> ${REPO_DIR}/build_clang_tidy/clang-tidy.log if [ -s ${REPO_DIR}/build_clang_tidy/clang-tidy.log ]; then - echo - echo "clang-tidy has not found any issue." - echo - echo "=============================================" - exit 0 -else echo echo "clang-tidy found the following issues:" echo @@ -118,4 +112,10 @@ else echo echo "=============================================" exit 1 +else + echo + echo "clang-tidy has not found any issue." + echo + echo "=============================================" + exit 0 fi From 5826dee4a1019191b9931b4beaec363dc12224fd Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 15:58:06 +0100 Subject: [PATCH 17/33] only keep cert-err58-cpp for the moment --- .clang-tidy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 5724d2951d..f43cecd647 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,6 @@ Checks: ' -*, - cert-*, + cert-err58-cpp, cppcoreguidelines-avoid-goto ' From 542090f500efcab81f99d98d50372b40ceed45c8 Mon Sep 17 00:00:00 2001 From: lucafedeli88 Date: Mon, 24 Nov 2025 16:11:17 +0100 Subject: [PATCH 18/33] remove cert checks and enable most checks of the bugprone family --- .clang-tidy | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index f43cecd647..67624473bd 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,6 +1,9 @@ Checks: ' -*, - cert-err58-cpp, + bugprone-*, + -bugprone-easily-swappable-parameters, + -bugprone-implicit-widening-of-multiplication-result, + -bugprone-misplaced-widening-cast, cppcoreguidelines-avoid-goto ' From 884c2b582e07f853ddd54dc0999d0c6f33e3229b Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 14:21:00 +0100 Subject: [PATCH 19/33] update rules --- .clang-tidy | 1 + 1 file changed, 1 insertion(+) diff --git a/.clang-tidy b/.clang-tidy index 67624473bd..2f0e8077f7 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -4,6 +4,7 @@ Checks: ' -bugprone-easily-swappable-parameters, -bugprone-implicit-widening-of-multiplication-result, -bugprone-misplaced-widening-cast, + -bugprone-narrowing-conversions, cppcoreguidelines-avoid-goto ' From b05306f3c6d67b6299ab1b10d622e722a86c4360 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 14:21:16 +0100 Subject: [PATCH 20/33] implement fixes --- src/mg_solver/HpMultiGrid.H | 8 ++++++-- src/utils/MultiBuffer.cpp | 13 +++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/mg_solver/HpMultiGrid.H b/src/mg_solver/HpMultiGrid.H index 68a5b56964..f8c0ca9d39 100644 --- a/src/mg_solver/HpMultiGrid.H +++ b/src/mg_solver/HpMultiGrid.H @@ -185,8 +185,10 @@ public: return 2; case 3: return 1; + default: + amrex::Abort(std::to_string(system_type) + " is not a valid system_type!"); } - return 0; + return 0; //unreachable } /** \brief Return the number of acf components used for a given system type @@ -201,8 +203,10 @@ public: return 2; case 3: return 0; + default: + amrex::Abort(std::to_string(system_type) + " is not a valid system_type!"); } - return 0; + return 0; //unreachable } /** When applying Dirichlet boundary conditions, shift boundary value by offset number of cells */ diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index a5e1474d02..1cb1f20b2e 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -807,9 +807,14 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated - memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), + if (bo.m_beam_idcpu[b].has_value()){ + memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); + } + else{ + amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) "] has no value!"); + } } for (int rcomp = 0; rcomp < beam.numRealComponents(); ++rcomp) { @@ -824,9 +829,13 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int for (int icomp = 0; icomp < beam.numIntComponents(); ++icomp) { // only pack int component if it should be communicated if (beam.communicateIntComponent(icomp)) { - memcpy_to_buffer(slice, bo.m_beam_int[b].at(icomp), + if (bo.m_beam_idcpu[b].has_value()){ + memcpy_to_buffer(slice, bo.m_beam_int[b].at(icomp), soa.GetIntData(icomp).dataPtr(), num_particles * sizeof(int)); + else{ + amrex::Abort("bo.m_beam_int[" + std::to_string(b) "] has no value!"); + } } } } From f308ac32dd58fa7096de60e2019c936f556fa248 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 14:23:07 +0100 Subject: [PATCH 21/33] fix bug introduced in previous commit --- src/utils/MultiBuffer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index 1cb1f20b2e..9c6ed78f72 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -813,7 +813,7 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int num_particles * sizeof(std::uint64_t)); } else{ - amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) "] has no value!"); + amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) + "] has no value!"); } } @@ -834,7 +834,7 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int soa.GetIntData(icomp).dataPtr(), num_particles * sizeof(int)); else{ - amrex::Abort("bo.m_beam_int[" + std::to_string(b) "] has no value!"); + amrex::Abort("bo.m_beam_int[" + std::to_string(b) + "] has no value!"); } } } From e7699873e292417e11d88a868f4ac6778d1cc9a7 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 14:30:16 +0100 Subject: [PATCH 22/33] fix bug introduced in previous commit --- src/utils/MultiBuffer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index 9c6ed78f72..ed71b717d5 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -833,6 +833,7 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int memcpy_to_buffer(slice, bo.m_beam_int[b].at(icomp), soa.GetIntData(icomp).dataPtr(), num_particles * sizeof(int)); + } else{ amrex::Abort("bo.m_beam_int[" + std::to_string(b) + "] has no value!"); } From a5b63aebdf5d82ff5c6253b2dabe011c115e8773 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 14:54:54 +0100 Subject: [PATCH 23/33] remove unused phys_const from 2 files --- src/particles/beam/BeamParticleContainer.cpp | 1 - src/particles/plasma/PlasmaParticleContainer.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/src/particles/beam/BeamParticleContainer.cpp b/src/particles/beam/BeamParticleContainer.cpp index 04e716acb1..a7d4cdb88d 100644 --- a/src/particles/beam/BeamParticleContainer.cpp +++ b/src/particles/beam/BeamParticleContainer.cpp @@ -504,7 +504,6 @@ BeamParticleContainer::InSituComputeDiags (int islice) m_insitu_sum_rdata.size()>0 && m_insitu_sum_idata.size()>0); const amrex::Real insitu_radius_sq = m_insitu_radius * m_insitu_radius; - const PhysConst phys_const = get_phys_const(); const auto ptd = getBeamSlice(WhichBeamSlice::This).getParticleTileData(); amrex::TypeMultiplier reduce_op; diff --git a/src/particles/plasma/PlasmaParticleContainer.cpp b/src/particles/plasma/PlasmaParticleContainer.cpp index 3c0558be59..5ea34cef22 100644 --- a/src/particles/plasma/PlasmaParticleContainer.cpp +++ b/src/particles/plasma/PlasmaParticleContainer.cpp @@ -835,7 +835,6 @@ PlasmaParticleContainer::InSituComputeDiags (int islice) m_insitu_sum_rdata.size()>0 && m_insitu_sum_idata.size()>0); const amrex::Real insitu_radius_sq = m_insitu_radius * m_insitu_radius; - const PhysConst phys_const = get_phys_const(); // Loop over particle boxes for (PlasmaParticleIterator pti(*this); pti.isValid(); ++pti) From ccda6b68b59e1ce506f14d6963f37cb01c24fa50 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 15:01:52 +0100 Subject: [PATCH 24/33] fix bug --- src/utils/MultiBuffer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index ed71b717d5..d1a8fd52e1 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -808,7 +808,7 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated if (bo.m_beam_idcpu[b].has_value()){ - memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), + memcpy_to_buffer(slice, *bo.m_beam_idcpu[b], soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); } @@ -829,8 +829,8 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int for (int icomp = 0; icomp < beam.numIntComponents(); ++icomp) { // only pack int component if it should be communicated if (beam.communicateIntComponent(icomp)) { - if (bo.m_beam_idcpu[b].has_value()){ - memcpy_to_buffer(slice, bo.m_beam_int[b].at(icomp), + if (bo.m_beam_int[b].has_value()){ + memcpy_to_buffer(slice, (*bo.m_beam_int[b]).at(icomp), soa.GetIntData(icomp).dataPtr(), num_particles * sizeof(int)); } From 2ff0b946ba4783ee90bf1478e93514ac115a4c80 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 15:06:31 +0100 Subject: [PATCH 25/33] fix bug --- src/utils/MultiBuffer.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index d1a8fd52e1..ad58debe76 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -829,14 +829,9 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int for (int icomp = 0; icomp < beam.numIntComponents(); ++icomp) { // only pack int component if it should be communicated if (beam.communicateIntComponent(icomp)) { - if (bo.m_beam_int[b].has_value()){ - memcpy_to_buffer(slice, (*bo.m_beam_int[b]).at(icomp), + memcpy_to_buffer(slice, bo.m_beam_int[b].at(icomp), soa.GetIntData(icomp).dataPtr(), num_particles * sizeof(int)); - } - else{ - amrex::Abort("bo.m_beam_int[" + std::to_string(b) + "] has no value!"); - } } } } @@ -871,9 +866,14 @@ void MultiBuffer::unpack_data (int slice, MultiBeam& beams, MultiLaser& laser, i if (beam.communicateIdCpuComponent()) { // only undpack idcpu component if it should be communicated - memcpy_from_buffer(slice, bo.m_beam_idcpu[b].value(), + if (bo.m_beam_idcpu[b].has_value()){ + memcpy_from_buffer(slice, *bo.m_beam_idcpu[b], soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); + } + else { + amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) + "] has no value!"); + } } else { // if idcpu is not communicated, then we need to initialize it here std::uint64_t* data_ptr = soa.GetIdCPUData().dataPtr(); From fccf1148db5540283267831d98dd2885d3520097 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 15:25:54 +0100 Subject: [PATCH 26/33] address issue found with clang-tidy --- src/utils/MultiBuffer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index ad58debe76..e4834a3ada 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -807,8 +807,8 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated - if (bo.m_beam_idcpu[b].has_value()){ - memcpy_to_buffer(slice, *bo.m_beam_idcpu[b], + if (const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; bo.m_beam_idcpu[b].has_value()){ + memcpy_to_buffer(slice, *bo_m_beam_idcpu_b, soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); } @@ -866,8 +866,8 @@ void MultiBuffer::unpack_data (int slice, MultiBeam& beams, MultiLaser& laser, i if (beam.communicateIdCpuComponent()) { // only undpack idcpu component if it should be communicated - if (bo.m_beam_idcpu[b].has_value()){ - memcpy_from_buffer(slice, *bo.m_beam_idcpu[b], + if (const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; bo.m_beam_idcpu[b].has_value()){ + memcpy_from_buffer(slice, *bo_m_beam_idcpu_b, soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); } From 0af034b0180656a64bbb22773274901d7dda3061 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Wed, 26 Nov 2025 15:45:16 +0100 Subject: [PATCH 27/33] change how the has_value check is done --- src/utils/MultiBuffer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index e4834a3ada..b31a292d20 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -807,7 +807,8 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated - if (const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; bo.m_beam_idcpu[b].has_value()){ + const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; + if (bo_m_beam_idcpu_b.has_value()){ memcpy_to_buffer(slice, *bo_m_beam_idcpu_b, soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); @@ -866,7 +867,8 @@ void MultiBuffer::unpack_data (int slice, MultiBeam& beams, MultiLaser& laser, i if (beam.communicateIdCpuComponent()) { // only undpack idcpu component if it should be communicated - if (const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; bo.m_beam_idcpu[b].has_value()){ + const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; + if (bo_m_beam_idcpu_b.has_value()){ memcpy_from_buffer(slice, *bo_m_beam_idcpu_b, soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); From 5ff5a6b3a5800140ae63246f9f83da179af15fe9 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 17:29:05 +0100 Subject: [PATCH 28/33] Update .github/workflows/clangtidy.yml Co-authored-by: Alexander Sinn <64009254+AlexanderSinn@users.noreply.github.com> --- .github/workflows/clangtidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clangtidy.yml b/.github/workflows/clangtidy.yml index c23b6c2298..bcee08174c 100644 --- a/.github/workflows/clangtidy.yml +++ b/.github/workflows/clangtidy.yml @@ -1,5 +1,5 @@ -name: ✨🧹✨ clang-tidy +name: 🧹 clang-tidy on: push: From f88aac3133b8822d74be11bda0d75490d4e8ae65 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 17:40:04 +0100 Subject: [PATCH 29/33] revert changes in MultiBuffer.cpp --- src/utils/MultiBuffer.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index b31a292d20..a5e1474d02 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -807,15 +807,9 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated - const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; - if (bo_m_beam_idcpu_b.has_value()){ - memcpy_to_buffer(slice, *bo_m_beam_idcpu_b, + memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); - } - else{ - amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) + "] has no value!"); - } } for (int rcomp = 0; rcomp < beam.numRealComponents(); ++rcomp) { @@ -867,15 +861,9 @@ void MultiBuffer::unpack_data (int slice, MultiBeam& beams, MultiLaser& laser, i if (beam.communicateIdCpuComponent()) { // only undpack idcpu component if it should be communicated - const auto& bo_m_beam_idcpu_b = bo.m_beam_idcpu[b]; - if (bo_m_beam_idcpu_b.has_value()){ - memcpy_from_buffer(slice, *bo_m_beam_idcpu_b, + memcpy_from_buffer(slice, bo.m_beam_idcpu[b].value(), soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); - } - else { - amrex::Abort("bo.m_beam_idcpu[" + std::to_string(b) + "] has no value!"); - } } else { // if idcpu is not communicated, then we need to initialize it here std::uint64_t* data_ptr = soa.GetIdCPUData().dataPtr(); From 3d70f1f8e169d538bdbb12ee7fa2b285f95d73d4 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 17:57:45 +0100 Subject: [PATCH 30/33] revert last change to .clang-tidy --- .clang-tidy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index 2f0e8077f7..a1b4425bc6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -8,4 +8,8 @@ Checks: ' cppcoreguidelines-avoid-goto ' +CheckOptions: +- key: bugprone-unchecked-optional-access.IgnoreValueCalls + value: "true" + HeaderFilterRegex: 'src[a-z_A-Z0-9\/]+\.H$' From 07639ad9e17bb9acbb8db9be541f78c53ffadb51 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 18:00:29 +0100 Subject: [PATCH 31/33] delete check option --- .clang-tidy | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index a1b4425bc6..2f0e8077f7 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -8,8 +8,4 @@ Checks: ' cppcoreguidelines-avoid-goto ' -CheckOptions: -- key: bugprone-unchecked-optional-access.IgnoreValueCalls - value: "true" - HeaderFilterRegex: 'src[a-z_A-Z0-9\/]+\.H$' From e2db7560213dd0135dc6ed348a443b63a0c63d21 Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 18:07:44 +0100 Subject: [PATCH 32/33] use NOLINT --- src/utils/MultiBuffer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/MultiBuffer.cpp b/src/utils/MultiBuffer.cpp index a5e1474d02..eaae5addcf 100644 --- a/src/utils/MultiBuffer.cpp +++ b/src/utils/MultiBuffer.cpp @@ -807,7 +807,7 @@ void MultiBuffer::pack_data (int slice, MultiBeam& beams, MultiLaser& laser, int if (beam.communicateIdCpuComponent()) { // only pack idcpu component if it should be communicated - memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), + memcpy_to_buffer(slice, bo.m_beam_idcpu[b].value(), // NOLINT(bugprone-unchecked-optional-access) soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); } @@ -861,7 +861,7 @@ void MultiBuffer::unpack_data (int slice, MultiBeam& beams, MultiLaser& laser, i if (beam.communicateIdCpuComponent()) { // only undpack idcpu component if it should be communicated - memcpy_from_buffer(slice, bo.m_beam_idcpu[b].value(), + memcpy_from_buffer(slice, bo.m_beam_idcpu[b].value(), // NOLINT(bugprone-unchecked-optional-access) soa.GetIdCPUData().dataPtr(), num_particles * sizeof(std::uint64_t)); } else { From 929df6834bc732f2f2caa279e931d1fea5001c3d Mon Sep 17 00:00:00 2001 From: Luca Fedeli Date: Mon, 8 Dec 2025 18:13:01 +0100 Subject: [PATCH 33/33] add comment to clang-tidy configuration file --- .clang-tidy | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index 2f0e8077f7..87e0bdc5dc 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,3 +1,11 @@ +# Enabled checks: +# +# - all the checks of the bugprone-* group except: +# easily-swappable-parameters, implicit-widening-of-multiplication-result, +# misplaced-widening-cast, narrowing-conversions +# +# - cppcoreguidelines-avoid-goto +# Checks: ' -*, bugprone-*,