From 14f2d6a58aa0be0deace84cdc31eb700ad002214 Mon Sep 17 00:00:00 2001
From: Thorsten Hater <24411438+thorstenhater@users.noreply.github.com>
Date: Mon, 13 Jun 2022 10:13:29 +0200
Subject: [PATCH] Code Quality: PVS Studio Finds (#1901)

Fix a number of small issues regarding performance and correctness found by PVS Studio in the quest for a better code base.
---
 arbor/backends/event.hpp                      |  2 +-
 .../backends/multicore/multi_event_stream.hpp |  2 +-
 arbor/benchmark_cell_group.cpp                |  2 +-
 arbor/communication/communicator.hpp          |  2 +-
 arbor/domain_decomposition.cpp                |  7 +-
 arbor/event_queue.hpp                         |  2 +-
 arbor/fvm_layout.cpp                          | 88 +++++++++----------
 arbor/fvm_layout.hpp                          |  6 +-
 arbor/fvm_lowered_cell_impl.hpp               | 15 ++--
 arbor/include/arbor/mechanism.hpp             |  2 +-
 arbor/include/arbor/morph/embed_pwlin.hpp     | 12 +--
 arbor/include/arbor/morph/mcable_map.hpp      |  6 +-
 arbor/include/arbor/morph/primitives.hpp      |  4 +-
 arbor/include/arbor/morph/region.hpp          |  2 +-
 arbor/include/arbor/s_expr.hpp                |  1 +
 arbor/include/arbor/simd/simd.hpp             |  4 +-
 arbor/include/arbor/util/any_ptr.hpp          |  2 +-
 arbor/include/arbor/util/expected.hpp         |  2 +-
 arbor/include/arbor/util/scope_exit.hpp       |  2 +-
 arbor/lif_cell_group.cpp                      |  4 +-
 arbor/mc_cell_group.cpp                       |  2 +-
 arbor/mechcat.cpp                             |  9 +-
 arbor/memory/allocator.hpp                    |  6 +-
 arbor/memory/array_view.hpp                   |  4 +-
 arbor/morph/embed_pwlin.cpp                   | 12 +--
 arbor/morph/locset.cpp                        | 16 ++--
 arbor/morph/place_pwlin.cpp                   |  6 +-
 arbor/morph/region.cpp                        | 33 ++++---
 arbor/partition_load_balance.cpp              |  9 +-
 arbor/s_expr.cpp                              |  4 +-
 arbor/simulation.cpp                          | 13 ++-
 arbor/thread_private_spike_store.cpp          | 17 ++--
 arbor/thread_private_spike_store.hpp          |  2 +-
 arbor/util/padded_alloc.hpp                   |  2 +-
 arbor/util/piecewise.hpp                      |  2 +-
 arbor/util/unwind.cpp                         |  2 -
 arbor/util/unwind.hpp                         |  2 +-
 arborio/cableio.cpp                           | 30 ++++---
 arborio/neurolucida.cpp                       | 10 +--
 sup/include/sup/ioutil.hpp                    |  2 +-
 40 files changed, 167 insertions(+), 183 deletions(-)

diff --git a/arbor/backends/event.hpp b/arbor/backends/event.hpp
index d6656102..d17792ab 100644
--- a/arbor/backends/event.hpp
+++ b/arbor/backends/event.hpp
@@ -15,7 +15,7 @@ struct target_handle {
     cell_local_size_type mech_index; // instance of the mechanism
     cell_size_type intdom_index;     // which integration domain (acts as index into e.g. vec_t)
 
-    target_handle() {}
+    target_handle() = default;
     target_handle(cell_local_size_type mech_id, cell_local_size_type mech_index, cell_size_type intdom_index):
         mech_id(mech_id), mech_index(mech_index), intdom_index(intdom_index) {}
 };
diff --git a/arbor/backends/multicore/multi_event_stream.hpp b/arbor/backends/multicore/multi_event_stream.hpp
index 6fe2a61a..03c2c29d 100644
--- a/arbor/backends/multicore/multi_event_stream.hpp
+++ b/arbor/backends/multicore/multi_event_stream.hpp
@@ -33,7 +33,7 @@ public:
 
     using state = multi_event_stream_state<event_data_type>;
 
-    multi_event_stream() {}
+    multi_event_stream() = default;
 
     explicit multi_event_stream(size_type n_stream):
        span_begin_(n_stream), span_end_(n_stream), mark_(n_stream) {}
diff --git a/arbor/benchmark_cell_group.cpp b/arbor/benchmark_cell_group.cpp
index 0069b405..8abe81c2 100644
--- a/arbor/benchmark_cell_group.cpp
+++ b/arbor/benchmark_cell_group.cpp
@@ -39,7 +39,7 @@ benchmark_cell_group::benchmark_cell_group(const std::vector<cell_gid_type>& gid
         cg_targets.add_label(c.target, {0, 1});
     }
 
-    reset();
+    benchmark_cell_group::reset();
 }
 
 void benchmark_cell_group::reset() {
diff --git a/arbor/communication/communicator.hpp b/arbor/communication/communicator.hpp
index fa93adfc..61872c86 100644
--- a/arbor/communication/communicator.hpp
+++ b/arbor/communication/communicator.hpp
@@ -28,7 +28,7 @@ namespace arb {
 
 class ARB_ARBOR_API communicator {
 public:
-    communicator() {}
+    communicator() = default;
 
     explicit communicator(const recipe& rec,
                           const domain_decomposition& dom_dec,
diff --git a/arbor/domain_decomposition.cpp b/arbor/domain_decomposition.cpp
index 7a58d804..de443ab3 100644
--- a/arbor/domain_decomposition.cpp
+++ b/arbor/domain_decomposition.cpp
@@ -33,8 +33,9 @@ domain_decomposition::domain_decomposition(
         std::unordered_map<cell_gid_type, int> gid_map;
     };
 
-    unsigned num_domains = ctx->distributed->size();
-    int domain_id = ctx->distributed->id();
+    const auto* dist = ctx->distributed.get();
+    unsigned num_domains = dist->size();
+    int domain_id = dist->id();
     cell_size_type num_global_cells = rec.num_cells();
     const bool has_gpu = ctx->gpu->has_gpu();
 
@@ -62,7 +63,7 @@ domain_decomposition::domain_decomposition(
     }
     cell_size_type num_local_cells = local_gids.size();
 
-    auto global_gids = ctx->distributed->gather_gids(local_gids);
+    auto global_gids = dist->gather_gids(local_gids);
     if (global_gids.size() != num_global_cells) {
         throw invalid_sum_local_cells(global_gids.size(), num_global_cells);
     }
diff --git a/arbor/event_queue.hpp b/arbor/event_queue.hpp
index 59885b43..5e314f8d 100644
--- a/arbor/event_queue.hpp
+++ b/arbor/event_queue.hpp
@@ -27,7 +27,7 @@ public:
     using value_type = Event;
     using event_time_type = ::arb::event_time_type<Event>;
 
-    event_queue() {}
+    event_queue() = default;
 
     void push(const value_type& e) {
          queue_.push(e);
diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp
index 65d0a61a..5f7dd6bc 100644
--- a/arbor/fvm_layout.cpp
+++ b/arbor/fvm_layout.cpp
@@ -105,7 +105,7 @@ cv_geometry::cv_geometry(const cable_cell& cell, const locset& ls):
     cell_cv_divs = {0, (fvm_index_type)n_cv};
 }
 
-fvm_size_type cv_geometry::location_cv(size_type cell_idx, mlocation loc, cv_prefer::type prefer) const {
+fvm_size_type cv_geometry::location_cv(size_type cell_idx, const mlocation& loc, cv_prefer::type prefer) const {
     auto& pw_cv_offset = branch_cv_map.at(cell_idx).at(loc.branch);
     auto zero_extent = [&pw_cv_offset](auto j) {
         return pw_cv_offset.extent(j).first==pw_cv_offset.extent(j).second;
@@ -248,18 +248,23 @@ ARB_ARBOR_API fvm_cv_discretization fvm_cv_discretize(const cable_cell& cell, co
     double dflt_potential =   *(dflt.init_membrane_potential | global_dflt.init_membrane_potential);
     double dflt_temperature = *(dflt.temperature_K | global_dflt.temperature_K);
 
+    const auto& assignments = cell.region_assignments();
+    const auto& resistivity = assignments.get<axial_resistivity>();
+    const auto& capacitance = assignments.get<membrane_capacitance>();
+    const auto& potential   = assignments.get<init_membrane_potential>();
+    const auto& temperature = assignments.get<temperature_K>();
+
     D.axial_resistivity.resize(1);
     msize_t n_branch = D.geometry.n_branch(0);
-    D.axial_resistivity[0].reserve(n_branch);
+    auto& ax_res_0 = D.axial_resistivity[0];
+    ax_res_0.reserve(n_branch);
     for (msize_t i = 0; i<n_branch; ++i) {
-        D.axial_resistivity[0].push_back(pw_over_cable(cell.region_assignments().get<axial_resistivity>(),
-                    mcable{i, 0., 1.}, dflt_resistivity));
+        ax_res_0.emplace_back(pw_over_cable(resistivity, mcable{i, 0., 1.}, dflt_resistivity));
     }
 
     const auto& embedding = cell.embedding();
     for (auto i: count_along(D.geometry.cv_parent)) {
         auto cv_cables = D.geometry.cables(i);
-
         // Computing face_conductance:
         //
         // Flux between adjacent CVs is computed as if there were no membrane currents, and with the CV voltage
@@ -267,13 +272,10 @@ ARB_ARBOR_API fvm_cv_discretization fvm_cv_discretize(const cable_cell& cell, co
         //     * If the CV is unbranched, the reference point is taken to be the CV midpoint.
         //     * If the CV is branched, the reference point is taken to be closest branch point to
         //       the interface between the two CVs.
-
         D.face_conductance[i] = 0;
-
         fvm_index_type p = D.geometry.cv_parent[i];
         if (p!=-1) {
             auto parent_cables = D.geometry.cables(p);
-
             msize_t bid = cv_cables.front().branch;
             double parent_refpt = 0;
             double cv_refpt = 1;
@@ -303,28 +305,19 @@ ARB_ARBOR_API fvm_cv_discretization fvm_cv_discretize(const cable_cell& cell, co
         double cv_length = 0;
 
         for (mcable c: cv_cables) {
-            D.cv_area[i] += embedding.integrate_area(c);
-
-            D.cv_capacitance[i] += embedding.integrate_area(c.branch,
-                pw_over_cable(cell.region_assignments().get<membrane_capacitance>(), c, dflt_capacitance));
-
-            D.init_membrane_potential[i] += embedding.integrate_area(c.branch,
-                pw_over_cable(cell.region_assignments().get<init_membrane_potential>(), c, dflt_potential));
-
-            D.temperature_K[i] += embedding.integrate_area(c.branch,
-                pw_over_cable(cell.region_assignments().get<temperature_K>(), c, dflt_temperature));
-
+            D.cv_area[i]                 += embedding.integrate_area(c);
+            D.cv_capacitance[i]          += embedding.integrate_area(c.branch, pw_over_cable(capacitance, c, dflt_capacitance));
+            D.init_membrane_potential[i] += embedding.integrate_area(c.branch, pw_over_cable(potential,   c, dflt_potential));
+            D.temperature_K[i]           += embedding.integrate_area(c.branch, pw_over_cable(temperature, c, dflt_temperature));
             cv_length += embedding.integrate_length(c);
         }
 
         if (D.cv_area[i]>0) {
             D.init_membrane_potential[i] /= D.cv_area[i];
             D.temperature_K[i] /= D.cv_area[i];
-
             // If parent is trivial, and there is no grandparent, then we can use values from this CV
             // to get initial values for the parent. (The other case, when there is a grandparent, is
             // caught below.)
-
             if (p!=-1 && D.geometry.cv_parent[p]==-1 && D.cv_area[p]==0) {
                 D.init_membrane_potential[p] = D.init_membrane_potential[i];
                 D.temperature_K[p] = D.temperature_K[i];
@@ -332,7 +325,6 @@ ARB_ARBOR_API fvm_cv_discretization fvm_cv_discretize(const cable_cell& cell, co
         }
         else if (p!=-1) {
             // Use parent CV to get a sensible initial value for voltage and temp on zero-size CVs.
-
             D.init_membrane_potential[i] = D.init_membrane_potential[p];
             D.temperature_K[i] = D.temperature_K[p];
         }
@@ -341,7 +333,6 @@ ARB_ARBOR_API fvm_cv_discretization fvm_cv_discretize(const cable_cell& cell, co
             D.diam_um[i] = D.cv_area[i]/(cv_length*math::pi<double>);
         }
     }
-
     return D;
 }
 
@@ -388,7 +379,7 @@ struct voltage_reference_pair {
 };
 
 // Collection of other locations that are coincident under projection.
-std::vector<mlocation> coincident_locations(const morphology& m, mlocation x) {
+std::vector<mlocation> coincident_locations(const morphology& m, const mlocation& x) {
     std::vector<mlocation> result;
     if (x.pos==0) {
         msize_t parent_bid = m.branch_parent(x.branch);
@@ -411,7 +402,7 @@ std::vector<mlocation> coincident_locations(const morphology& m, mlocation x) {
 
 // Test if location intersects (sorted) sequence of cables.
 template <typename Seq>
-bool cables_intersect_location(Seq&& cables, mlocation x) {
+bool cables_intersect_location(Seq&& cables, const mlocation& x) {
     struct cmp_branch {
         bool operator()(const mcable& c, msize_t bid) const { return c.branch<bid; }
         bool operator()(msize_t bid, const mcable& c) const { return bid<c.branch; }
@@ -425,7 +416,7 @@ bool cables_intersect_location(Seq&& cables, mlocation x) {
         [&x](const mcable& c) { return c.prox_pos<=x.pos && x.pos<=c.dist_pos; });
 }
 
-voltage_reference_pair fvm_voltage_reference_points(const morphology& morph, const cv_geometry& geom, fvm_size_type cell_idx, mlocation site) {
+voltage_reference_pair fvm_voltage_reference_points(const morphology& morph, const cv_geometry& geom, fvm_size_type cell_idx, const mlocation& site) {
     voltage_reference site_ref, parent_ref, child_ref;
     bool check_parent = true, check_child = true;
     msize_t bid = site.branch;
@@ -439,7 +430,7 @@ voltage_reference_pair fvm_voltage_reference_points(const morphology& morph, con
         return mlocation{c.branch, (c.prox_pos+c.dist_pos)/2};
     };
 
-    auto cv_contains_fork = [&](auto cv, mlocation x) {
+    auto cv_contains_fork = [&](auto cv, const mlocation& x) {
         // CV contains fork if it intersects any location coincident with x
         // other than x itselfv.
 
@@ -515,7 +506,7 @@ voltage_reference_pair fvm_voltage_reference_points(const morphology& morph, con
 
 // Interpolate membrane voltage from reference points in adjacent CVs.
 
-ARB_ARBOR_API fvm_voltage_interpolant fvm_interpolate_voltage(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, mlocation site) {
+ARB_ARBOR_API fvm_voltage_interpolant fvm_interpolate_voltage(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, const mlocation& site) {
     auto& embedding = cell.embedding();
     fvm_voltage_interpolant vi;
 
@@ -556,7 +547,7 @@ ARB_ARBOR_API fvm_voltage_interpolant fvm_interpolate_voltage(const cable_cell&
 
 // Axial current as linear combination of membrane voltages at reference points in adjacent CVs.
 
-ARB_ARBOR_API fvm_voltage_interpolant fvm_axial_current(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, mlocation site) {
+ARB_ARBOR_API fvm_voltage_interpolant fvm_axial_current(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, const mlocation& site) {
     auto& embedding = cell.embedding();
     fvm_voltage_interpolant vi;
 
@@ -751,6 +742,8 @@ fvm_mechanism_data fvm_build_mechanism_data(
 
     fvm_mechanism_data M;
 
+    const auto& assignments = cell.region_assignments();
+
     // Verify mechanism ion usage, parameter values.
     auto verify_mechanism = [&gprop](const mechanism_info& info, const mechanism_desc& desc) {
         const auto& global_ions = gprop.ion_species;
@@ -802,8 +795,7 @@ fvm_mechanism_data fvm_build_mechanism_data(
 
     // Density mechanisms:
 
-    for (const auto& entry: cell.region_assignments().get<density>()) {
-        const std::string& name = entry.first;
+    for (const auto& [name, cables]: assignments.get<density>()) {
         mechanism_info info = catalogue[name];
 
         fvm_mechanism_config config;
@@ -832,7 +824,7 @@ fvm_mechanism_data fvm_build_mechanism_data(
 
         param_maps.resize(n_param);
 
-        for (auto& on_cable: entry.second) {
+        for (auto& on_cable: cables) {
             const auto& mech = on_cable.second.mech;
             verify_mechanism(info, mech);
             mcable cable = on_cable.first;
@@ -1190,10 +1182,9 @@ fvm_mechanism_data fvm_build_mechanism_data(
     }
 
     // Ions:
-
-    auto initial_iconc_map = cell.region_assignments().get<init_int_concentration>();
-    auto initial_econc_map = cell.region_assignments().get<init_ext_concentration>();
-    auto initial_rvpot_map = cell.region_assignments().get<init_reversal_potential>();
+    auto initial_iconc_map = assignments.get<init_int_concentration>();
+    auto initial_econc_map = assignments.get<init_ext_concentration>();
+    auto initial_rvpot_map = assignments.get<init_reversal_potential>();
 
     for (const auto& [ion, cvs]: ion_support) {
         fvm_ion_config config;
@@ -1212,14 +1203,15 @@ fvm_mechanism_data fvm_build_mechanism_data(
         auto dflt_rvpot = global_ion_data.init_reversal_potential.value();
 
         if (auto ion_data = value_by_key(dflt.ion_data, ion)) {
-            dflt_iconc = ion_data.value().init_int_concentration.value_or(dflt_iconc);
-            dflt_econc = ion_data.value().init_ext_concentration.value_or(dflt_econc);
-            dflt_rvpot = ion_data.value().init_reversal_potential.value_or(dflt_rvpot);
+            auto val = ion_data.value();
+            dflt_iconc = val.init_int_concentration.value_or(dflt_iconc);
+            dflt_econc = val.init_ext_concentration.value_or(dflt_econc);
+            dflt_rvpot = val.init_reversal_potential.value_or(dflt_rvpot);
         }
 
-        const mcable_map<init_int_concentration>&  iconc_on_cable = initial_iconc_map[ion];
-        const mcable_map<init_ext_concentration>&  econc_on_cable = initial_econc_map[ion];
-        const mcable_map<init_reversal_potential>& rvpot_on_cable = initial_rvpot_map[ion];
+        const auto& iconc_on_cable = initial_iconc_map[ion];
+        const auto& econc_on_cable = initial_econc_map[ion];
+        const auto& rvpot_on_cable = initial_rvpot_map[ion];
 
         auto pw_times = [](const pw_elements<double>& pwa, const pw_elements<double>& pwb) {
             return pw_zip_with(pwa, pwb, [](std::pair<double, double>, double a, double b) { return a*b; });
@@ -1275,19 +1267,19 @@ fvm_mechanism_data fvm_build_mechanism_data(
             revpot_specified.insert(ion);
 
             bool writes_this_revpot = false;
-            for (auto& iondep: info.ions) {
-                if (iondep.second.write_reversal_potential) {
-                    if (revpot_tbl.count(iondep.first)) {
-                        auto& existing_revpot_desc = revpot_tbl.at(iondep.first);
+            for (auto& [name, info]: info.ions) {
+                if (info.write_reversal_potential) {
+                    if (revpot_tbl.count(name)) {
+                        auto& existing_revpot_desc = revpot_tbl.at(name);
                         if (existing_revpot_desc.name() != revpot.name() || existing_revpot_desc.values() != revpot.values()) {
                             throw cable_cell_error("inconsistent revpot ion assignment for mechanism "+revpot.name());
                         }
                     }
                     else {
-                        revpot_tbl[iondep.first] = revpot;
+                        revpot_tbl[name] = revpot;
                     }
 
-                    writes_this_revpot |= iondep.first==ion;
+                    writes_this_revpot |= name==ion;
                 }
             }
 
diff --git a/arbor/fvm_layout.hpp b/arbor/fvm_layout.hpp
index b81debcd..9306c78f 100644
--- a/arbor/fvm_layout.hpp
+++ b/arbor/fvm_layout.hpp
@@ -113,7 +113,7 @@ struct ARB_ARBOR_API cv_geometry: public cell_cv_data_impl {
         return branch_cv_map.at(cell_idx).size();
     }
 
-    size_type location_cv(size_type cell_idx, mlocation loc, cv_prefer::type prefer) const;
+    size_type location_cv(size_type cell_idx, const mlocation& loc, cv_prefer::type prefer) const;
 
     cv_geometry(const cable_cell& cell, const locset& ls);
 };
@@ -180,10 +180,10 @@ struct fvm_voltage_interpolant {
 };
 
 // Interpolated membrane voltage.
-ARB_ARBOR_API fvm_voltage_interpolant fvm_interpolate_voltage(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, mlocation site);
+ARB_ARBOR_API fvm_voltage_interpolant fvm_interpolate_voltage(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, const mlocation& site);
 
 // Axial current as linear combiantion of voltages.
-ARB_ARBOR_API fvm_voltage_interpolant fvm_axial_current(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, mlocation site);
+ARB_ARBOR_API fvm_voltage_interpolant fvm_axial_current(const cable_cell& cell, const fvm_cv_discretization& D, fvm_size_type cell_idx, const mlocation& site);
 
 
 // Post-discretization data for point and density mechanism instantiation.
diff --git a/arbor/fvm_lowered_cell_impl.hpp b/arbor/fvm_lowered_cell_impl.hpp
index fcd7c4ee..400df4d8 100644
--- a/arbor/fvm_lowered_cell_impl.hpp
+++ b/arbor/fvm_lowered_cell_impl.hpp
@@ -207,7 +207,8 @@ fvm_integration_result fvm_lowered_cell_impl<Backend>::integrate(
         sample_value_ = array(n_samples);
     }
 
-    state_->deliverable_events.init(std::move(staged_events));
+    auto& events = state_->deliverable_events;
+    events.init(std::move(staged_events));
     sample_events_.init(std::move(staged_samples));
 
     arb_assert((assert_tmin(), true));
@@ -235,7 +236,7 @@ fvm_integration_result fvm_lowered_cell_impl<Backend>::integrate(
         state_->zero_currents();
         PL();
         for (auto& m: mechanisms_) {
-            auto state = state_->deliverable_events.marked_events();
+            auto state = events.marked_events();
             arb_deliverable_event_stream events;
             events.n_streams = state.n;
             events.begin     = state.begin_offset;
@@ -246,12 +247,12 @@ fvm_integration_result fvm_lowered_cell_impl<Backend>::integrate(
         }
 
         PE(advance:integrate:events);
-        state_->deliverable_events.drop_marked_events();
+        events.drop_marked_events();
 
         // Update event list and integration step times.
 
         state_->update_time_to(dt_max, tfinal);
-        state_->deliverable_events.event_time_if_before(state_->time_to);
+        events.event_time_if_before(state_->time_to);
         state_->set_dt();
         PL();
 
@@ -592,10 +593,10 @@ fvm_initialization_data fvm_lowered_cell_impl<Backend>::initialize(
         }
 
         if (config.kind==arb_mechanism_kind_reversal_potential) {
-            revpot_mechanisms_.push_back(mechanism_ptr(minst.mech.release()));
+            revpot_mechanisms_.emplace_back(minst.mech.release());
         }
         else {
-            mechanisms_.push_back(mechanism_ptr(minst.mech.release()));
+            mechanisms_.emplace_back(minst.mech.release());
         }
     }
 
@@ -616,7 +617,7 @@ fvm_initialization_data fvm_lowered_cell_impl<Backend>::initialize(
         std::vector<probe_info> rec_probes = rec.get_probes(gid);
         for (cell_lid_type i: count_along(rec_probes)) {
             probe_info& pi = rec_probes[i];
-            resolve_probe_address(probe_data, cells, cell_idx, std::move(pi.address),
+            resolve_probe_address(probe_data, cells, cell_idx, pi.address,
                 D, mech_data, fvm_info.target_handles, mechptr_by_name);
 
             if (!probe_data.empty()) {
diff --git a/arbor/include/arbor/mechanism.hpp b/arbor/include/arbor/mechanism.hpp
index d5cc221d..8d30eb35 100644
--- a/arbor/include/arbor/mechanism.hpp
+++ b/arbor/include/arbor/mechanism.hpp
@@ -31,7 +31,7 @@ public:
     using index_type = fvm_index_type;
     using size_type  = fvm_size_type;
 
-    mechanism(const arb_mechanism_type m,
+    mechanism(const arb_mechanism_type& m,
               const arb_mechanism_interface& i): mech_{m}, iface_{i}, ppack_{} {
         if (mech_.abi_version != ARB_MECH_ABI_VERSION) throw unsupported_abi_error{mech_.abi_version};
         state_prof_id   = profile::profiler_region_id("advance:integrate:state:"+internal_name());
diff --git a/arbor/include/arbor/morph/embed_pwlin.hpp b/arbor/include/arbor/morph/embed_pwlin.hpp
index 0fe4bf8d..c282cb1f 100644
--- a/arbor/include/arbor/morph/embed_pwlin.hpp
+++ b/arbor/include/arbor/morph/embed_pwlin.hpp
@@ -44,23 +44,23 @@ struct ARB_ARBOR_API embed_pwlin {
     mcable_list projection_cmp(msize_t bid, double proj_lim, comp_op op) const;
 
     // Computed length of mcable.
-    double integrate_length(mcable c) const;
+    double integrate_length(const mcable& c) const;
     double integrate_length(mlocation proxmal, mlocation distal) const;
 
-    double integrate_length(mcable c, const pw_constant_fn&) const;
+    double integrate_length(const mcable& c, const pw_constant_fn&) const;
     double integrate_length(msize_t bid, const pw_constant_fn&) const;
 
     // Membrane surface area of given mcable.
-    double integrate_area(mcable c) const;
+    double integrate_area(const mcable& c) const;
     double integrate_area(mlocation proxmal, mlocation distal) const;
 
-    double integrate_area(mcable c, const pw_constant_fn&) const;
+    double integrate_area(const mcable& c, const pw_constant_fn&) const;
     double integrate_area(msize_t bid, const pw_constant_fn&) const;
 
     // Integrated inverse cross-sectional area of given mcable.
-    double integrate_ixa(mcable c) const;
+    double integrate_ixa(const mcable& c) const;
 
-    double integrate_ixa(mcable c, const pw_constant_fn&) const;
+    double integrate_ixa(const mcable& c, const pw_constant_fn&) const;
     double integrate_ixa(msize_t bid, const pw_constant_fn&) const;
 
     // Length of whole branch.
diff --git a/arbor/include/arbor/morph/mcable_map.hpp b/arbor/include/arbor/morph/mcable_map.hpp
index 76fae779..ce2c270e 100644
--- a/arbor/include/arbor/morph/mcable_map.hpp
+++ b/arbor/include/arbor/morph/mcable_map.hpp
@@ -57,8 +57,7 @@ struct mcable_map {
     bool insert(const mcable& c, T value) {
         auto opt_it = insertion_point(c);
         if (!opt_it) return false;
-
-        elements_.insert(*opt_it, value_type{c, std::move(value)});
+        elements_.emplace(*opt_it, c, std::move(value));
         assert_invariants();
         return true;
     }
@@ -67,12 +66,10 @@ struct mcable_map {
     bool emplace(const mcable& c, Args&&... args) {
         auto opt_it = insertion_point(c);
         if (!opt_it) return false;
-
         elements_.emplace(*opt_it,
            std::piecewise_construct,
            std::forward_as_tuple(c),
            std::forward_as_tuple(std::forward<Args>(args)...));
-
         assert_invariants();
         return true;
     }
@@ -82,7 +79,6 @@ struct mcable_map {
         s.reserve(elements_.size());
         std::transform(elements_.begin(), elements_.end(), std::back_inserter(s),
             [](const auto& x) { return x.first; });
-
         return s;
     }
 
diff --git a/arbor/include/arbor/morph/primitives.hpp b/arbor/include/arbor/morph/primitives.hpp
index 2cfff2ef..6c5d64bf 100644
--- a/arbor/include/arbor/morph/primitives.hpp
+++ b/arbor/include/arbor/morph/primitives.hpp
@@ -59,9 +59,9 @@ struct ARB_SYMBOL_VISIBLE msegment {
 // Describe a specific location on a morpholology.
 struct ARB_SYMBOL_VISIBLE mlocation {
     // The id of the branch.
-    msize_t branch;
+    msize_t branch = 0;
     // The relative position on the branch ∈ [0,1].
-    double pos;
+    double pos = 0.0;
 
     friend std::ostream& operator<<(std::ostream&, const mlocation&);
 };
diff --git a/arbor/include/arbor/morph/region.hpp b/arbor/include/arbor/morph/region.hpp
index 0439e23d..a2825abf 100644
--- a/arbor/include/arbor/morph/region.hpp
+++ b/arbor/include/arbor/morph/region.hpp
@@ -101,7 +101,7 @@ private:
         explicit wrap(Impl&& impl): wrapped(std::move(impl)) {}
 
         virtual std::unique_ptr<interface> clone() override {
-            return std::unique_ptr<interface>(new wrap<Impl>(wrapped));
+            return std::make_unique<wrap<Impl>>(wrapped);
         }
 
         virtual mextent thingify(const mprovider& m) override {
diff --git a/arbor/include/arbor/s_expr.hpp b/arbor/include/arbor/s_expr.hpp
index e8ddb218..c5c4f51f 100644
--- a/arbor/include/arbor/s_expr.hpp
+++ b/arbor/include/arbor/s_expr.hpp
@@ -214,6 +214,7 @@ struct ARB_ARBOR_API s_expr {
     s_expr(s_expr l, s_expr r):
         state(pair_type(std::move(l), std::move(r)))
     {}
+    s_expr& operator=(const s_expr& s) { state = s.state; return *this; }
 
     explicit s_expr(std::string s):
         s_expr(token{{0,0}, tok::string, std::move(s)}) {}
diff --git a/arbor/include/arbor/simd/simd.hpp b/arbor/include/arbor/simd/simd.hpp
index b0d3b028..d9acc07f 100644
--- a/arbor/include/arbor/simd/simd.hpp
+++ b/arbor/include/arbor/simd/simd.hpp
@@ -104,7 +104,7 @@ detail::simd_mask_impl<T> logical_not(const detail::simd_mask_impl<T>& a) {
 }
 
 template <typename T>
-detail::simd_impl<T> fma(const detail::simd_impl<T> a, detail::simd_impl<T> b, detail::simd_impl<T> c) {
+detail::simd_impl<T> fma(const detail::simd_impl<T>& a, detail::simd_impl<T> b, detail::simd_impl<T> c) {
     return detail::simd_impl<T>::wrap(T::fma(a.value_, b.value_, c.value_));
 }
 
@@ -684,7 +684,7 @@ namespace detail {
         #undef ARB_DECLARE_BINARY_COMPARISON_
 
         template <typename T>
-        friend simd_impl<T> arb::simd::fma(const simd_impl<T> a, simd_impl<T> b, simd_impl<T> c);
+        friend simd_impl<T> arb::simd::fma(const simd_impl<T>& a, simd_impl<T> b, simd_impl<T> c);
 
         // Declare Indirect/Indirect indexed/Where Expression copy function as friends
 
diff --git a/arbor/include/arbor/util/any_ptr.hpp b/arbor/include/arbor/util/any_ptr.hpp
index c6e11b82..80f5273c 100644
--- a/arbor/include/arbor/util/any_ptr.hpp
+++ b/arbor/include/arbor/util/any_ptr.hpp
@@ -37,7 +37,7 @@ namespace arb {
 namespace util {
 
 struct ARB_SYMBOL_VISIBLE any_ptr {
-    any_ptr() {}
+    any_ptr() = default;
 
     any_ptr(std::nullptr_t) {}
 
diff --git a/arbor/include/arbor/util/expected.hpp b/arbor/include/arbor/util/expected.hpp
index 45ab99bc..9d860f65 100644
--- a/arbor/include/arbor/util/expected.hpp
+++ b/arbor/include/arbor/util/expected.hpp
@@ -44,7 +44,7 @@ struct bad_expected_access;
 
 template <>
 struct bad_expected_access<void>: std::exception {
-    bad_expected_access() {}
+    bad_expected_access() = default;
     virtual const char* what() const noexcept override { return "bad expected access"; }
 };
 
diff --git a/arbor/include/arbor/util/scope_exit.hpp b/arbor/include/arbor/util/scope_exit.hpp
index f353c1e2..30cdd8b7 100644
--- a/arbor/include/arbor/util/scope_exit.hpp
+++ b/arbor/include/arbor/util/scope_exit.hpp
@@ -50,7 +50,7 @@ namespace impl {
     struct wrap_std_function {
         std::function<R ()> f;
 
-        wrap_std_function() noexcept {}
+        wrap_std_function() noexcept = default;
         wrap_std_function(const std::function<R ()>& f): f(f) {}
         wrap_std_function(std::function<R ()>&& f): f(std::move(f)) {}
         wrap_std_function(wrap_std_function&& other) noexcept {
diff --git a/arbor/lif_cell_group.cpp b/arbor/lif_cell_group.cpp
index 64ca1c36..e7d24e95 100644
--- a/arbor/lif_cell_group.cpp
+++ b/arbor/lif_cell_group.cpp
@@ -18,7 +18,7 @@ lif_cell_group::lif_cell_group(const std::vector<cell_gid_type>& gids, const rec
         }
     }
     // Default to no binning of events
-    set_binning_policy(binning_kind::none, 0);
+    lif_cell_group::set_binning_policy(binning_kind::none, 0);
 
     cells_.reserve(gids_.size());
     last_time_updated_.resize(gids_.size());
@@ -119,4 +119,4 @@ void lif_cell_group::advance_cell(time_type tfinal, time_type dt, cell_gid_type
 
     // This is the last time a cell was updated.
     last_time_updated_[lid] = t;
-}
\ No newline at end of file
+}
diff --git a/arbor/mc_cell_group.cpp b/arbor/mc_cell_group.cpp
index 8d140e6e..4507afae 100644
--- a/arbor/mc_cell_group.cpp
+++ b/arbor/mc_cell_group.cpp
@@ -38,7 +38,7 @@ mc_cell_group::mc_cell_group(const std::vector<cell_gid_type>& gids,
     gids_(gids), lowered_(std::move(lowered))
 {
     // Default to no binning of events
-    set_binning_policy(binning_kind::none, 0);
+    mc_cell_group::set_binning_policy(binning_kind::none, 0);
 
     // Build lookup table for gid to local index.
     for (auto i: util::count_along(gids_)) {
diff --git a/arbor/mechcat.cpp b/arbor/mechcat.cpp
index 4fb428d6..06c60ed2 100644
--- a/arbor/mechcat.cpp
+++ b/arbor/mechcat.cpp
@@ -176,7 +176,7 @@ struct catalogue_state {
 
     // Set mechanism info (unchecked).
     void bind(const std::string& name, mechanism_info info) {
-        info_map_[name] = mechanism_info_ptr(new mechanism_info(std::move(info)));
+        info_map_[name] = std::make_unique<mechanism_info>(std::move(info));
     }
 
     // Add derived mechanism (unchecked).
@@ -488,9 +488,12 @@ struct catalogue_state {
             }
         }
 
-        apply_globals(apply_globals, implicit_deriv? implicit_deriv->parent: name, over);
         if (implicit_deriv) {
-            apply_deriv(over, implicit_deriv.value());
+            apply_globals(apply_globals, implicit_deriv->parent, over);
+            apply_deriv(over, *implicit_deriv);
+        }
+        else {
+            apply_globals(apply_globals, name, over);
         }
 
         return over;
diff --git a/arbor/memory/allocator.hpp b/arbor/memory/allocator.hpp
index 26ca109c..1c2a79f0 100644
--- a/arbor/memory/allocator.hpp
+++ b/arbor/memory/allocator.hpp
@@ -173,9 +173,9 @@ public:
     using rebind = allocator<U, Policy>;
 
 public:
-    allocator() {}
-    ~allocator() {}
-    allocator(allocator const&) {}
+    allocator() = default;
+    ~allocator() = default;
+    allocator(allocator const&) = default;
 
     pointer address(reference r) {
         return &r;
diff --git a/arbor/memory/array_view.hpp b/arbor/memory/array_view.hpp
index 53a69dfd..0180f6fb 100644
--- a/arbor/memory/array_view.hpp
+++ b/arbor/memory/array_view.hpp
@@ -262,7 +262,7 @@ public:
     }
 
     // do nothing for destructor: we don't own the memory in range
-    ~array_view() {}
+    ~array_view() = default;
 
     // test whether memory overlaps that referenced by other
     template <
@@ -428,7 +428,7 @@ public:
     }
 
     // do nothing for destructor: we don't own the memory in range
-    ~const_array_view() {}
+    ~const_array_view() = default;
 
     // test whether memory overlaps that referenced by other
     template <
diff --git a/arbor/morph/embed_pwlin.cpp b/arbor/morph/embed_pwlin.cpp
index dfe84c73..474cabf9 100644
--- a/arbor/morph/embed_pwlin.cpp
+++ b/arbor/morph/embed_pwlin.cpp
@@ -176,15 +176,15 @@ double embed_pwlin::integrate_ixa(msize_t bid, const pw_constant_fn& g) const {
 
 // Integrate over cable:
 
-double embed_pwlin::integrate_length(mcable c) const {
+double embed_pwlin::integrate_length(const mcable& c) const {
     return integrate_length(c.branch, pw_constant_fn{{c.prox_pos, c.dist_pos}, {1.}});
 }
 
-double embed_pwlin::integrate_area(mcable c) const {
+double embed_pwlin::integrate_area(const mcable& c) const {
     return integrate_area(c.branch, pw_constant_fn{{c.prox_pos, c.dist_pos}, {1.}});
 }
 
-double embed_pwlin::integrate_ixa(mcable c) const {
+double embed_pwlin::integrate_ixa(const mcable& c) const {
     return integrate_ixa(c.branch, pw_constant_fn{{c.prox_pos, c.dist_pos}, {1.}});
 }
 
@@ -194,15 +194,15 @@ static pw_constant_fn restrict(const pw_constant_fn& g, double left, double righ
     return pw_zip_with(g, pw_elements<void>{{left, right}});
 }
 
-double embed_pwlin::integrate_length(mcable c, const pw_constant_fn& g) const {
+double embed_pwlin::integrate_length(const mcable& c, const pw_constant_fn& g) const {
     return integrate_length(c.branch, restrict(g, c.prox_pos, c.dist_pos));
 }
 
-double embed_pwlin::integrate_area(mcable c, const pw_constant_fn& g) const {
+double embed_pwlin::integrate_area(const mcable& c, const pw_constant_fn& g) const {
     return integrate_area(c.branch, restrict(g, c.prox_pos, c.dist_pos));
 }
 
-double embed_pwlin::integrate_ixa(mcable c, const pw_constant_fn& g) const {
+double embed_pwlin::integrate_ixa(const mcable& c, const pw_constant_fn& g) const {
     return integrate_ixa(c.branch, restrict(g, c.prox_pos, c.dist_pos));
 }
 
diff --git a/arbor/morph/locset.cpp b/arbor/morph/locset.cpp
index 3502caa0..dc503510 100644
--- a/arbor/morph/locset.cpp
+++ b/arbor/morph/locset.cpp
@@ -23,7 +23,7 @@ namespace arb {
 namespace ls {
 
 // Throw on invalid mlocation.
-void assert_valid(mlocation x) {
+void assert_valid(const mlocation& x) {
     if (!test_invariants(x)) {
         throw invalid_mlocation(x);
     }
@@ -119,7 +119,7 @@ std::ostream& operator<<(std::ostream& o, const terminal_& x) {
 
 // Translate locations in locset distance μm in the proximal direction
 struct proximal_translate_: locset_tag {
-    proximal_translate_(const locset& ls, double distance): start(ls), distance(distance) {}
+    proximal_translate_(locset ls, double distance): start(std::move(ls)), distance(distance) {}
     locset start;
     double distance;
 };
@@ -163,7 +163,7 @@ mlocation_list thingify_(const proximal_translate_& dt, const mprovider& p) {
 }
 
 ARB_ARBOR_API locset proximal_translate(locset ls, double distance) {
-    return locset(proximal_translate_{ls, distance});
+    return locset(proximal_translate_{std::move(ls), distance});
 }
 
 std::ostream& operator<<(std::ostream& o, const proximal_translate_& l) {
@@ -172,13 +172,13 @@ std::ostream& operator<<(std::ostream& o, const proximal_translate_& l) {
 
 // Translate locations in locset distance μm in the distal direction
 struct distal_translate_: locset_tag {
-    distal_translate_(const locset& ls, double distance): start(ls), distance(distance) {}
+    distal_translate_(locset ls, double distance): start(std::move(ls)), distance(distance) {}
     locset start;
     double distance;
 };
 
 ARB_ARBOR_API locset distal_translate(locset ls, double distance) {
-    return locset(distal_translate_{ls, distance});
+    return locset(distal_translate_{std::move(ls), distance});
 }
 
 mlocation_list thingify_(const distal_translate_& dt, const mprovider& p) {
@@ -532,8 +532,8 @@ std::ostream& operator<<(std::ostream& o, const on_components_& x) {
 // Uniform locset.
 
 struct uniform_: locset_tag {
-    uniform_(const arb::region& reg_, unsigned left_, unsigned right_, uint64_t seed_):
-        reg{reg_}, left{left_}, right{right_}, seed{seed_}
+    uniform_(arb::region reg_, unsigned left_, unsigned right_, uint64_t seed_):
+        reg{std::move(reg_)}, left{left_}, right{right_}, seed{seed_}
     {}
     region reg;
     unsigned left;
@@ -542,7 +542,7 @@ struct uniform_: locset_tag {
 };
 
 ARB_ARBOR_API locset uniform(arb::region reg, unsigned left, unsigned right, uint64_t seed) {
-    return locset(uniform_{reg, left, right, seed});
+    return locset(uniform_{std::move(reg), left, right, seed});
 }
 
 mlocation_list thingify_(const uniform_& u, const mprovider& p) {
diff --git a/arbor/morph/place_pwlin.cpp b/arbor/morph/place_pwlin.cpp
index f42475bc..b5492317 100644
--- a/arbor/morph/place_pwlin.cpp
+++ b/arbor/morph/place_pwlin.cpp
@@ -191,7 +191,7 @@ struct p3d {
     friend constexpr double dot(const p3d& l, const p3d& r) {
         return l.x*r.x + l.y*r.y + l.z*r.z;
     }
-    friend double norm(const p3d p) {
+    friend double norm(const p3d& p) {
         return std::sqrt(dot(p, p));
     }
     friend std::ostream& operator<<(std::ostream& o, const p3d& p) {
@@ -220,9 +220,7 @@ std::pair<mlocation, double> place_pwlin::closest(double x, double y, double z)
             // v and w are the proximal and distal ends of the segment.
             const p3d v = seg.prox;
             const p3d w = seg.dist;
-
             const p3d vw = w-v;
-            const p3d vp = p-v;
             const double wvs = dot(vw, vw);
             if (wvs==0.) { // zero length segment is a special case
                 const double distance = norm(p-v);
@@ -237,7 +235,7 @@ std::pair<mlocation, double> place_pwlin::closest(double x, double y, double z)
                 //   t=0 -> proximal end of the segment
                 //   t=1 -> distal end of the segment
                 // values are clamped to the range [0, 1]
-                const double t = std::max(0., std::min(1., dot(vw, vp) / wvs));
+                const double t = std::max(0., std::min(1., dot(vw, p-v) / wvs));
                 const double distance =
                     t<=0.? norm(p-v):
                     t>=1.? norm(p-w):
diff --git a/arbor/morph/region.cpp b/arbor/morph/region.cpp
index cf59e6ea..2b84bed7 100644
--- a/arbor/morph/region.cpp
+++ b/arbor/morph/region.cpp
@@ -215,13 +215,13 @@ std::ostream& operator<<(std::ostream& o, const all_& t) {
 // Region comprising points up to `distance` distal to a point in `start`.
 
 struct distal_interval_: region_tag {
-    distal_interval_(const locset& ls, double d): start{ls}, distance{d} {}
+    distal_interval_(locset ls, double d): start{std::move(ls)}, distance{d} {}
     locset start;
     double distance; //um
 };
 
 ARB_ARBOR_API region distal_interval(locset start, double distance) {
-    return region(distal_interval_{start, distance});
+    return region(distal_interval_{std::move(start), distance});
 }
 
 mextent thingify_(const distal_interval_& reg, const mprovider& p) {
@@ -290,13 +290,13 @@ std::ostream& operator<<(std::ostream& o, const distal_interval_& d) {
 // Region comprising points up to `distance` proximal to a point in `end`.
 
 struct proximal_interval_: region_tag {
-    proximal_interval_(const locset& ls, double d): end{ls}, distance{d} {}
+    proximal_interval_(locset ls, double d): end{std::move(ls)}, distance{d} {}
     locset end;
     double distance; //um
 };
 
 ARB_ARBOR_API region proximal_interval(locset end, double distance) {
-    return region(proximal_interval_{end, distance});
+    return region(proximal_interval_{std::move(end), distance});
 }
 
 mextent thingify_(const proximal_interval_& reg, const mprovider& p) {
@@ -347,29 +347,26 @@ std::ostream& operator<<(std::ostream& o, const proximal_interval_& d) {
 mextent radius_cmp(const mprovider& p, region r, double val, comp_op op) {
     const auto& e = p.embedding();
     auto reg_extent = thingify(r, p);
-
     msize_t bid = mnpos;
     mcable_list cmp_cables;
-
     for (auto c: reg_extent) {
         if (bid != c.branch) {
             bid = c.branch;
             util::append(cmp_cables, e.radius_cmp(bid, val, op));
         }
     }
-
     return intersect(reg_extent, mextent(cmp_cables));
 }
 
 // Region with all segments with radius less than r
 struct radius_lt_: region_tag {
-    radius_lt_(const region& rg, double d): reg{rg}, val{d} {}
+    radius_lt_(region rg, double d): reg{std::move(rg)}, val{d} {}
     region reg;
     double val; //um
 };
 
 ARB_ARBOR_API region radius_lt(region reg, double val) {
-    return region(radius_lt_{reg, val});
+    return region(radius_lt_{std::move(reg), val});
 }
 
 mextent thingify_(const radius_lt_& r, const mprovider& p) {
@@ -382,13 +379,13 @@ std::ostream& operator<<(std::ostream& o, const radius_lt_& r) {
 
 // Region with all segments with radius less than r
 struct radius_le_: region_tag {
-    radius_le_(const region& rg, double d): reg{rg}, val{d} {}
+    radius_le_(region rg, double d): reg{std::move(rg)}, val{d} {}
     region reg;
     double val; //um
 };
 
 ARB_ARBOR_API region radius_le(region reg, double val) {
-    return region(radius_le_{reg, val});
+    return region(radius_le_{std::move(reg), val});
 }
 
 mextent thingify_(const radius_le_& r, const mprovider& p) {
@@ -401,13 +398,13 @@ std::ostream& operator<<(std::ostream& o, const radius_le_& r) {
 
 // Region with all segments with radius greater than r
 struct radius_gt_: region_tag {
-    radius_gt_(const region& rg, double d): reg{rg}, val{d} {}
+    radius_gt_(region rg, double d): reg{std::move(rg)}, val{d} {}
     region reg;
     double val; //um
 };
 
 ARB_ARBOR_API region radius_gt(region reg, double val) {
-    return region(radius_gt_{reg, val});
+    return region(radius_gt_{std::move(reg), val});
 }
 
 mextent thingify_(const radius_gt_& r, const mprovider& p) {
@@ -420,13 +417,13 @@ std::ostream& operator<<(std::ostream& o, const radius_gt_& r) {
 
 // Region with all segments with radius greater than or equal to r
 struct radius_ge_: region_tag {
-    radius_ge_(const region& rg, double d): reg{rg}, val{d} {}
+    radius_ge_(region rg, double d): reg{std::move(rg)}, val{d} {}
     region reg;
     double val; //um
 };
 
 ARB_ARBOR_API region radius_ge(region reg, double val) {
-    return region(radius_ge_{reg, val});
+    return region(radius_ge_{std::move(reg), val});
 }
 
 mextent thingify_(const radius_ge_& r, const mprovider& p) {
@@ -527,13 +524,13 @@ ARB_ARBOR_API region z_dist_from_root_lt(double r0) {
     }
     region lt = reg::projection_lt(r0);
     region gt = reg::projection_gt(-r0);
-    return intersect(std::move(lt), std::move(gt));
+    return intersect(lt, gt);
 }
 
 ARB_ARBOR_API region z_dist_from_root_le(double r0) {
     region le = reg::projection_le(r0);
     region ge = reg::projection_ge(-r0);
-    return intersect(std::move(le), std::move(ge));
+    return intersect(le, ge);
 }
 
 ARB_ARBOR_API region z_dist_from_root_gt(double r0) {
@@ -720,7 +717,7 @@ struct reg_minus: region_tag {
 };
 
 mextent thingify_(const reg_minus& P, const mprovider& p) {
-    return thingify(intersect(std::move(P.lhs), complement(std::move(P.rhs))), p);
+    return thingify(intersect(P.lhs, complement(P.rhs)), p);
 }
 
 std::ostream& operator<<(std::ostream& o, const reg_minus& x) {
diff --git a/arbor/partition_load_balance.cpp b/arbor/partition_load_balance.cpp
index 254bd53a..19f3ac09 100644
--- a/arbor/partition_load_balance.cpp
+++ b/arbor/partition_load_balance.cpp
@@ -27,8 +27,9 @@ ARB_ARBOR_API domain_decomposition partition_load_balance(
 {
     using util::make_span;
 
-    unsigned num_domains = ctx->distributed->size();
-    unsigned domain_id = ctx->distributed->id();
+    const auto& dist = ctx->distributed;
+    unsigned num_domains = dist->size();
+    unsigned domain_id = dist->id();
     const bool gpu_avail = ctx->gpu->has_gpu();
     auto num_global_cells = rec.num_cells();
 
@@ -63,7 +64,7 @@ ARB_ARBOR_API domain_decomposition partition_load_balance(
 
     // Gather the global gj_connection table.
     // The global gj_connection table after gathering is indexed by gid.
-    auto global_gj_connection_table = ctx->distributed->gather_gj_connections(local_gj_connection_table);
+    auto global_gj_connection_table = dist->gather_gj_connections(local_gj_connection_table);
 
     // Make all gj_connections bidirectional.
     std::vector<std::unordered_set<cell_gid_type>> missing_peers(global_gj_connection_table.size());
@@ -224,7 +225,7 @@ ARB_ARBOR_API domain_decomposition partition_load_balance(
 
     // Exchange gid list with all other nodes
     // global all-to-all to gather a local copy of the global gid list on each node.
-    auto global_gids = ctx->distributed->gather_gids(local_gids);
+    auto global_gids = dist->gather_gids(local_gids);
 
     return domain_decomposition(rec, ctx, groups);
 }
diff --git a/arbor/s_expr.cpp b/arbor/s_expr.cpp
index 01e626c3..70da5c7c 100644
--- a/arbor/s_expr.cpp
+++ b/arbor/s_expr.cpp
@@ -281,7 +281,7 @@ private:
         }
 
         // test if the symbol matches a keyword
-        auto it = keyword_to_tok.find(symbol.c_str());
+        auto it = keyword_to_tok.find(symbol);
         if (it!=keyword_to_tok.end()) {
             return {start, it->second, std::move(symbol)};
         }
@@ -413,7 +413,7 @@ std::ostream& print(std::ostream& o, const s_expr& x, int indent) {
     bool first=true;
     o << "(";
     while (it!=end) {
-        if (!first && !it->is_atom() && length(*it)>=0) {
+        if (!first && !it->is_atom()) {
             o << "\n" << in;
             print(o, *it, indent+1);
             ++it;
diff --git a/arbor/simulation.cpp b/arbor/simulation.cpp
index 940de4ba..21a07d3c 100644
--- a/arbor/simulation.cpp
+++ b/arbor/simulation.cpp
@@ -189,9 +189,10 @@ simulation_state::simulation_state(
     local_spikes_({thread_private_spike_store(ctx.thread_pool), thread_private_spike_store(ctx.thread_pool)})
 {
     // Generate the cell groups in parallel, with one task per cell group.
-    cell_groups_.resize(decomp.num_groups());
-    std::vector<cell_labels_and_gids> cg_sources(cell_groups_.size());
-    std::vector<cell_labels_and_gids> cg_targets(cell_groups_.size());
+    auto num_groups = decomp.num_groups();
+    cell_groups_.resize(num_groups);
+    std::vector<cell_labels_and_gids> cg_sources(num_groups);
+    std::vector<cell_labels_and_gids> cg_targets(num_groups);
     foreach_group_index(
         [&](cell_group_ptr& group, int i) {
           const auto& group_info = decomp.group(i);
@@ -204,7 +205,7 @@ simulation_state::simulation_state(
         });
 
     cell_labels_and_gids local_sources, local_targets;
-    for(const auto& i: util::make_span(cell_groups_.size())) {
+    for(const auto& i: util::make_span(num_groups)) {
         local_sources.append(cg_sources.at(i));
         local_targets.append(cg_targets.at(i));
     }
@@ -402,8 +403,6 @@ time_type simulation_state::run(time_type tfinal, time_type dt) {
             });
     };
 
-    threading::task_group g(task_system_.get());
-
     epoch prev = epoch_;
     epoch current = next_epoch(prev, t_interval_);
     epoch next = next_epoch(current, t_interval_);
@@ -418,7 +417,7 @@ time_type simulation_state::run(time_type tfinal, time_type dt) {
     }
     else {
         enqueue(current);
-
+        threading::task_group g(task_system_.get());
         g.run([&]() { enqueue(next); });
         g.run([&]() { update(current); });
         g.wait();
diff --git a/arbor/thread_private_spike_store.cpp b/arbor/thread_private_spike_store.cpp
index f7805350..e1645f03 100644
--- a/arbor/thread_private_spike_store.cpp
+++ b/arbor/thread_private_spike_store.cpp
@@ -1,4 +1,5 @@
 #include <vector>
+#include <numeric>
 
 #include <arbor/common_types.hpp>
 #include <arbor/spike.hpp>
@@ -23,20 +24,14 @@ thread_private_spike_store::thread_private_spike_store(const task_system_handle&
     impl_(new local_spike_store_type(ts))
 {}
 
-thread_private_spike_store::~thread_private_spike_store() {}
+thread_private_spike_store::~thread_private_spike_store() = default;
 
 std::vector<spike> thread_private_spike_store::gather() const {
+    const auto& bs = impl_->buffers_;
+    auto len = std::accumulate(bs.begin(), bs.end(), 0u, [](auto acc, const auto& b) { return acc + b.size(); });
     std::vector<spike> spikes;
-    unsigned num_spikes = 0u;
-    for (auto& b: impl_->buffers_) {
-        num_spikes += b.size();
-    }
-    spikes.reserve(num_spikes);
-
-    for (auto& b: impl_->buffers_) {
-        spikes.insert(spikes.begin(), b.begin(), b.end());
-    }
-
+    spikes.reserve(len);
+    std::for_each(bs.begin(), bs.end(), [&spikes] (const auto& b) { spikes.insert(spikes.end(), b.begin(), b.end()); });
     return spikes;
 }
 
diff --git a/arbor/thread_private_spike_store.hpp b/arbor/thread_private_spike_store.hpp
index ff31f207..d0006ef9 100644
--- a/arbor/thread_private_spike_store.hpp
+++ b/arbor/thread_private_spike_store.hpp
@@ -21,7 +21,7 @@ struct local_spike_store_type;
 /// and collate all of the buffers into a single vector respectively.
 class ARB_ARBOR_API thread_private_spike_store {
 public :
-    thread_private_spike_store();
+    thread_private_spike_store() = default;
     ~thread_private_spike_store();
 
     thread_private_spike_store(thread_private_spike_store&& t);
diff --git a/arbor/util/padded_alloc.hpp b/arbor/util/padded_alloc.hpp
index 9dfe7ea3..d29908f0 100644
--- a/arbor/util/padded_alloc.hpp
+++ b/arbor/util/padded_alloc.hpp
@@ -38,7 +38,7 @@ struct padded_allocator {
     using propagate_on_container_swap = std::true_type;
     using is_always_equal = std::false_type;
 
-    padded_allocator() noexcept {}
+    padded_allocator() noexcept = default;
 
     template <typename U>
     padded_allocator(const padded_allocator<U>& b) noexcept: alignment_(b.alignment()) {}
diff --git a/arbor/util/piecewise.hpp b/arbor/util/piecewise.hpp
index 8f029b4f..1a2a0fae 100644
--- a/arbor/util/piecewise.hpp
+++ b/arbor/util/piecewise.hpp
@@ -742,7 +742,7 @@ struct pw_zip_iterator {
     bool is_end = true;
     typename pw_elements<A>::const_iterator ai, a_end;
     typename pw_elements<B>::const_iterator bi, b_end;
-    double left;
+    double left = std::nan("");
 
     pw_zip_iterator() = default;
     pw_zip_iterator(const pw_elements<A>& a, const pw_elements<B>& b) {
diff --git a/arbor/util/unwind.cpp b/arbor/util/unwind.cpp
index 4e0e1cb9..28ba9afe 100644
--- a/arbor/util/unwind.cpp
+++ b/arbor/util/unwind.cpp
@@ -111,8 +111,6 @@ void backtrace::print(bool stop_at_main) const {
 namespace arb {
 namespace util {
 
-backtrace::backtrace() {}
-
 std::ostream& operator<<(std::ostream& out, const backtrace& trace) {
     return out;
 }
diff --git a/arbor/util/unwind.hpp b/arbor/util/unwind.hpp
index deb8d937..293ecf50 100644
--- a/arbor/util/unwind.hpp
+++ b/arbor/util/unwind.hpp
@@ -19,7 +19,7 @@ struct source_location {
 class backtrace {
 public:
     /// the default constructor will build and store the strack trace.
-    backtrace();
+    backtrace() = default;
 
     /// Creates a new file named backtrace_# where # is a number chosen
     /// The back trace is printed to the file, and a message printed to
diff --git a/arborio/cableio.cpp b/arborio/cableio.cpp
index 88eefd72..e06d81c1 100644
--- a/arborio/cableio.cpp
+++ b/arborio/cableio.cpp
@@ -68,7 +68,7 @@ s_expr mksexp(const threshold_detector& d) {
 }
 s_expr mksexp(const mechanism_desc& d) {
     std::vector<s_expr> mech;
-    mech.push_back(s_expr(d.name()));
+    mech.emplace_back(d.name());
     std::transform(d.values().begin(), d.values().end(), std::back_inserter(mech),
         [](const auto& x){return slist(s_expr(x.first), x.second);});
     return s_expr{"mechanism"_symbol, slist_range(mech)};
@@ -144,7 +144,7 @@ s_expr mksexp(const morphology& morph) {
     };
     std::vector<s_expr> branches;
     for (msize_t i = 0; i < morph.num_branches(); ++i) {
-        branches.push_back(make_branch(i));
+        branches.emplace_back(make_branch(i));
     }
     return s_expr{"morphology"_symbol, slist_range(branches)};
 }
@@ -218,7 +218,7 @@ arb::i_clamp make_i_clamp(const std::vector<arb::i_clamp::envelope_point>& envlp
 pulse_tuple make_envelope_pulse(double delay, double duration, double amplitude) {
     return pulse_tuple{delay, duration, amplitude};
 }
-arb::i_clamp make_i_clamp_pulse(pulse_tuple p, double freq, double phase) {
+arb::i_clamp make_i_clamp_pulse(const pulse_tuple& p, double freq, double phase) {
     return arb::i_clamp::box(std::get<0>(p), std::get<1>(p), std::get<2>(p), freq, phase);
 }
 arb::cv_policy make_cv_policy(const cv_policy& p) {
@@ -235,10 +235,10 @@ T make_wrapped_mechanism(const arb::mechanism_desc& mech) {return T(mech);}
 // Define makers for placeable pairs, paintable pairs, defaultables and decors
 using place_tuple = std::tuple<arb::locset, arb::placeable, std::string>;
 using paint_pair = std::pair<arb::region, arb::paintable>;
-place_tuple make_place(locset where, placeable what, std::string name) {
+place_tuple make_place(const locset& where, const placeable& what, const std::string& name) {
     return place_tuple{where, what, name};
 }
-paint_pair make_paint(region where, paintable what) {
+paint_pair make_paint(const region& where, const paintable& what) {
     return paint_pair{where, what};
 }
 defaultable make_default(defaultable what) {
@@ -259,10 +259,10 @@ decor make_decor(const std::vector<std::variant<place_tuple, paint_pair, default
 // Define maker for locset pairs, region pairs and label_dicts
 using locset_pair = std::pair<std::string, locset>;
 using region_pair = std::pair<std::string, region>;
-locset_pair make_locset_pair(std::string name, locset desc) {
+locset_pair make_locset_pair(const std::string& name, const locset& desc) {
     return locset_pair{name, desc};
 }
-region_pair make_region_pair(std::string name, region desc) {
+region_pair make_region_pair(const std::string name, const region& desc) {
     return region_pair{name, desc};
 }
 label_dict make_label_dict(const std::vector<std::variant<locset_pair, region_pair>>& args) {
@@ -276,7 +276,7 @@ label_dict make_label_dict(const std::vector<std::variant<locset_pair, region_pa
 arb::mpoint make_point(double x, double y, double z, double r) {
     return arb::mpoint{x, y, z, r};
 }
-arb::msegment make_segment(unsigned id, arb::mpoint prox, arb::mpoint dist, int tag) {
+arb::msegment make_segment(unsigned id, const arb::mpoint& prox, const arb::mpoint& dist, int tag) {
     return arb::msegment{id, prox, dist, tag};
 }
 morphology make_morphology(const std::vector<std::variant<branch_tuple>>& args) {
@@ -328,10 +328,10 @@ cable_cell make_cable_cell(const std::vector<std::variant<morphology, label_dict
     }
     return cable_cell(morpho, dict, dec);
 }
-version_tuple make_version(std::string v) {
+version_tuple make_version(const std::string& v) {
     return version_tuple{v};
 }
-meta_data make_meta_data(version_tuple v) {
+meta_data make_meta_data(const version_tuple& v) {
     return meta_data{std::get<0>(v)};
 }
 template <typename T>
@@ -502,9 +502,11 @@ parse_hopefully<std::any> eval(const s_expr& e, const eval_map& map, const eval_
         return eval_atom<cableio_parse_error>(e);
     }
 
-    if (e.head().is_atom()) {
+    auto& hd = e.head();
+    if (hd.is_atom()) {
+        auto& atom = hd.atom();
         // If this is not a symbol, parse as a tuple
-        if (e.head().atom().kind != tok::symbol) {
+        if (atom.kind != tok::symbol) {
             auto args = eval_args(e, map, vec);
             if (!args) {
                 return util::unexpected(args.error());
@@ -535,7 +537,7 @@ parse_hopefully<std::any> eval(const s_expr& e, const eval_map& map, const eval_
         }
 
         // Find all candidate functions that match the name of the function.
-        auto& name = e.head().atom().spelling;
+        auto& name = atom.spelling;
         auto matches = map.equal_range(name);
 
         // Search for a candidate that matches the argument list.
@@ -661,7 +663,7 @@ eval_vec unnamed_evals{
     make_call<double, double>(std::make_tuple<double, double>, "tuple<double, double>")
 };
 
-inline parse_hopefully<std::any> parse(const arb::s_expr& s) {
+inline parse_hopefully<std::any> parse(arb::s_expr s) {
     return eval(std::move(s), named_evals, unnamed_evals);
 }
 
diff --git a/arborio/neurolucida.cpp b/arborio/neurolucida.cpp
index 2cfc8b16..af033796 100644
--- a/arborio/neurolucida.cpp
+++ b/arborio/neurolucida.cpp
@@ -46,8 +46,8 @@ struct parse_error {
         stack.push_back(cpp);
     }
 
-    parse_error& append(cpp_info i) {
-        stack.push_back(i);
+    parse_error& append(const cpp_info& i) {
+        stack.emplace_back(i);
         return *this;
     }
 
@@ -165,9 +165,9 @@ bool is_marker_symbol(const asc::token& t) {
 //  (Color Red)                 ; labeled
 //  (Color RGB (152, 251, 152)) ; RGB literal
 struct asc_color {
-    uint8_t r;
-    uint8_t g;
-    uint8_t b;
+    uint8_t r = 0;
+    uint8_t g = 0;
+    uint8_t b = 0;
 };
 
 [[maybe_unused]]
diff --git a/sup/include/sup/ioutil.hpp b/sup/include/sup/ioutil.hpp
index c1be394b..af05fa6c 100644
--- a/sup/include/sup/ioutil.hpp
+++ b/sup/include/sup/ioutil.hpp
@@ -32,7 +32,7 @@ public:
     typedef typename streambuf_type::off_type off_type;
     typedef typename streambuf_type::traits_type traits_type;
 
-    virtual ~basic_null_streambuf() {}
+    virtual ~basic_null_streambuf() = default;
 
 protected:
     std::streamsize xsputn(const char_type* s, std::streamsize count) override {
-- 
GitLab