Update clang-tidy Rules (#427)

* Update clang-tidy En-par with WarpX now. * Clang-Tidy Fixes * Update src/ImpactX.cpp
ECP-WarpX · Aug 30, 2023 · 30a6e31 · 30a6e31
1 parent 08efac0
commit 30a6e31
Show file tree

Hide file tree

Showing 19 changed files with 184 additions and 100 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,6 +1,82 @@
-# Future consideration: cppcoreguidelines-*,clang-analyzer-*,google-*,hicpp-*,llvm-*,misc-*,readability-*,-clang-diagnostic-unused-command-line-argument,-*-namespace-comment*,-*-braces-around-statements,-google-readability-todo,-readability-inconsistent-declaration-parameter-name,-readability-named-parameter
-# FIXME: all performance-* reports
-# FIXME: all cert-* reports
-# FIXME: all bugprone-* reports
-Checks: -*,bugprone-*,-bugprone-unhandled-self-assignment,-bugprone-parent-virtual-call,-bugprone-narrowing-conversions,-bugprone-exception-escape,-bugprone-string-literal-with-embedded-nul,cppcoreguidelines-slicing,mpi-*,readability-non-const-parameter,performance-for-range-copy,modernize-*,-modernize-use-trailing-return-type,-modernize-use-bool-literals,-modernize-avoid-c-arrays,-modernize-use-auto,-modernize-return-braced-init-list
-HeaderFilterRegex: '((^(?!\/fetchedamrex-src\/).*)$)'
+Checks: '-*,
+    bugprone-*
+        -bugprone-easily-swappable-parameters,
+        -bugprone-implicit-widening-of-multiplication-result,
+        -bugprone-misplaced-widening-cast,
+    cert-*
+        -cert-err58-cpp,
+    cppcoreguidelines-avoid-goto,
+    cppcoreguidelines-interfaces-global-init,
+        -cppcoreguidelines-avoid-non-const-global-variables,
+        -cppcoreguidelines-init-variables,
+        -cppcoreguidelines-macro-usage,
+        -cppcoreguidelines-narrowing-conversions,
+        -cppcoreguidelines-no-malloc,
+        -cppcoreguidelines-owning-memory,
+    misc-const-correctness,
+    misc-misleading-bidirectional,
+    misc-misleading-identifier,
+    misc-misplaced-const,
+    misc-uniqueptr-reset-release,
+    misc-unused-alias-decls,
+    misc-unused-parameters,
+    misc-unused-using-decls,
+        -misc-definitions-in-headers,
+    modernize-avoid-bind,
+    modernize-concat-nested-namespaces,
+    modernize-deprecated-headers,
+    modernize-deprecated-ios-base-aliases,
+    modernize-loop-convert,
+    modernize-make-shared,
+    modernize-make-unique,
+    modernize-pass-by-value,
+    modernize-raw-string-literal,
+    modernize-redundant-void-arg,
+    modernize-replace-auto-ptr,
+    modernize-replace-disallow-copy-and-assign-macro,
+    modernize-replace-random-shuffle,
+    modernize-return-braced-init-list,
+    modernize-shrink-to-fit,
+    modernize-unary-static-assert,
+    modernize-use-nullptr,
+    mpi-*,
+    performance-faster-string-find,
+    performance-for-range-copy,
+    performance-implicit-conversion-in-loop,
+    performance-inefficient-algorithm,
+    performance-inefficient-string-concatenation,
+    performance-inefficient-vector-operation,
+    performance-move-const-arg,
+    performance-move-constructor-init,
+    performance-no-automatic-move,
+    performance-no-int-to-ptr,
+    readability-avoid-const-params-in-decls,
+    readability-const-return-type,
+    readability-container-contains,
+    readability-container-data-pointer,
+    readability-container-size-empty,
+    readability-non-const-parameter,
+    readability-redundant-control-flow,
+    readability-redundant-declaration,
+    readability-redundant-function-ptr-dereference,
+    readability-redundant-member-init,
+    readability-redundant-preprocessor,
+    readability-redundant-smartptr-get,
+    readability-redundant-string-cstr,
+    readability-redundant-string-init,
+    readability-simplify-boolean-expr,
+    readability-simplify-subscript-expr,
+    readability-static-accessed-through-instance,
+    readability-static-definition-in-anonymous-namespace,
+    readability-string-compare,
+    readability-suspicious-call-argument,
+    readability-uniqueptr-delete-release,
+    readability-use-anyofallof,
+        -readability-uppercase-literal-suffix
+    '
+
+CheckOptions:
+- key:          modernize-pass-by-value.ValuesOnly
+  value:        'true'
+
+HeaderFilterRegex: 'src[a-z_A-Z0-9\/]+\.H$'
diff --git a/src/ImpactX.cpp b/src/ImpactX.cpp
@@ -55,10 +55,11 @@ namespace impactx
         // has set n_cells in the inputs file
         {
             amrex::Vector<int> n_cell(AMREX_SPACEDIM);
-            amrex::ParmParse pp_amr("amr");
+            amrex::ParmParse const pp_amr("amr");
             pp_amr.queryarr("n_cell", n_cell);
 
-            amrex::IntVect lo(amrex::IntVect::TheZeroVector()), hi(n_cell);
+            amrex::IntVect const lo(amrex::IntVect::TheZeroVector());
+            amrex::IntVect hi(n_cell);
             hi -= amrex::IntVect::TheUnitVector();
             amrex::Box index_domain(lo,hi);
             for (int i = 0; i <= max_level; i++)
@@ -111,7 +112,7 @@ namespace impactx
                                           global_step);
 
             // print the initial values of the two invariants H and I
-            std::string diag_name = amrex::Concatenate("diags/nonlinear_lens_invariants_", global_step, file_min_digits);
+            std::string const diag_name = amrex::Concatenate("diags/nonlinear_lens_invariants_", global_step, file_min_digits);
             diagnostics::DiagnosticOutput(*m_particle_container,
                                           diagnostics::OutputType::PrintNonlinearLensInvariants,
                                           diag_name);

diff --git a/src/initialization/InitDistribution.cpp b/src/initialization/InitDistribution.cpp
@@ -64,10 +64,10 @@ namespace impactx
         // position, per MPI rank. We then measure the distribution's spatial
         // extent, create a grid, resize it to fit the beam, and then
         // redistribute particles so that they reside on the correct MPI rank.
-        int myproc = amrex::ParallelDescriptor::MyProc();
-        int nprocs = amrex::ParallelDescriptor::NProcs();
-        int navg = npart / nprocs;
-        int nleft = npart - navg * nprocs;
+        int const myproc = amrex::ParallelDescriptor::MyProc();
+        int const nprocs = amrex::ParallelDescriptor::NProcs();
+        int const navg = npart / nprocs;
+        int const nleft = npart - navg * nprocs;
         int npart_this_proc = (myproc < nleft) ? navg+1 : navg;
         auto const rel_part_this_proc = amrex::ParticleReal(npart_this_proc) /
                                         amrex::ParticleReal(npart);
@@ -110,7 +110,7 @@ namespace impactx
         using namespace amrex::literals;
 
         // Parse the beam distribution parameters
-        amrex::ParmParse pp_dist("beam");
+        amrex::ParmParse const pp_dist("beam");
 
         amrex::ParticleReal energy = 0.0;  // Beam kinetic energy (MeV)
         pp_dist.get("energy", energy);
@@ -169,7 +169,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions waterbag(distribution::Waterbag(
+          distribution::KnownDistributions const waterbag(distribution::Waterbag(
               sigx, sigy, sigt,
               sigpx, sigpy, sigpt,
               muxpx, muypy, mutpt));
@@ -189,7 +189,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions kurth6D(distribution::Kurth6D(
+          distribution::KnownDistributions const kurth6D(distribution::Kurth6D(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));
@@ -209,7 +209,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions gaussian(distribution::Gaussian(
+          distribution::KnownDistributions const gaussian(distribution::Gaussian(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));
@@ -229,7 +229,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions kvDist(distribution::KVdist(
+          distribution::KnownDistributions const kvDist(distribution::KVdist(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));
@@ -249,7 +249,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions kurth4D(distribution::Kurth4D(
+          distribution::KnownDistributions const kurth4D(distribution::Kurth4D(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));
@@ -268,7 +268,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions semigaussian(distribution::Semigaussian(
+          distribution::KnownDistributions const semigaussian(distribution::Semigaussian(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));
@@ -288,7 +288,7 @@ namespace impactx
           pp_dist.query("muypy", muypy);
           pp_dist.query("mutpt", mutpt);
 
-          distribution::KnownDistributions triangle(distribution::Triangle(
+          distribution::KnownDistributions const triangle(distribution::Triangle(
             sigx, sigy, sigt,
             sigpx, sigpy, sigpt,
             muxpx, muypy, mutpt));

diff --git a/src/initialization/InitElement.cpp b/src/initialization/InitElement.cpp
@@ -37,7 +37,7 @@ namespace detail
     template <typename T>
     int queryAddResize (amrex::ParmParse& pp, const char* name, std::vector<T>& ref) {
         std::vector<T> empty;
-        int exist = pp.queryarr(name, empty);
+        int const exist = pp.queryarr(name, empty);
         if (exist) {
             ref.resize(empty.size());
             pp.queryarr(name, ref);
@@ -141,7 +141,7 @@ namespace detail
             amrex::Real ds, escale, freq, phase;
             int nslice = nslice_default;
             int mapsteps = mapsteps_default;
-            RF_field_data ez;
+            RF_field_data const ez;
             std::vector<amrex::ParticleReal> cos_coef = ez.default_cos_coef;
             std::vector<amrex::ParticleReal> sin_coef = ez.default_sin_coef;
             pp_element.get("ds", ds);
@@ -169,7 +169,7 @@ namespace detail
             amrex::Real ds, bscale;
             int nslice = nslice_default;
             int mapsteps = mapsteps_default;
-            Sol_field_data bz;
+            Sol_field_data const bz;
             std::vector<amrex::ParticleReal> cos_coef = bz.default_cos_coef;
             std::vector<amrex::ParticleReal> sin_coef = bz.default_sin_coef;
             pp_element.get("ds", ds);
@@ -183,7 +183,7 @@ namespace detail
             amrex::Real ds, gscale;
             int nslice = nslice_default;
             int mapsteps = mapsteps_default;
-            Quad_field_data gz;
+            Quad_field_data const gz;
             std::vector<amrex::ParticleReal> cos_coef = gz.default_cos_coef;
             std::vector<amrex::ParticleReal> sin_coef = gz.default_sin_coef;
             pp_element.get("ds", ds);
@@ -239,7 +239,7 @@ namespace detail
             pp_element.queryAdd("units", units_str);
             AMREX_ALWAYS_ASSERT_WITH_MESSAGE(units_str == "dimensionless" || units_str == "T-m",
                                              element_name + ".units must be \"dimensionless\" or \"T-m\"");
-            Kicker::UnitSystem units = units_str == "dimensionless" ?
+            Kicker::UnitSystem const units = units_str == "dimensionless" ?
                 Kicker::UnitSystem::dimensionless :
                 Kicker::UnitSystem::Tm;
             m_lattice.emplace_back( Kicker(xkick, ykick, units) );
@@ -306,7 +306,7 @@ namespace detail
         pp_lattice.query("nslice", nslice_default);
 
         // Default number of map integration steps per slice
-        int mapsteps_default = 10;  // used only in RF cavity
+        int const mapsteps_default = 10;  // used only in RF cavity
 
         // Loop through lattice elements
         for (std::string const & element_name : lattice_elements) {

diff --git a/src/initialization/InitMeshRefinement.cpp b/src/initialization/InitMeshRefinement.cpp
@@ -52,7 +52,7 @@ namespace impactx
         };
 
         // charge (rho) mesh
-        amrex::BoxArray cba = ba;
+        amrex::BoxArray const cba = ba;
         // for MR levels (TODO):
         //cba.coarsen(refRatio(lev - 1));
 

diff --git a/src/initialization/Validate.cpp b/src/initialization/Validate.cpp
@@ -43,7 +43,7 @@ namespace impactx
         }
 
         // elements
-        if (m_lattice.size() == 0u)
+        if (m_lattice.empty())
             throw std::runtime_error("Beamline lattice has zero elements. Not yet initialized?");
     }
 } // namespace impactx
diff --git a/src/initialization/Warnings.cpp b/src/initialization/Warnings.cpp
@@ -23,7 +23,7 @@ namespace impactx
 {
 void ImpactX::init_warning_logger ()
 {
-    amrex::ParmParse pp_impactx("impactx");
+    amrex::ParmParse const pp_impactx("impactx");
 
     // Set the flag to control if ImpactX has to emit a warning message
     // as soon as a warning is recorded
@@ -34,8 +34,9 @@ void ImpactX::init_warning_logger ()
 
     // Set the WarnPriority threshold to decide if ImpactX has to abort
     // when a warning is recorded
-    if(std::string str_abort_on_warning_threshold = "";
-            pp_impactx.query("abort_on_warning_threshold", str_abort_on_warning_threshold)){
+    if (std::string str_abort_on_warning_threshold;
+        pp_impactx.query("abort_on_warning_threshold", str_abort_on_warning_threshold))
+   {
         std::optional<ablastr::warn_manager::WarnPriority> abort_on_warning_threshold = std::nullopt;
         if (str_abort_on_warning_threshold == "high")
             abort_on_warning_threshold = ablastr::warn_manager::WarnPriority::high;
@@ -61,7 +62,7 @@ bool ImpactX::early_param_check ()
 
     // Print the warning list right after the first step.
     amrex::Print() << ablastr::warn_manager::GetWMInstance()
-            .PrintGlobalWarnings("FIRST STEP");
+        .PrintGlobalWarnings("FIRST STEP");
 
     return true;
 }

diff --git a/src/particles/ImpactXParticleContainer.H b/src/particles/ImpactXParticleContainer.H
@@ -195,7 +195,7 @@ namespace impactx
          *
          * @returns refpart
          */
-        RefPart const
+        RefPart const &
         GetRefParticle () const;
 
         /** Update reference particle element edge

diff --git a/src/particles/ImpactXParticleContainer.cpp b/src/particles/ImpactXParticleContainer.cpp
@@ -22,15 +22,19 @@
 #include <stdexcept>
 
 
-namespace impactx
+namespace
 {
-    bool do_omp_dynamic () {
+    bool do_omp_dynamic ()
+    {
         bool do_dynamic = true;
-        amrex::ParmParse pp_impactx("impactx");
+        amrex::ParmParse const pp_impactx("impactx");
         pp_impactx.query("do_dynamic_scheduling", do_dynamic);
         return do_dynamic;
     }
+}
 
+namespace impactx
+{
     ParIter::ParIter (ContainerType& pc, int level)
         : amrex::ParIter<0, 0, RealSoA::nattribs, IntSoA::nattribs>(pc, level,
                    amrex::MFItInfo().SetDynamic(do_omp_dynamic())) {}
@@ -69,9 +73,9 @@ namespace impactx
 
     void ImpactXParticleContainer::SetParticleShape ()
     {
-        amrex::ParmParse pp_algo("algo");
+        amrex::ParmParse const pp_algo("algo");
         int v = 0;
-        bool has_shape = pp_algo.query("particle_shape", v);
+        bool const has_shape = pp_algo.query("particle_shape", v);
         if (!has_shape)
             throw std::runtime_error("particle_shape is not set, cannot initialize grids with guard cells.");
         SetParticleShape(v);
@@ -154,13 +158,15 @@ namespace impactx
     ImpactXParticleContainer::SetRefParticle (RefPart const refpart)
     {
         m_refpart = refpart;
-    }    RefPart &
+    }
+
+    RefPart &
     ImpactXParticleContainer::GetRefParticle ()
     {
         return m_refpart;
     }
 
-    RefPart const
+    RefPart const &
     ImpactXParticleContainer::GetRefParticle () const
     {
         return m_refpart;