Use clang-tidy.

.clang-tidy

@@ -0,0 +1,10 @@
+Checks: "-*,clang-analyzer-*,bugprone-*,-bugprone-exception-escape,-bugprone-macro-parentheses,-bugprone-easily-swappable-parameters,-bugprone-narrowing-conversions"
+# ,modernize-*,-modernize-use-trailing-return-type,-modernize-concat-nested-namespaces,-modernize-avoid-c-arrays,-modernize-use-nodiscard
+# ,misc-*,-misc-throw-by-value-catch-by-reference,-misc-no-recursion,-misc-non-private-member-variables-in-classes,-misc-const-correctness,-misc-confusable-identifiers
+#
+
+# WarningsAsErrors: "*"
+
+# IMPORTANT: Set HeaderFilterRegex as shown below.
+# Do not set it to '.*', for example.
+HeaderFilterRegex: ""

.github/workflows/lint.yml

@@ -12,6 +12,8 @@ jobs:
       fail-fast: false
     steps:
       - uses: actions/checkout@v4
+        with:
+          submodules: recursive
       - name: Set up Python
         uses: actions/setup-python@v5
         with:
@@ -30,3 +32,13 @@ jobs:
       - name: Python formatting
         run: |
           ruff format --check .
+      - name: Install tools
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y clang clang-tidy cmake ninja-build
+      - name: Lint
+        run: |
+          mkdir build
+          cd build
+          cmake .. -GNinja -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang -DARB_WITH_PYTHON=OFF -DARB_VECTORIZE=OFF -DARB_WITH_MPI=OFF -DARB_WITH_ASSERTIONS=ON -DARB_CLANG_TIDY=ON
+          ninja

CMakeLists.txt

@@ -79,6 +79,14 @@ mark_as_advanced(ARB_USE_HWLOC)
 option(BUILD_TESTING "build tests and benchmarks" ON)
 option(BUILD_DOCUMENTATION "build documentation" ON)
 
+# Use C++ linter
+option(ARB_CLANG_TIDY "lint C++ sources" OFF)
+
+if (ARB_CLANG_TIDY)
+    set(CMAKE_CXX_CLANG_TIDY "clang-tidy;--config-file=${CMAKE_SOURCE_DIR}/.clang-tidy")
+endif ()
+
+
 # Use externally built modcc?
 
 set(ARB_MODCC "" CACHE STRING "path to external modcc NMODL compiler")
@@ -198,8 +206,7 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 
 # Generate a .json file with full compilation command for each file.
-
-set(CMAKE_EXPORT_COMPILE_COMMANDS "YES")
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
 # Detect and deprecate xlC.
 
@@ -310,6 +317,7 @@ CPMAddPackage(NAME random123
               VERSION 1.14.0)
 if(random123_ADDED)
     target_include_directories(ext-random123 INTERFACE $<BUILD_INTERFACE:${random123_SOURCE_DIR}/include>)
+    write_file(${random123_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
 else()
     find_package(Random123 REQUIRED)
     target_include_directories(ext-random123 INTERFACE ${RANDOM123_INCLUDE_DIR})
@@ -333,6 +341,7 @@ add_library(ext-pugixml INTERFACE)
 if(pugixml_ADDED)
     target_compile_definitions(ext-pugixml INTERFACE PUGIXML_HEADER_ONLY)
     target_include_directories(ext-pugixml INTERFACE $<BUILD_INTERFACE:${pugixml_SOURCE_DIR}/src>)
+    write_file(${pugixml_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
 else()
     find_package(pugixml REQUIRED)
     target_link_libraries(ext-pugixml INTERFACE pugixml::pugixml)
@@ -354,6 +363,12 @@ if (BUILD_TESTING)
                   GIT_TAG release-1.12.1
                   VERSION 1.12.1
                   OPTIONS "INSTALL_GTEST OFF" "BUILD_GMOCK OFF")
+    if(benchmark_ADDED)
+        write_file(${benchmark_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
+    endif()
+    if(googletest_ADDED)
+        write_file(${googletest_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
+    endif()
 endif()
 
 CPMAddPackage(NAME units
@@ -368,6 +383,7 @@ CPMAddPackage(NAME units
 add_library(ext-units INTERFACE)
 if(units_ADDED)
     target_link_libraries(ext-units INTERFACE units::units)
+    write_file(${units_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
 else()
     find_package(units REQUIRED)
     target_link_libraries(ext-units INTERFACE units::units)
@@ -383,6 +399,7 @@ CPMAddPackage(NAME tinyopt
 add_library(ext-tinyopt INTERFACE)
 if(tinyopt_ADDED)
   target_include_directories(ext-tinyopt INTERFACE $<BUILD_INTERFACE:${tinyopt_SOURCE_DIR}/include>)
+  write_file(${tinyopt_SOURCE_DIR}/.clang-tidy "Checks: \"-*\"")
 else()
     message(FATAL_ERROR "Could not obtain tinyopt.")
 endif()
@@ -393,13 +410,15 @@ mark_as_advanced(FORCE googletest_DIR BUILD_GMOCK)
 mark_as_advanced(FORCE json_DIR JSON_CI JSON_BuildTests JSON_Diagnostics JSON_DisableEnumSerialization JSON_GlobalUDLs JSON_ImplicitConversions JSON_Install JSON_LegacyDiscardedValueComparison JSON_MultipleHeaders JSON_SystemInclude)
 mark_as_advanced(FORCE RANDOM123_INCLUDE_DIR)
 mark_as_advanced(FORCE pybind11_DIR PYBIND11_PYTHONLIBS_OVERWRITE PYBIND11_PYTHON_VERSION PYBIND11_FINDPYTHON PYBIND11_INSTALL PYBIND11_INTERNALS_VERSION PYBIND11_NOPYTHON PYBIND11_SIMPLE_GIL_MANAGEMENT PYBIND11_TEST)
-mark_as_advanced(FORCE pugixml_DIR)
+mark_as_advanced(FORCE pugixml_DIR FETCHCONTENT_SOURCE_DIR_PUGIXML FETCHCONTENT_UPDATES_DISCONNECTED_PUGIXML)
 mark_as_advanced(FORCE fmt_DIR)
 mark_as_advanced(FORCE units_DIR UNITS_BUILD_OBJECT_LIBRARY UNITS_BUILD_SHARED_LIBRARY UNITS_HEADER_ONLY UNITS_NAMESPACE UNITS_BUILD_FUZZ_TARGETS UNITS_ENABLE_TESTS)
 mark_as_advanced(FORCE tinyopt_DIR)
 mark_as_advanced(FORCE CXXFEATURECHECK_DEBUG)
 mark_as_advanced(FORCE CPM_DONT_CREATE_PACKAGE_LOCK CPM_DONT_UPDATE_MODULE_PATH CPM_DOWNLOAD_ALL CPM_INCLUDE_ALL_IN_PACKAGE_LOCK CPM_LOCAL_PACKAGES_ONLY CPM_SOURCE_CACHE CPM_USE_NAMED_CACHE_DIRECTORIES)
 mark_as_advanced(FORCE FETCHCONTENT_BASE_DIR FETCHCONTENT_FULLY_DISCONNECTED FETCHCONTENT_QUIET FETCHCONTENT_SOURCE_DIR_BENCHMARK FETCHCONTENT_SOURCE_DIR_GOOGLETEST FETCHCONTENT_SOURCE_DIR_JSON FETCHCONTENT_SOURCE_DIR_PYBIND11 FETCHCONTENT_SOURCE_DIR_RANDOM123 FETCHCONTENT_SOURCE_DIR_TINYOPT FETCHCONTENT_SOURCE_DIR_UNITS FETCHCONTENT_UPDATES_DISCONNECTED FETCHCONTENT_UPDATES_DISCONNECTED_BENCHMARK FETCHCONTENT_UPDATES_DISCONNECTED_GOOGLETEST FETCHCONTENT_UPDATES_DISCONNECTED_JSON FETCHCONTENT_UPDATES_DISCONNECTED_PYBIND11 FETCHCONTENT_UPDATES_DISCONNECTED_RANDOM123 FETCHCONTENT_UPDATES_DISCONNECTED_TINYOPT FETCHCONTENT_UPDATES_DISCONNECTED_UNITS)
+mark_as_advanced(FORCE CMAKE_PDB_OUTPUT_DIRECTORY)
+mark_as_advanced(FORCE PY_PYBIND11_STUBGEN PY_SVGWRITE)
 
 
 # Restore chattyness

arbor/CMakeLists.txt

@@ -128,7 +128,9 @@ set(_saved_CMAKE_MESSAGE_LOG_LEVEL ${CMAKE_MESSAGE_LOG_LEVEL})
 set(CMAKE_MESSAGE_LOG_LEVEL WARNING)
 add_subdirectory(../mechanisms "${CMAKE_BINARY_DIR}/mechanisms")
 set(CMAKE_MESSAGE_LOG_LEVEL ${_saved_CMAKE_MESSAGE_LOG_LEVEL})
-set_source_files_properties(${arbor-builtin-mechanisms} DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTIES GENERATED TRUE)
+set_source_files_properties(${arbor-builtin-mechanisms}
+                            PROPERTIES GENERATED TRUE
+                                       SKIP_LINTING ON)
 
 if(ARB_WITH_CUDA_CLANG OR ARB_WITH_HIP_CLANG)
     set_source_files_properties(${arbor_sources} DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTIES LANGUAGE CXX)

arbor/affinity.hpp

@@ -44,7 +44,7 @@ void set_affinity(int index, int count, affinity_kind kind) {
     hwloc(hwloc_topology_load(topology), "Topo load");
     // Fetch our current restrictions and apply them to our topology
     hwloc_cpuset_t cpus = hwloc_bitmap_alloc();
-    hwloc(cpus == NULL, "Bitmap allocation");
+    hwloc(cpus == nullptr, "Bitmap allocation");
     auto bitmap_guard = util::on_scope_exit([&] { hwloc_bitmap_free(cpus); });
     hwloc(hwloc_get_cpubind(topology, cpus, HWLOC_CPUBIND_PROCESS), "Get cpuset.");
     hwloc(hwloc_topology_restrict(topology, cpus, 0), "Topo restrict.");

arbor/arbexcept.cpp

@@ -80,7 +80,7 @@ bad_probeset_id::bad_probeset_id(cell_member_type probeset_id):
     probeset_id(probeset_id)
 {}
 
-gj_unsupported_lid_selection_policy::gj_unsupported_lid_selection_policy(cell_gid_type gid, cell_tag_type label):
+gj_unsupported_lid_selection_policy::gj_unsupported_lid_selection_policy(cell_gid_type gid, const cell_tag_type& label):
     arbor_exception(pprintf("Model building error on cell {}: gap junction site label \"{}\" must be univalent.", gid, label)),
     gid(gid),
     label(label)
@@ -190,8 +190,8 @@ bad_catalogue_error::bad_catalogue_error(const std::string& msg)
     : arbor_exception(pprintf("Error while opening catalogue '{}'", msg))
 {}
 
-bad_catalogue_error::bad_catalogue_error(const std::string& msg, const std::any& pe)
-    : arbor_exception(pprintf("Error while opening catalogue '{}'", msg)), platform_error(pe)
+bad_catalogue_error::bad_catalogue_error(const std::string& msg, std::any pe)
+    : arbor_exception(pprintf("Error while opening catalogue '{}'", msg)), platform_error(std::move(pe))
 {}
 
 unsupported_abi_error::unsupported_abi_error(size_t v):

arbor/backends/event.hpp

@@ -14,8 +14,8 @@ namespace arb {
 // Post-synaptic spike events
 
 struct target_handle {
-    cell_local_size_type mech_id;    // mechanism type identifier (per cell group).
-    cell_local_size_type mech_index; // instance of the mechanism
+    cell_local_size_type mech_id = 0;    // mechanism type identifier (per cell group).
+    cell_local_size_type mech_index = 0; // instance of the mechanism
 
     target_handle() = default;
 
@@ -81,8 +81,8 @@ inline arb_deliverable_event_stream make_event_stream_state(arb_deliverable_even
 using probe_handle = const arb_value_type*;
 
 struct raw_probe_info {
-    probe_handle handle;      // where the to-be-probed value sits
-    sample_size_type offset;  // offset into array to store raw probed value
+    probe_handle handle = nullptr; // where the to-be-probed value sits
+    sample_size_type offset = 0;   // offset into array to store raw probed value
 };
 
 struct sample_event {

arbor/backends/event_stream_state.hpp

@@ -11,8 +11,8 @@ template <typename EvData>
 struct event_stream_state {
     using value_type = EvData;
 
-    const value_type* begin_marked;   // offset to beginning of marked events
-    const value_type* end_marked;     // offset to one-past-end of marked events
+    const value_type* begin_marked = nullptr;   // offset to beginning of marked events
+    const value_type* end_marked = nullptr;     // offset to one-past-end of marked events
 
     std::size_t size() const noexcept {
         return end_marked - begin_marked;

arbor/backends/multicore/shared_state.cpp

@@ -177,7 +177,7 @@ void istim_state::add_current(const arb_value_type time, array& current_density)
             J = math::lerp(J, J1, u);
         }
 
-        if (frequency_[i]) {
+        if (0 != frequency_[i]) {
             J *= std::sin(two_pi*frequency_[i]*time + phase_[i]);
         }
 
@@ -212,7 +212,7 @@ shared_state::shared_state(task_system_handle,    // ignored in mc backend
     temperature_degC(n_cv_, pad(alignment)),
     diam_um(diam.begin(), diam.end(), pad(alignment)),
     area_um2(area.begin(), area.end(), pad(alignment)),
-    time_since_spike(n_cell*n_detector, pad(alignment)),
+    time_since_spike(n_cell*static_cast<std::size_t>(n_detector), pad(alignment)),
     src_to_spike(src_to_spike_.begin(), src_to_spike_.end(), pad(alignment)),
     cbprng_seed(cbprng_seed_),
     watcher{n_cv_, src_to_spike.data(), detector_info}
@@ -256,13 +256,15 @@ std::pair<arb_value_type, arb_value_type> shared_state::voltage_bounds() const {
 
 void shared_state::take_samples() {
     sample_events.mark();
-    if (!sample_events.empty()) {
-        const auto [begin, end] = sample_events.marked_events();
-        // Null handles are explicitly permitted, and always give a sample of zero.
-        for (auto p = begin; p<end; ++p) {
-            sample_time[p->offset] = time;
-            sample_value[p->offset] = p->handle? *p->handle: 0;
-        }
+    auto [begin, end] = sample_events.marked_events();
+    if (begin >= end) return;
+    if (begin == nullptr || end == nullptr) throw arbor_internal_error{"Invalid sample stream state."};
+    // Null handles are explicitly permitted, and always give a sample of zero.
+    for (; begin < end; ++begin) {
+        auto off = begin->offset;
+        sample_time[off] = time;
+        sample_value[off] = 0;
+        if (begin->handle) sample_value[off] = *begin->handle;
     }
 }
 
@@ -398,7 +400,7 @@ void shared_state::instantiate(arb::mechanism& m,
     m.ppack_.diam_um          = diam_um.data();
     m.ppack_.area_um2         = area_um2.data();
     m.ppack_.time_since_spike = time_since_spike.data();
-    m.ppack_.n_detectors      = n_detector;
+    m.ppack_.n_detectors      = static_cast<arb_index_type>(n_detector);
     m.ppack_.events           = {};
 
     bool mult_in_place = !pos_data.multiplicity.empty();
@@ -421,7 +423,7 @@ void shared_state::instantiate(arb::mechanism& m,
         if (!oion) throw arbor_internal_error(util::pprintf("multicore/mechanism: mechanism holds ion '{}' with no corresponding shared state", ion));
 
         auto& ion_state = m.ppack_.ion_states[idx];
-        ion_state = {0};
+        ion_state = {nullptr};
         ion_state.current_density         = oion->iX_.data();
         ion_state.reversal_potential      = oion->eX_.data();
         ion_state.internal_concentration  = oion->Xi_.data();

arbor/backends/multicore/threshold_watcher.hpp

@@ -15,7 +15,7 @@ namespace multicore {
 class threshold_watcher {
 public:
     threshold_watcher() = default;
-    threshold_watcher(const execution_context& ctx) {}
+    threshold_watcher(const execution_context&) {}
 
     threshold_watcher(const arb_size_type num_cv,
                       const arb_index_type* src_to_spike,
@@ -27,7 +27,7 @@ class threshold_watcher {
                       const arb_index_type* src_to_spike,
                       const std::vector<arb_index_type>& cv_index,
                       const std::vector<arb_value_type>& thresholds,
-                      const execution_context& context):
+                      const execution_context&):
         src_to_spike_(src_to_spike),
         n_detectors_(cv_index.size()),
         cv_index_(cv_index),

arbor/backends/shared_state_base.hpp

@@ -3,6 +3,8 @@
 #include <arbor/mechanism_abi.h>
 #include <arbor/common_types.hpp>
 
+#include "timestep_range.hpp"
+
 #include "backends/event.hpp"
 #include "backends/common_types.hpp"
 #include "fvm_layout.hpp"

arbor/benchmark_cell_group.cpp

@@ -51,7 +51,7 @@ void benchmark_cell_group::reset() {
         c.time_sequence.reset();
     }
 
-    clear_spikes();
+    benchmark_cell_group::clear_spikes();
 }
 
 void benchmark_cell_group::t_serialize(serializer& ser, const std::string& k) const {
@@ -66,8 +66,8 @@ cell_kind benchmark_cell_group::get_cell_kind() const {
 }
 
 void benchmark_cell_group::advance(epoch ep,
-                                   time_type dt,
-                                   const event_lane_subrange& event_lanes)
+                                   time_type /*dt*/,
+                                   const event_lane_subrange& /*event_lanes*/)
 {
     using std::chrono::high_resolution_clock;
     using duration_type = std::chrono::duration<double, std::micro>;

arbor/cable_cell.cpp

@@ -205,7 +205,7 @@ struct cable_cell_impl {
 
 using impl_ptr = std::unique_ptr<cable_cell_impl, void (*)(cable_cell_impl*)>;
 impl_ptr make_impl(cable_cell_impl* c) {
-    return impl_ptr(c, [](cable_cell_impl* p){delete p;});
+    return {c, [](cable_cell_impl* p){ delete p; }};
 }
 
 void cable_cell_impl::init() {