From 51e3b20f777fa9e15df9e38b79b67a6c76c4d662 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 14:04:38 -0400 Subject: [PATCH 01/18] test: save build commands suitable for run-clang-tidy Also tidy up an unneeded line --- devel-tools/build_test_library.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devel-tools/build_test_library.cmake b/devel-tools/build_test_library.cmake index b0c7677f0..6d8bc0bed 100644 --- a/devel-tools/build_test_library.cmake +++ b/devel-tools/build_test_library.cmake @@ -130,6 +130,7 @@ execute_process( -D BUILD_SHARED_LIBS=${BUILD_SHARED_LIBS} -D WARNINGS_ARE_ERRORS=ON -D CMAKE_VERBOSE_MAKEFILE=ON + -D CMAKE_EXPORT_COMPILE_COMMANDS=ON -D CMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD} ${DEFINE_CXX_COMPILER} ${DEFINE_CC_CCACHE} @@ -142,7 +143,6 @@ execute_process( ${DEFINE_TORCH_PREFIX} -D COLVARS_LEPTON=${COLVARS_LEPTON} -D LEPTON_DIR=${LEPTON_DIR} - -D CMAKE_PREFIX_PATH="/opt/libtorch/share/cmake" RESULT_VARIABLE result ) From 89edfe907465a775b676fb8410f6026dc104e1ba Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 14:13:25 -0400 Subject: [PATCH 02/18] Add clang-tidy CI job --- .github/workflows/test-library.yml | 50 ++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index c9c16c477..965ee5d72 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -225,6 +225,56 @@ jobs: path: warnings_analyzer.log + build-linux-x86_64-clang-tidy: + name: Linux x86_64 (Clang tidy) + runs-on: ubuntu-latest + needs: basicchecks + + steps: + - uses: actions/checkout@v4 + + - name: Checkout OpenMM (for Lepton library) + uses: actions/checkout@v4 + with: + repository: 'openmm/openmm' + ref: 'master' + path: 'openmm-source' + + - name: Install Apptainer + shell: bash + run: | + sudo add-apt-repository -y ppa:apptainer/ppa + sudo apt update + sudo apt install -y apptainer-suid + + - name: Get container images for build dependencies + shell: bash + working-directory: devel-tools + run: | + apptainer pull CentOS9-devel.sif oras://ghcr.io/colvars/devel-containers:CentOS9-devel + + - name: Configure and build the library (but do not run tests) + env: + CXX: clang++ + CC: clang + run: | + apptainer exec ${{github.workspace}}/devel-tools/CentOS9-devel.sif \ + cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake + + - name: Run "run-clang-tidy" in the "build" folder + working-directory: build + run: | + apptainer exec ${{github.workspace}}/devel-tools/CentOS9-devel.sif \ + run-clang-tidy -header-filter=.* -warnings-as-errors \* | tee clang-tidy.log + + - name: Archive warnings artifacts + uses: actions/upload-artifact@v4 + if: failure() + with: + name: clang_tidy_log + path: build/clang-tidy.log + + address-sanitizer: name: Linux x86_64 (Clang address sanitizer) runs-on: ubuntu-latest From 94fc7501e65c167d2c52fedc62812f853dec6624 Mon Sep 17 00:00:00 2001 From: HanatoK Date: Tue, 9 Sep 2025 13:48:55 -0500 Subject: [PATCH 03/18] fix: add use_all enum to rotation_derivative_dldq This commit should fix the clang-tidy warning. --- src/colvar_rotation_derivative.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/colvar_rotation_derivative.h b/src/colvar_rotation_derivative.h index 154114f28..78ec2e92a 100644 --- a/src/colvar_rotation_derivative.h +++ b/src/colvar_rotation_derivative.h @@ -21,7 +21,9 @@ enum class rotation_derivative_dldq { /// Require the derivative of the leading eigenvalue with respect to the atom coordinates use_dl = 1 << 0, /// Require the derivative of the leading eigenvector with respect to the atom coordinates - use_dq = 1 << 1 + use_dq = 1 << 1, + /// Require the derivative of both L and Q + use_all = (use_dl) + (use_dq) }; inline constexpr rotation_derivative_dldq operator|(rotation_derivative_dldq Lhs, rotation_derivative_dldq Rhs) { From fcba32b21843ed0b5f3910a0de5ae27e95af25b4 Mon Sep 17 00:00:00 2001 From: HanatoK Date: Tue, 9 Sep 2025 13:56:00 -0500 Subject: [PATCH 04/18] fix: fix the uninitialized variable m_height in colvarbias_opes::kernel --- src/colvarbias_opes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/colvarbias_opes.h b/src/colvarbias_opes.h index 0c52ba241..697569dd3 100644 --- a/src/colvarbias_opes.h +++ b/src/colvarbias_opes.h @@ -35,7 +35,7 @@ class colvarbias_opes: public colvarbias { cvm::real m_height; std::vector m_center; std::vector m_sigma; - kernel() {} + kernel(): m_height(0) {} kernel(cvm::real h, const std::vector& c, const std::vector& s): m_height(h), m_center(c), m_sigma(s) {} From d25d765336c718be5382ab98109df78cbb814da5 Mon Sep 17 00:00:00 2001 From: HanatoK Date: Tue, 9 Sep 2025 14:49:39 -0500 Subject: [PATCH 05/18] fix: fix the uninitialized variables in ArithmeticPathBase --- src/colvar_arithmeticpath.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/colvar_arithmeticpath.h b/src/colvar_arithmeticpath.h index ee97390df..182fd78fd 100644 --- a/src/colvar_arithmeticpath.h +++ b/src/colvar_arithmeticpath.h @@ -16,7 +16,10 @@ using std::vector; template class ArithmeticPathBase { public: - ArithmeticPathBase() {} + ArithmeticPathBase(): + lambda(scalar_type(0)), num_elements(0), total_frames(0), + max_exponent(scalar_type(0)), saved_exponent_sum(scalar_type(0)), + normalization_factor(scalar_type(0)), saved_s(scalar_type(0)) {} ~ArithmeticPathBase() {} void initialize(size_t p_num_elements, size_t p_total_frames, scalar_type p_lambda, const vector& p_weights); void reComputeLambda(const vector& rmsd_between_refs); From 0868ddc7238bbce7aca8078f404de2df90e3b624 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 15:47:35 -0400 Subject: [PATCH 06/18] fix: use valid default values for std::ios::fmtflags --- src/colvarbias_abf.cpp | 4 ++-- src/colvarbias_histogram.cpp | 2 +- src/colvarbias_histogram_reweight_amd.cpp | 2 +- src/colvars_memstream.h | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/colvarbias_abf.cpp b/src/colvarbias_abf.cpp index 12c1753d1..3eb2f4f96 100644 --- a/src/colvarbias_abf.cpp +++ b/src/colvarbias_abf.cpp @@ -925,7 +925,7 @@ template OST & colvarbias_abf::write_state_data_template_(OST &os { auto flags = os.flags(); - os.setf(std::ios::fmtflags(0), std::ios::floatfield); // default floating-point format + os.setf(std::ios::fmtflags(std::ios::dec), std::ios::floatfield); // default floating-point format write_state_data_key(os, "samples"); samples->write_raw(os, 8); @@ -941,7 +941,7 @@ template OST & colvarbias_abf::write_state_data_template_(OST &os } if (b_CZAR_estimator) { - os.setf(std::ios::fmtflags(0), std::ios::floatfield); // default floating-point format + os.setf(std::ios::fmtflags(std::ios::dec), std::ios::floatfield); // default floating-point format write_state_data_key(os, "z_samples"); z_samples->write_raw(os, 8); write_state_data_key(os, "z_gradient"); diff --git a/src/colvarbias_histogram.cpp b/src/colvarbias_histogram.cpp index aab2c8f59..54eabbe72 100644 --- a/src/colvarbias_histogram.cpp +++ b/src/colvarbias_histogram.cpp @@ -216,7 +216,7 @@ cvm::memory_stream & colvarbias_histogram::read_state_data(cvm::memory_stream& i std::ostream & colvarbias_histogram::write_state_data(std::ostream& os) { std::ios::fmtflags flags(os.flags()); - os.setf(std::ios::fmtflags(0), std::ios::floatfield); + os.setf(std::ios::fmtflags(std::ios::dec), std::ios::floatfield); write_state_data_key(os, "grid"); grid->write_raw(os, 8); os.flags(flags); diff --git a/src/colvarbias_histogram_reweight_amd.cpp b/src/colvarbias_histogram_reweight_amd.cpp index b0e6b1c0f..562fcdf31 100644 --- a/src/colvarbias_histogram_reweight_amd.cpp +++ b/src/colvarbias_histogram_reweight_amd.cpp @@ -318,7 +318,7 @@ void colvarbias_reweightaMD::compute_cumulant_expansion_factor( template OST & colvarbias_reweightaMD::write_state_data_template_(OST& os) { std::ios::fmtflags flags(os.flags()); - os.setf(std::ios::fmtflags(0), std::ios::floatfield); + os.setf(std::ios::fmtflags(std::ios::dec), std::ios::floatfield); write_state_data_key(os, "grid"); grid->write_raw(os, 8); write_state_data_key(os, "grid_count"); diff --git a/src/colvars_memstream.h b/src/colvars_memstream.h index c9564a3c4..e1ff9b02b 100644 --- a/src/colvars_memstream.h +++ b/src/colvars_memstream.h @@ -106,16 +106,16 @@ class cvm::memory_stream { inline memory_stream & seekg(size_t pos) { read_pos_ = pos; return *this; } /// Ignore formatting operators - inline void setf(decltype(std::ios::fmtflags(0)), decltype(std::ios::floatfield)) {} + inline void setf(decltype(std::ios::fmtflags(std::ios::unitbuf)), decltype(std::ios::floatfield)) {} /// Ignore formatting operators - inline void setf(decltype(std::ios::fmtflags(0))) {} + inline void setf(decltype(std::ios::fmtflags(std::ios::unitbuf))) {} /// Ignore formatting operators - inline void flags(decltype(std::ios::fmtflags(0))) {} + inline void flags(decltype(std::ios::fmtflags(std::ios::unitbuf))) {} - /// Get the current formatting flags (i.e. none because this stream is unformatted) - inline decltype(std::ios::fmtflags(0)) flags() const { return std::ios::fmtflags(0); } + /// Get the current formatting flags (throw a useless result because this stream is unformatted) + inline decltype(std::ios::fmtflags(std::ios::unitbuf)) flags() const { return std::ios::fmtflags(std::ios::unitbuf); } /// Get the error code inline std::ios::iostate rdstate() const { return state_; } From b539931bd0fd9efb7b9696ea0e581297ee2fcfa7 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 16:12:10 -0400 Subject: [PATCH 07/18] fix: Call virtual functions consistently from constructor --- src/colvar.cpp | 4 ++-- src/colvaratoms.cpp | 4 ++-- src/colvarbias.cpp | 4 ++-- src/colvarcomp.cpp | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/colvar.cpp b/src/colvar.cpp index 011625d6d..d81d24587 100644 --- a/src/colvar.cpp +++ b/src/colvar.cpp @@ -1139,9 +1139,9 @@ int colvar::parse_analysis(std::string const &conf) int colvar::init_dependencies() { size_t i; - if (features().size() == 0) { + if (colvar::features().size() == 0) { for (i = 0; i < f_cv_ntot; i++) { - modify_features().push_back(new feature); + colvar::modify_features().push_back(new feature); } init_feature(f_cv_active, "active", f_type_dynamic); diff --git a/src/colvaratoms.cpp b/src/colvaratoms.cpp index 826a0943e..827ba4429 100644 --- a/src/colvaratoms.cpp +++ b/src/colvaratoms.cpp @@ -176,9 +176,9 @@ int cvm::atom_group::init() int cvm::atom_group::init_dependencies() { size_t i; // Initialize static array once and for all - if (features().size() == 0) { + if (atom_group::features().size() == 0) { for (i = 0; i < f_ag_ntot; i++) { - modify_features().push_back(new feature); + atom_group::modify_features().push_back(new feature); } init_feature(f_ag_active, "active", f_type_dynamic); diff --git a/src/colvarbias.cpp b/src/colvarbias.cpp index c5a29c1b5..bd3fd4e4a 100644 --- a/src/colvarbias.cpp +++ b/src/colvarbias.cpp @@ -179,9 +179,9 @@ int colvarbias::init_mts(std::string const &conf) { int colvarbias::init_dependencies() { int i; - if (features().size() == 0) { + if (colvarbias::features().size() == 0) { for (i = 0; i < f_cvb_ntot; i++) { - modify_features().push_back(new feature); + colvarbias::modify_features().push_back(new feature); } init_feature(f_cvb_active, "active", f_type_dynamic); diff --git a/src/colvarcomp.cpp b/src/colvarcomp.cpp index a6fde20f3..760e2697f 100644 --- a/src/colvarcomp.cpp +++ b/src/colvarcomp.cpp @@ -232,9 +232,9 @@ cvm::atom_group *colvar::cvc::parse_group(std::string const &conf, int colvar::cvc::init_dependencies() { size_t i; // Initialize static array once and for all - if (features().size() == 0) { + if (cvc::features().size() == 0) { for (i = 0; i < colvardeps::f_cvc_ntot; i++) { - modify_features().push_back(new feature); + cvc::modify_features().push_back(new feature); } init_feature(f_cvc_active, "active", f_type_dynamic); From 41d904122dac17ca57d139c40be973bb4c21e3de Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 16:19:09 -0400 Subject: [PATCH 08/18] fix: Tell Clang that Parse_Mode is a flag enum --- src/colvarparse.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/colvarparse.h b/src/colvarparse.h index 105a0857a..1cefce44c 100644 --- a/src/colvarparse.h +++ b/src/colvarparse.h @@ -49,8 +49,14 @@ class colvarparse : public colvarparams { return config_string; } +#ifdef __clang__ +#define PARSE_MODE_IS_FLAG [[clang::flag_enum]] +#else +#define PARSE_MODE_IS_FLAG +#endif + /// How a keyword is parsed in a string - enum Parse_Mode { + enum PARSE_MODE_IS_FLAG Parse_Mode { /// Zero for all flags parse_null = 0, /// Print the value of a keyword if it is given From cf07727d82b0aae1e1fd7a5e0f4e522f23d9bff1 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Tue, 9 Sep 2025 17:24:46 -0400 Subject: [PATCH 09/18] test: Write clang-tidy output directly, to be retrieved later --- .github/workflows/test-library.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 965ee5d72..f1fe12404 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -265,7 +265,7 @@ jobs: working-directory: build run: | apptainer exec ${{github.workspace}}/devel-tools/CentOS9-devel.sif \ - run-clang-tidy -header-filter=.* -warnings-as-errors \* | tee clang-tidy.log + run-clang-tidy -header-filter=.* -warnings-as-errors \* >& clang-tidy.log - name: Archive warnings artifacts uses: actions/upload-artifact@v4 From 6eb426a19c8c4b51a3e94db4e9dad3ff52e222dc Mon Sep 17 00:00:00 2001 From: HanatoK Date: Wed, 10 Sep 2025 11:12:58 -0500 Subject: [PATCH 10/18] test: simplify the running of clang-tidy This commit uses a shell script to run clang-tidy only over the Colvars source files, which could skip the Lepton source files. This commit also avoids pulling the apptainer image. --- .github/workflows/test-library.yml | 19 +++------------ devel-tools/run_clang_tidy.sh | 39 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 15 deletions(-) create mode 100755 devel-tools/run_clang_tidy.sh diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index f1fe12404..5502c980a 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -243,29 +243,18 @@ jobs: - name: Install Apptainer shell: bash run: | - sudo add-apt-repository -y ppa:apptainer/ppa - sudo apt update - sudo apt install -y apptainer-suid - - - name: Get container images for build dependencies - shell: bash - working-directory: devel-tools - run: | - apptainer pull CentOS9-devel.sif oras://ghcr.io/colvars/devel-containers:CentOS9-devel + sudo apt -y bash install ccache tcl8.6-dev clang clang-tidy - name: Configure and build the library (but do not run tests) env: + CMAKE_BUILD_DIR: build CXX: clang++ CC: clang - run: | - apptainer exec ${{github.workspace}}/devel-tools/CentOS9-devel.sif \ - cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake + run: cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake - name: Run "run-clang-tidy" in the "build" folder working-directory: build - run: | - apptainer exec ${{github.workspace}}/devel-tools/CentOS9-devel.sif \ - run-clang-tidy -header-filter=.* -warnings-as-errors \* >& clang-tidy.log + run: bash ${{ github.workspace }}/devel-tools/run_clang_tidy.sh &> clang-tidy.log - name: Archive warnings artifacts uses: actions/upload-artifact@v4 diff --git a/devel-tools/run_clang_tidy.sh b/devel-tools/run_clang_tidy.sh new file mode 100755 index 000000000..7c374c644 --- /dev/null +++ b/devel-tools/run_clang_tidy.sh @@ -0,0 +1,39 @@ +#!/bin/bash + +# Get the top Colvars repo directory +TOPDIR=$(git rev-parse --show-toplevel) +if [ ! -d ${TOPDIR} ] ; then + echo "Error: cannot identify top project directory." >& 2 + exit 1 +fi + +# Get the build directory +COLVARS_BUILD_DIR="$(pwd)" + +# Get the Colvars source directory +COLVARS_SOURCE_DIR="$TOPDIR/src" +# Find all source files that are required to build the run_colvars_test +COLVARS_STUB_SOURCE_DIR="$TOPDIR/misc_interfaces/stubs" +COLVARS_TEST_SOURCE_DIR="$TOPDIR/tests/functional/" +COLVARS_SOURCE_FILES=$(find $COLVARS_STUB_SOURCE_DIR $COLVARS_TEST_SOURCE_DIR $COLVARS_SOURCE_DIR -name "*.cpp") + +num_test_failed=0 +all_output="" +# Run clang-tidy over all files and save the results +for colvars_src_file in $COLVARS_SOURCE_FILES; do + clang_tidy_command="clang-tidy -p=$COLVARS_BUILD_DIR --warnings-as-errors='*' $colvars_src_file" + echo "Running $clang_tidy_command" + output="$(eval $clang_tidy_command 2>&1)" || exit_code=$? + all_output+="$output" + if [[ $exit_code -gt 0 ]]; then + num_test_failed=`expr $num_test_failed + 1` + fi +done + +if [[ $num_test_failed -gt 0 ]]; then + echo "There are $num_test_failed test(s) failed." + echo "$all_output" + exit 1 +else + exit 0 +fi From a23bfd108ba1349c0b5a78a004324953900bfb14 Mon Sep 17 00:00:00 2001 From: HanatoK Date: Wed, 10 Sep 2025 11:20:02 -0500 Subject: [PATCH 11/18] test: fix the typo in test-library.yml --- .github/workflows/test-library.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 5502c980a..99981452a 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -243,7 +243,7 @@ jobs: - name: Install Apptainer shell: bash run: | - sudo apt -y bash install ccache tcl8.6-dev clang clang-tidy + sudo apt -y install bash ccache tcl8.6-dev clang clang-tidy - name: Configure and build the library (but do not run tests) env: From 0a77a7afadc5c8605a1741304c30c52f9aa7b6fb Mon Sep 17 00:00:00 2001 From: HanatoK Date: Wed, 10 Sep 2025 11:29:42 -0500 Subject: [PATCH 12/18] test: rename the github actions --- .github/workflows/test-library.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 99981452a..384c21035 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -240,7 +240,7 @@ jobs: ref: 'master' path: 'openmm-source' - - name: Install Apptainer + - name: Install build dependencies, clang-tidy and bash for library shell: bash run: | sudo apt -y install bash ccache tcl8.6-dev clang clang-tidy @@ -252,7 +252,7 @@ jobs: CC: clang run: cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake - - name: Run "run-clang-tidy" in the "build" folder + - name: Run "run_clang_tidy.sh" in the "build" folder working-directory: build run: bash ${{ github.workspace }}/devel-tools/run_clang_tidy.sh &> clang-tidy.log From ea3811c94007ac7790fedd7b6522041c66013b86 Mon Sep 17 00:00:00 2001 From: HanatoK Date: Wed, 10 Sep 2025 12:52:48 -0500 Subject: [PATCH 13/18] test: use clang-tidy-20 in ubuntu --- .github/workflows/test-library.yml | 8 +++++--- devel-tools/run_clang_tidy.sh | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 384c21035..960eeb42b 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -243,16 +243,18 @@ jobs: - name: Install build dependencies, clang-tidy and bash for library shell: bash run: | - sudo apt -y install bash ccache tcl8.6-dev clang clang-tidy + sudo apt -y install bash ccache tcl8.6-dev clang-20 clang-tidy-20 - name: Configure and build the library (but do not run tests) env: CMAKE_BUILD_DIR: build - CXX: clang++ - CC: clang + CXX: clang++-20 + CC: clang-20 run: cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake - name: Run "run_clang_tidy.sh" in the "build" folder + env: + CLANG_TIDY: clang-tidy-20 working-directory: build run: bash ${{ github.workspace }}/devel-tools/run_clang_tidy.sh &> clang-tidy.log diff --git a/devel-tools/run_clang_tidy.sh b/devel-tools/run_clang_tidy.sh index 7c374c644..edbae66dd 100755 --- a/devel-tools/run_clang_tidy.sh +++ b/devel-tools/run_clang_tidy.sh @@ -10,6 +10,14 @@ fi # Get the build directory COLVARS_BUILD_DIR="$(pwd)" +# Get the clang-tidy binary +if [[ -v CLANG_TIDY ]]; then + CLANG_TIDY_BINARY=$CLANG_TIDY + echo "Using $CLANG_TIDY_BINARY as clang-tidy" +else + CLANG_TIDY_BINARY="clang-tidy" +fi + # Get the Colvars source directory COLVARS_SOURCE_DIR="$TOPDIR/src" # Find all source files that are required to build the run_colvars_test @@ -21,7 +29,7 @@ num_test_failed=0 all_output="" # Run clang-tidy over all files and save the results for colvars_src_file in $COLVARS_SOURCE_FILES; do - clang_tidy_command="clang-tidy -p=$COLVARS_BUILD_DIR --warnings-as-errors='*' $colvars_src_file" + clang_tidy_command="$CLANG_TIDY_BINARY -p=$COLVARS_BUILD_DIR --warnings-as-errors='*' $colvars_src_file" echo "Running $clang_tidy_command" output="$(eval $clang_tidy_command 2>&1)" || exit_code=$? all_output+="$output" From 139865d51ce9aa544083460d194704b3c6e009d5 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Wed, 10 Sep 2025 13:59:03 -0400 Subject: [PATCH 14/18] test: Add option to run CMake helper script only to configure targets --- .github/workflows/test-library.yml | 2 +- devel-tools/build_test_library.cmake | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 960eeb42b..01d19b473 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -250,7 +250,7 @@ jobs: CMAKE_BUILD_DIR: build CXX: clang++-20 CC: clang-20 - run: cmake -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake + run: cmake -DBUILD_TARGETS=OFF -DRUN_TESTS=OFF -P devel-tools/build_test_library.cmake - name: Run "run_clang_tidy.sh" in the "build" folder env: diff --git a/devel-tools/build_test_library.cmake b/devel-tools/build_test_library.cmake index 6d8bc0bed..be49ea422 100644 --- a/devel-tools/build_test_library.cmake +++ b/devel-tools/build_test_library.cmake @@ -150,12 +150,16 @@ if(NOT result EQUAL 0) message(FATAL_ERROR "Error generating CMake buildsystem.") endif() -execute_process( - COMMAND ${CMAKE_COMMAND} - --build ${BUILD_DIR} - --parallel - RESULT_VARIABLE result +option(BUILD_TARGETS "Build library and related tools" ON) + +if(BUILD_TARGETS) + execute_process( + COMMAND ${CMAKE_COMMAND} + --build ${BUILD_DIR} + --parallel + RESULT_VARIABLE result ) +endif() option(RUN_TESTS "Run library tests" ON) From 03e3ef1955dbb45a7a2b7058ec8ca950cf4842bd Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Wed, 10 Sep 2025 15:05:13 -0400 Subject: [PATCH 15/18] fix: Manually add combined value to silence clang-tidy --- src/colvarbias_abmd.cpp | 8 +++----- src/colvarparse.h | 10 ++++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/colvarbias_abmd.cpp b/src/colvarbias_abmd.cpp index 0e44143b8..12a09ddd3 100644 --- a/src/colvarbias_abmd.cpp +++ b/src/colvarbias_abmd.cpp @@ -105,12 +105,10 @@ int colvarbias_abmd::set_state_params(std::string const &conf) colvarparse::parse_restart | colvarparse::parse_required); ref_initialized = true; - get_keyval(conf, "forceConstant", k, k, - colvarparse::parse_restart | colvarparse::parse_required); - get_keyval(conf, "decreasing", decreasing, decreasing, - colvarparse::parse_restart | colvarparse::parse_required); + get_keyval(conf, "forceConstant", k, k, colvarparse::parse_required_restart); + get_keyval(conf, "decreasing", decreasing, decreasing, colvarparse::parse_required_restart); get_keyval(conf, "stoppingValue", stopping_val, stopping_val, - colvarparse::parse_restart | colvarparse::parse_required); + colvarparse::parse_required_restart); return COLVARS_OK; } diff --git a/src/colvarparse.h b/src/colvarparse.h index 1cefce44c..3d5813fc9 100644 --- a/src/colvarparse.h +++ b/src/colvarparse.h @@ -74,6 +74,8 @@ class colvarparse : public colvarparams { parse_override = (1<<17), /// The call is being executed from a read_restart() function parse_restart = (1<<18), + /// Provide explicitly to silence clang-tidy warnings + parse_required_restart = (1<<16) | (1<<18), /// Alias for old default behavior (should be phased out) parse_normal = (1<<1) | (1<<2) | (1<<17), /// Settings for a deprecated keyword @@ -392,11 +394,11 @@ class colvarparse : public colvarparams { /// Bitwise OR between two Parse_mode flags -inline colvarparse::Parse_Mode operator | (colvarparse::Parse_Mode const &mode1, - colvarparse::Parse_Mode const &mode2) +inline constexpr colvarparse::Parse_Mode operator|(colvarparse::Parse_Mode mode1, + colvarparse::Parse_Mode mode2) { - return static_cast(static_cast(mode1) | - static_cast(mode2)); + using T = std::underlying_type_t; + return static_cast(static_cast(mode1) | static_cast(mode2)); } #endif From 0e6766e346002003435465ae4f04bc3b695593ce Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Wed, 10 Sep 2025 15:18:34 -0400 Subject: [PATCH 16/18] fix: Run clang-tidy on headers as well (and in parallel) --- devel-tools/run_clang_tidy.sh | 34 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/devel-tools/run_clang_tidy.sh b/devel-tools/run_clang_tidy.sh index edbae66dd..b819cdbf9 100755 --- a/devel-tools/run_clang_tidy.sh +++ b/devel-tools/run_clang_tidy.sh @@ -23,25 +23,17 @@ COLVARS_SOURCE_DIR="$TOPDIR/src" # Find all source files that are required to build the run_colvars_test COLVARS_STUB_SOURCE_DIR="$TOPDIR/misc_interfaces/stubs" COLVARS_TEST_SOURCE_DIR="$TOPDIR/tests/functional/" -COLVARS_SOURCE_FILES=$(find $COLVARS_STUB_SOURCE_DIR $COLVARS_TEST_SOURCE_DIR $COLVARS_SOURCE_DIR -name "*.cpp") - -num_test_failed=0 -all_output="" -# Run clang-tidy over all files and save the results -for colvars_src_file in $COLVARS_SOURCE_FILES; do - clang_tidy_command="$CLANG_TIDY_BINARY -p=$COLVARS_BUILD_DIR --warnings-as-errors='*' $colvars_src_file" - echo "Running $clang_tidy_command" - output="$(eval $clang_tidy_command 2>&1)" || exit_code=$? - all_output+="$output" - if [[ $exit_code -gt 0 ]]; then - num_test_failed=`expr $num_test_failed + 1` - fi -done - -if [[ $num_test_failed -gt 0 ]]; then - echo "There are $num_test_failed test(s) failed." - echo "$all_output" - exit 1 -else - exit 0 +COLVARS_SOURCE_FILES=($(find $COLVARS_STUB_SOURCE_DIR $COLVARS_TEST_SOURCE_DIR $COLVARS_SOURCE_DIR -name "*.cpp")) + +printf '%s\n' ${COLVARS_SOURCE_FILES[@]} | \ + xargs -I{} -P $(nproc) \ + bash -c "$CLANG_TIDY_BINARY -p=$COLVARS_BUILD_DIR -header-filter=.* --warnings-as-errors='*' '{}' > '{}'.log" + +retcode=$? + +if [ $retcode != 0 ] ; then + cat "${COLVARS_SOURCE_FILES[@]/%/.log}" + exit $retcode fi + +exit 0 From 5cf856d4e5d41e66cdbf556ceac7db4ada6fbf14 Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Wed, 10 Sep 2025 15:26:42 -0400 Subject: [PATCH 17/18] fix: use C++11 compatible construct --- src/colvarparse.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/colvarparse.h b/src/colvarparse.h index 3d5813fc9..e47e2ff0d 100644 --- a/src/colvarparse.h +++ b/src/colvarparse.h @@ -397,7 +397,7 @@ class colvarparse : public colvarparams { inline constexpr colvarparse::Parse_Mode operator|(colvarparse::Parse_Mode mode1, colvarparse::Parse_Mode mode2) { - using T = std::underlying_type_t; + using T = std::underlying_type::type; return static_cast(static_cast(mode1) | static_cast(mode2)); } From ae99b67c8211a819bc3c4a4da3df7a99e5fa193b Mon Sep 17 00:00:00 2001 From: Giacomo Fiorin Date: Mon, 22 Sep 2025 20:57:02 -0400 Subject: [PATCH 18/18] Print clang-tidy errors to screen --- .github/workflows/test-library.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml index 01d19b473..8daab76c0 100644 --- a/.github/workflows/test-library.yml +++ b/.github/workflows/test-library.yml @@ -258,6 +258,14 @@ jobs: working-directory: build run: bash ${{ github.workspace }}/devel-tools/run_clang_tidy.sh &> clang-tidy.log + - name: Print clang-tidy errors to screen + if: failure() + working-directory: build + shell: bash + run: | + cat clang-tidy.log >&2 + exit 1 + - name: Archive warnings artifacts uses: actions/upload-artifact@v4 if: failure()