Clang-tidy CI test: bump version from 15 to 16 (#5592)

This PR bumps the version used for `clang-tidy` CI tests from 15 to 16.
It also addresses all the issues found with the upgraded tool.

Most of the issues are related to this newly introduced check:
-
[cppcoreguidelines-avoid-const-or-ref-data-members](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members.html)
The check enforces [CppCoreGuidelines about constant and reference data
members](
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c12-dont-make-data-members-const-or-references)
.
In general, I understand the argument against using constant or
reference data members.
~~There is however one case in which I am not fully convinced by the
suggestion of the tool: in
[PML.H](https://github.com/ECP-WarpX/WarpX/pull/5592/files#diff-f1e020ebe3cd2222f399d50ff05769d0c70482f0e12bbe29b498e9ab2d0f4a53)
`const amrex::BoxArray& m_grids;` becomes `amrex::BoxArray m_grids;` and
I am wondering if this can be an issue for performances. Maybe we could
consider using a (possibly smart) pointer to the `BoxArray`, instead of
making a copy.~~ (we are now using a pointer for `amrex::BoxArray
m_grids;`).

Few issues were instead related to these checks:
-
[modernize-loop-convert](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/modernize/loop-convert.html)
This check was already enabled, but `clang-tidy-16` broadens its scope
with respect to previous versions.

-
[modernize-use-auto](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/modernize/use-auto.html)
Only one case. I am a bit confused because this should have been found
also by version 15 of the tool.

-
[misc-use-anonymous-namespace](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.html)
This is a new check. Actually, the issues found with this check were
false positives, but they disappeared when I properly set
`misc-use-anonymous-namespace.HeaderFileExtensions` in the `.clang-tidy`
configuration file to recognize `.H` files as headers.

-
[misc-misplaced-const](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/misc/misplaced-const.html)
Only one case. I am a bit confused because this should have been found
also by version 15 of the tool.

-
[readability-misleading-indentation](https://releases.llvm.org/16.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability/misleading-indentation.html)
[**NOW DISABLED DUE TO FALSE POSITIVES**]
This check was already enabled. However, with this newer version of the
tool, it seems to me that it generates some false positives. Therefore,
I would like to propose to **disable** it. We may try to re-enable it
when we will bump the version from 16 to 17.

Author: lucafedeli88

.clang-tidy

@@ -44,6 +44,7 @@ Checks: '
         -readability-implicit-bool-conversion,
         -readability-isolate-declaration,
         -readability-magic-numbers,
+        -readability-misleading-indentation,
         -readability-named-parameter,
         -readability-uppercase-literal-suffix
     '
@@ -55,6 +56,8 @@ CheckOptions:
   value:        "H,"
 - key:          modernize-pass-by-value.ValuesOnly
   value:        "true"
+- key:          misc-use-anonymous-namespace.HeaderFileExtensions
+  value:        "H,"
 
 
 HeaderFilterRegex: 'Source[a-z_A-Z0-9\/]+\.H$'

.github/workflows/clang_tidy.yml

@@ -26,7 +26,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: install dependencies
       run: |
-        .github/workflows/dependencies/clang.sh 15
+        .github/workflows/dependencies/clang.sh 16
     - name: set up cache
       uses: actions/cache@v4
       with:
@@ -43,8 +43,8 @@ jobs:
         export CCACHE_LOGFILE=${{ github.workspace }}/ccache.log.txt
         ccache -z
 
-        export CXX=$(which clang++-15)
-        export CC=$(which clang-15)
+        export CXX=$(which clang++-16)
+        export CC=$(which clang-16)
 
         cmake -S . -B build_clang_tidy      \
           -DCMAKE_VERBOSE_MAKEFILE=ON       \
@@ -62,7 +62,7 @@ jobs:
 
         ${{github.workspace}}/.github/workflows/source/makeMakefileForClangTidy.py --input ${{github.workspace}}/ccache.log.txt
         make -j4 --keep-going -f clang-tidy-ccache-misses.mak \
-            CLANG_TIDY=clang-tidy-15 \
+            CLANG_TIDY=clang-tidy-16 \
             CLANG_TIDY_ARGS="--config-file=${{github.workspace}}/.clang-tidy --warnings-as-errors=*"
 
         ccache -s

Source/BoundaryConditions/PML.H

@@ -81,10 +81,10 @@ class SigmaBoxFactory
     : public amrex::FabFactory<SigmaBox>
 {
 public:
-    SigmaBoxFactory (const amrex::BoxArray& grid_ba, const amrex::Real* dx,
+    SigmaBoxFactory (const amrex::BoxArray* grid_ba, const amrex::Real* dx,
                      const amrex::IntVect& ncell, const amrex::IntVect& delta,
                      const amrex::Box& regular_domain, const amrex::Real v_sigma_sb)
-        : m_grids(grid_ba), m_dx(dx), m_ncell(ncell), m_delta(delta), m_regdomain(regular_domain), m_v_sigma_sb(v_sigma_sb) {}
+        : m_grids{grid_ba}, m_dx(dx), m_ncell(ncell), m_delta(delta), m_regdomain(regular_domain), m_v_sigma_sb(v_sigma_sb) {}
     ~SigmaBoxFactory () override = default;
 
     SigmaBoxFactory (const SigmaBoxFactory&) = default;
@@ -97,7 +97,7 @@ public:
     [[nodiscard]] SigmaBox* create (const amrex::Box& box, int /*ncomps*/,
         const amrex::FabInfo& /*info*/, int /*box_index*/) const final
     {
-        return new SigmaBox(box, m_grids, m_dx, m_ncell, m_delta, m_regdomain, m_v_sigma_sb);
+        return new SigmaBox(box, *m_grids, m_dx, m_ncell, m_delta, m_regdomain, m_v_sigma_sb);
     }
 
     void destroy (SigmaBox* fab) const final
@@ -112,7 +112,7 @@ public:
     }
 
 private:
-    const amrex::BoxArray& m_grids;
+    const amrex::BoxArray* m_grids;
     const amrex::Real* m_dx;
     amrex::IntVect m_ncell;
     amrex::IntVect m_delta;
@@ -125,7 +125,7 @@ class MultiSigmaBox
 {
 public:
     MultiSigmaBox(const amrex::BoxArray& ba, const amrex::DistributionMapping& dm,
-                  const amrex::BoxArray& grid_ba, const amrex::Real* dx,
+                  const amrex::BoxArray* grid_ba, const amrex::Real* dx,
                   const amrex::IntVect& ncell, const amrex::IntVect& delta,
                   const amrex::Box& regular_domain, amrex::Real v_sigma_sb);
     void ComputePMLFactorsB (const amrex::Real* dx, amrex::Real dt);
@@ -204,8 +204,8 @@ private:
     bool m_dive_cleaning;
     bool m_divb_cleaning;
 
-    const amrex::IntVect m_fill_guards_fields;
-    const amrex::IntVect m_fill_guards_current;
+    amrex::IntVect m_fill_guards_fields;
+    amrex::IntVect m_fill_guards_current;
 
     const amrex::Geometry* m_geom;
     const amrex::Geometry* m_cgeom;

Source/BoundaryConditions/PML.cpp

@@ -506,7 +506,7 @@ SigmaBox::ComputePMLFactorsE (const Real* a_dx, Real dt)
 }
 
 MultiSigmaBox::MultiSigmaBox (const BoxArray& ba, const DistributionMapping& dm,
-                              const BoxArray& grid_ba, const Real* dx,
+                              const BoxArray* grid_ba, const Real* dx,
                               const IntVect& ncell, const IntVect& delta,
                               const amrex::Box& regular_domain, const amrex::Real v_sigma_sb)
     : FabArray<SigmaBox>(ba,dm,1,0,MFInfo(),
@@ -764,7 +764,7 @@ PML::PML (const int lev, const BoxArray& grid_ba,
 
     Box single_domain_box = is_single_box_domain ? domain0 : Box();
     // Empty box (i.e., Box()) means it's not a single box domain.
-    sigba_fp = std::make_unique<MultiSigmaBox>(ba, dm, grid_ba_reduced, geom->CellSize(),
+    sigba_fp = std::make_unique<MultiSigmaBox>(ba, dm, &grid_ba_reduced, geom->CellSize(),
                                                IntVect(ncell), IntVect(delta), single_domain_box, v_sigma_sb);
 
     if (WarpX::electromagnetic_solver_id == ElectromagneticSolverAlgo::PSATD) {
@@ -879,7 +879,7 @@ PML::PML (const int lev, const BoxArray& grid_ba,
         warpx.m_fields.alloc_init(FieldType::pml_j_cp, Direction{2}, lev, cba_jz, cdm, 1, ngb, 0.0_rt, false, false);
 
         single_domain_box = is_single_box_domain ? cdomain : Box();
-        sigba_cp = std::make_unique<MultiSigmaBox>(cba, cdm, grid_cba_reduced, cgeom->CellSize(),
+        sigba_cp = std::make_unique<MultiSigmaBox>(cba, cdm, &grid_cba_reduced, cgeom->CellSize(),
                                                    cncells, cdelta, single_domain_box, v_sigma_sb);
 
         if (WarpX::electromagnetic_solver_id == ElectromagneticSolverAlgo::PSATD) {

Source/BoundaryConditions/PML_RZ.H

@@ -53,8 +53,8 @@ public:
 
 private:
 
-    const int m_ncell;
-    const int m_do_pml_in_domain;
+    int m_ncell;
+    int m_do_pml_in_domain;
     const amrex::Geometry* m_geom;
 
     // The MultiFabs pml_E_fp and pml_B_fp are setup using the registry.

Source/Diagnostics/BTDiagnostics.H

@@ -242,7 +242,7 @@ private:
      *  will be used by all snapshots to obtain lab-frame data at the respective
      *  z slice location.
      */
-    std::string const m_cell_centered_data_name;
+    std::string m_cell_centered_data_name;
     /** Vector of pointers to compute cell-centered data, per level, per component
      *  using the coarsening-ratio provided by the user.
      */

Source/Diagnostics/ComputeDiagFunctors/BackTransformFunctor.H

@@ -100,11 +100,11 @@ public:
                            amrex::Real beta_boost) const;
 private:
     /** pointer to source multifab (cell-centered multi-component multifab) */
-    amrex::MultiFab const * const m_mf_src = nullptr;
+    const amrex::MultiFab* m_mf_src = nullptr;
     /** level at which m_mf_src is defined */
-    int const m_lev;
+    int m_lev;
     /** Number of buffers or snapshots */
-    int const m_num_buffers;
+    int m_num_buffers;
     /** Vector of amrex::Box with index-space in the lab-frame */
     amrex::Vector<amrex::Box> m_buffer_box;
     /** Vector of current z co-ordinate in the boosted-frame for each buffer */

Source/Diagnostics/ComputeDiagFunctors/CellCenterFunctor.H

@@ -36,7 +36,7 @@ public:
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
     /** pointer to source multifab (can be multi-component) */
-    amrex::MultiFab const * const m_mf_src = nullptr;
+    const amrex::MultiFab* m_mf_src = nullptr;
     int m_lev; /**< level on which mf_src is defined (used in cylindrical) */
     /**< (for cylindrical) whether to average all modes into 1 comp */
     bool m_convertRZmodes2cartesian;

Source/Diagnostics/ComputeDiagFunctors/DivBFunctor.H

@@ -42,7 +42,7 @@ public:
 private:
     /** Vector of pointer to source multifab Bx, By, Bz */
     ablastr::fields::VectorField m_arr_mf_src;
-    int const m_lev; /**< level on which mf_src is defined (used in cylindrical) */
+    int m_lev; /**< level on which mf_src is defined (used in cylindrical) */
     /**< (for cylindrical) whether to average all modes into 1 comp */
     bool m_convertRZmodes2cartesian;
 };

Source/Diagnostics/ComputeDiagFunctors/DivEFunctor.H

@@ -41,7 +41,7 @@ public:
 private:
     /** Vector of pointer to source multifab Bx, By, Bz */
     ablastr::fields::VectorField m_arr_mf_src;
-    int const m_lev; /**< level on which mf_src is defined (used in cylindrical) */
+    int m_lev; /**< level on which mf_src is defined (used in cylindrical) */
     /**< (for cylindrical) whether to average all modes into 1 comp */
     bool m_convertRZmodes2cartesian;
 };

Source/Diagnostics/ComputeDiagFunctors/JFunctor.H

@@ -39,7 +39,7 @@ public:
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
     /** direction of the current density to save */
-    const int m_dir;
+    int m_dir;
     /** level on which mf_src is defined */
     int m_lev;
     /** (for cylindrical) whether to average all modes into 1 comp */

Source/Diagnostics/ComputeDiagFunctors/PartPerCellFunctor.H

@@ -30,7 +30,7 @@ public:
      */
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
-    int const m_lev; /**< level on which mf_src is defined */
+    int m_lev; /**< level on which mf_src is defined */
 };
 
 #endif // WARPX_PARTPERCELLFUNCTOR_H_

Source/Diagnostics/ComputeDiagFunctors/PartPerGridFunctor.H

@@ -30,7 +30,7 @@ public:
      */
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
-    int const m_lev; /**< level on which mf_src is defined */
+    int m_lev; /**< level on which mf_src is defined */
 };
 
 #endif // WARPX_PARTPERGRIDFUNCTOR_H_

Source/Diagnostics/ComputeDiagFunctors/ParticleReductionFunctor.H

@@ -43,10 +43,10 @@ public:
      */
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
-    int const m_lev; /**< level on which mf_src is defined */
-    int const m_ispec; /**< index of species to average over */
-    bool const m_do_average; /**< Whether to calculate the average of the data */
-    bool const m_do_filter; /**< whether to apply #m_filter_fn */
+    int m_lev; /**< level on which mf_src is defined */
+    int m_ispec; /**< index of species to average over */
+    bool m_do_average; /**< Whether to calculate the average of the data */
+    bool m_do_filter; /**< whether to apply #m_filter_fn */
     /** Parser function to be averaged by the functor. Arguments: x, y, z, ux, uy, uz */
     std::unique_ptr<amrex::Parser> m_map_fn_parser;
     /** Parser function to filter particles before pass to map. Arguments: x, y, z, ux, uy, uz */

Source/Diagnostics/ComputeDiagFunctors/RhoFunctor.H

@@ -43,13 +43,13 @@ public:
 private:
 
     // Level on which source MultiFab mf_src is defined in RZ geometry
-    int const m_lev;
+    int m_lev;
 
     // Whether to apply k-space filtering of charge density in the diagnostics output in RZ PSATD
     bool m_apply_rz_psatd_filter;
 
     // Species index to dump rho per species
-    const int m_species_index;
+    int m_species_index;
 
     // Whether to average all modes into one component in RZ geometry
     bool m_convertRZmodes2cartesian;

Source/Diagnostics/ComputeDiagFunctors/TemperatureFunctor.H

@@ -28,8 +28,8 @@ public:
      */
     void operator()(amrex::MultiFab& mf_dst, int dcomp, int /*i_buffer=0*/) const override;
 private:
-    int const m_lev; /**< level on which mf_src is defined */
-    int const m_ispec; /**< index of species to average over */
+    int m_lev; /**< level on which mf_src is defined */
+    int m_ispec; /**< index of species to average over */
 };
 
 #endif // WARPX_TEMPERATUREFUNCTOR_H_

Source/Diagnostics/FlushFormats/FlushFormatInSitu.cpp

@@ -27,14 +27,14 @@ FlushFormatInSitu::WriteParticles(const amrex::Vector<ParticleDiag>& particle_di
     // we prefix the fields with "particle_{species_name}" b/c we
     // want to to uniquely name all the fields that can be plotted
 
-    for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
+    for (const auto & particle_diag : particle_diags) {
         Vector<std::string> particle_varnames;
         Vector<std::string> particle_int_varnames;
-        std::string prefix = "particle_" + particle_diags[i].getSpeciesName();
+        std::string prefix = "particle_" + particle_diag.getSpeciesName();
 
         // Get pc for species
         // auto& pc = mypc->GetParticleContainer(i);
-        WarpXParticleContainer* pc = particle_diags[i].getParticleContainer();
+        WarpXParticleContainer* pc = particle_diag.getParticleContainer();
 
         // get names of real comps
         std::map<std::string, int> real_comps_map = pc->getParticleComps();

Source/Diagnostics/FullDiagnostics.cpp

@@ -565,12 +565,10 @@ FullDiagnostics::AddRZModesToDiags (int lev)
 
     // Check if divE is requested
     // If so, all components will be written out
-    bool divE_requested = false;
-    for (int comp = 0; comp < m_varnames.size(); comp++) {
-        if ( m_varnames[comp] == "divE" ) {
-            divE_requested = true;
-        }
-    }
+    const bool divE_requested = std::any_of(
+        std::begin(m_varnames),
+        std::end(m_varnames),
+        [](const auto& varname) { return varname == "divE"; });
 
     // If rho is requested, all components will be written out
     const bool rho_requested = utils::algorithms::is_in( m_varnames, "rho" );

Source/Diagnostics/ReducedDiags/LoadBalanceCosts.H

@@ -40,9 +40,9 @@ public:
      *  (cost, processor, level, i_low, j_low, k_low, gpu_ID [if GPU run], num_cells, num_macro_particles
      * note: the hostname per box is stored separately (in m_data_string) */
 #ifdef AMREX_USE_GPU
-    const int m_nDataFields = 9;
+    static const int m_nDataFields = 9;
 #else
-    const int m_nDataFields = 8;
+    static const int m_nDataFields = 8;
 #endif
 
     /** used to keep track of max number of boxes over all timesteps; this allows

Source/Diagnostics/WarpXOpenPMD.cpp

@@ -531,10 +531,10 @@ WarpXOpenPMDPlot::WriteOpenPMDParticles (const amrex::Vector<ParticleDiag>& part
 {
 WARPX_PROFILE("WarpXOpenPMDPlot::WriteOpenPMDParticles()");
 
-for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
+for (const auto & particle_diag : particle_diags) {
 
-    WarpXParticleContainer* pc = particle_diags[i].getParticleContainer();
-    PinnedMemoryParticleContainer* pinned_pc = particle_diags[i].getPinnedParticleContainer();
+    WarpXParticleContainer* pc = particle_diag.getParticleContainer();
+    PinnedMemoryParticleContainer* pinned_pc = particle_diag.getPinnedParticleContainer();
     if (isBTD || use_pinned_pc) {
         if (!pinned_pc->isDefined()) {
             continue;  // Skip to the next particle container
@@ -546,17 +546,17 @@ for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
         pc->make_alike<amrex::PinnedArenaAllocator>();
 
     const auto mass = pc->AmIA<PhysicalSpecies::photon>() ? PhysConst::m_e : pc->getMass();
-    RandomFilter const random_filter(particle_diags[i].m_do_random_filter,
-                                     particle_diags[i].m_random_fraction);
-    UniformFilter const uniform_filter(particle_diags[i].m_do_uniform_filter,
-                                       particle_diags[i].m_uniform_stride);
-    ParserFilter parser_filter(particle_diags[i].m_do_parser_filter,
+    RandomFilter const random_filter(particle_diag.m_do_random_filter,
+                                     particle_diag.m_random_fraction);
+    UniformFilter const uniform_filter(particle_diag.m_do_uniform_filter,
+                                       particle_diag.m_uniform_stride);
+    ParserFilter parser_filter(particle_diag.m_do_parser_filter,
                                utils::parser::compileParser<ParticleDiag::m_nvars>
-                                     (particle_diags[i].m_particle_filter_parser.get()),
+                                     (particle_diag.m_particle_filter_parser.get()),
                                  pc->getMass(), time);
     parser_filter.m_units = InputUnits::SI;
-    GeometryFilter const geometry_filter(particle_diags[i].m_do_geom_filter,
-                                           particle_diags[i].m_diag_domain);
+    GeometryFilter const geometry_filter(particle_diag.m_do_geom_filter,
+                                           particle_diag.m_diag_domain);
 
     if (isBTD || use_pinned_pc) {
         particlesConvertUnits(ConvertDirection::WarpX_to_SI, pinned_pc, mass);
@@ -587,7 +587,7 @@ for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
     }
 
     // Gather the electrostatic potential (phi) on the macroparticles
-    if ( particle_diags[i].m_plot_phi ) {
+    if ( particle_diag.m_plot_phi ) {
         storePhiOnParticles( tmp, WarpX::electrostatic_solver_id, !use_pinned_pc );
     }
 
@@ -619,7 +619,7 @@ for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
         real_names[x.second+PIdx::nattribs] = detail::snakeToCamel(x.first);
     }
     // plot any "extra" fields by default
-    real_flags = particle_diags[i].m_plot_flags;
+    real_flags = particle_diag.m_plot_flags;
     real_flags.resize(tmp.NumRealComps(), 1);
     // and the names
     int_names.resize(tmp.NumIntComps());
@@ -634,7 +634,7 @@ for (unsigned i = 0, n = particle_diags.size(); i < n; ++i) {
     // real_names contains a list of all real particle attributes.
     // real_flags is 1 or 0, whether quantity is dumped or not.
     DumpToFile(&tmp,
-        particle_diags.at(i).getSpeciesName(),
+        particle_diag.getSpeciesName(),
         m_CurrentStep,
         real_flags,
         int_flags,

Source/FieldSolver/FiniteDifferenceSolver/FiniteDifferenceAlgorithms/FieldAccessorFunctors.H

@@ -42,9 +42,9 @@ struct FieldAccessorMacroscopic
     }
 private:
     /** Array4 of the source field to be scaled and returned by the operator() */
-    amrex::Array4<amrex::Real const> const m_field;
+    amrex::Array4<amrex::Real const> m_field;
     /** Array4 of the macroscopic parameter used to divide m_field in the operator() */
-    amrex::Array4<amrex::Real const> const m_parameter;
+    amrex::Array4<amrex::Real const> m_parameter;
 };
 
 

Source/FieldSolver/FiniteDifferenceSolver/HybridPICModel/HybridPICModel.cpp

@@ -339,7 +339,7 @@ void HybridPICModel::HybridPICSolveE (
     auto& warpx = WarpX::GetInstance();
 
     ablastr::fields::VectorField current_fp_plasma = warpx.m_fields.get_alldirs(FieldType::hybrid_current_fp_plasma, lev);
-    const ablastr::fields::ScalarField electron_pressure_fp = warpx.m_fields.get(FieldType::hybrid_electron_pressure_fp, lev);
+    auto* const electron_pressure_fp = warpx.m_fields.get(FieldType::hybrid_electron_pressure_fp, lev);
 
     // Solve E field in regular cells
     warpx.get_pointer_fdtd_solver_fp(lev)->HybridPICSolveE(