Clang-Tidy: modernize-use-ranges

Author: WeiqunZhang

.clang-tidy

@@ -72,5 +72,6 @@ HeaderFilterRegex: '([^n].....|[^o]....|[^l]...|[^i]..|[^n].|[^t])\.H$'
 # Only available in clang-tidy >= 17
 HeaderFileExtensions: ['', "H", 'h', 'hh', 'hpp', 'hxx']
 
-# We will try to modernize this after switching to C++20
-# -modernize-use-ranges
+# modernize-use-ranges is not a required check:
+# (1) Clang <= 15 has bugs with std::ranges.
+# (2) std::ranges::sort requires std::sortable, which is stricter than operator<.

CONTRIBUTING.md

@@ -346,6 +346,9 @@ parenthesis of the parameter list (but not when simply calling the function). Fo
         ```
       * Or define the member **inline** in the class body, where referencing the class
         template parameters in the constraint is fine.
+  * Avoid `std::ranges::find_if` and `std::ranges::unique` — Clang ≤ 15
+    has bugs when compiling them with libstdc++. Use `std::find_if` and `std::unique` instead.
+
 These guidelines should be adhered to in new contributions to AMReX, but
 please refrain from making stylistic changes to unrelated sections of code in your PRs.
 

Src/Amr/AMReX_Amr.cpp

@@ -607,14 +607,14 @@ Amr::finalizeInSitu()
 bool
 Amr::isStatePlotVar (const std::string& name)
 {
-    auto it = std::find(state_plot_vars.begin(), state_plot_vars.end(), name);
+    auto it = std::ranges::find(state_plot_vars, name);
     return (it != state_plot_vars.end());
 }
 
 bool
 Amr::isStateSmallPlotVar (const std::string& name)
 {
-    auto it = std::find(state_small_plot_vars.begin(), state_small_plot_vars.end(), name);
+    auto it = std::ranges::find(state_small_plot_vars, name);
     return (it != state_small_plot_vars.end());
 }
 
@@ -685,14 +685,14 @@ Amr::deleteStatePlotVar (const std::string& name)
 bool
 Amr::isDerivePlotVar (const std::string& name) noexcept
 {
-    auto it = std::find(derive_plot_vars.begin(), derive_plot_vars.end(), name);
+    auto it = std::ranges::find(derive_plot_vars, name);
     return (it != derive_plot_vars.end());
 }
 
 bool
 Amr::isDeriveSmallPlotVar (const std::string& name) noexcept
 {
-    auto it = std::find(derive_small_plot_vars.begin(), derive_small_plot_vars.end(), name);
+    auto it = std::ranges::find(derive_small_plot_vars, name);
     return (it != derive_small_plot_vars.end());
 }
 

Src/AmrCore/AMReX_AmrMesh.cpp

@@ -485,7 +485,7 @@ AmrMesh::ChopGrids (int lev, BoxArray& ba, int target_size) const
             chunk_dir{AMREX_D_DECL(std::make_pair(chunk[0],int(0)),
                                    std::make_pair(chunk[1],int(1)),
                                    std::make_pair(chunk[2],int(2)))};
-        std::sort(chunk_dir.begin(), chunk_dir.end());
+        std::ranges::sort(chunk_dir);
 
         for (int idx = AMREX_SPACEDIM-1; idx >= 0; idx--) {
             int idim = chunk_dir[idx].second;

Src/AmrCore/AMReX_TagBox.cpp

@@ -661,8 +661,8 @@ TagBoxArray::collate (Gpu::PinnedVector<IntVect>& TheGlobalCollateSpace) const
     auto countvec_int = std::vector<int>(countvec.size());
     auto offset_int = std::vector<int>(offset.size());
     const auto mul_funct = [](const auto el){return el*AMREX_SPACEDIM;};
-    std::transform(countvec.begin(), countvec.end(), countvec_int.begin(), mul_funct);
-    std::transform(offset.begin(), offset.end(), offset_int.begin(), mul_funct);
+    std::ranges::transform(countvec, countvec_int.begin(), mul_funct);
+    std::ranges::transform(offset, offset_int.begin(), mul_funct);
     ParallelDescriptor::Gatherv(
         psend_int, count_int, precv_int, countvec_int, offset_int, IOProcNumber);
 #endif

Src/Base/AMReX_BoxArray.cpp

@@ -1943,7 +1943,7 @@ bool match (const BoxArray& x, const BoxArray& y)
 BoxArray decompose (Box const& domain, int nboxes,
                     Array<bool,AMREX_SPACEDIM> const& decomp, bool no_overlap)
 {
-    auto ndecomp = std::count(decomp.begin(), decomp.end(), true);
+    auto ndecomp = std::ranges::count(decomp, true);
 
     if (nboxes <= 1 || ndecomp == 0) {
         return BoxArray(domain);
@@ -2017,7 +2017,7 @@ BoxArray decompose (Box const& domain, int nboxes,
 
         int nprocs_tot = 1;
         while (!factors.empty()) {
-            std::sort(procdim.begin(), procdim.end(), comp);
+            std::ranges::sort(procdim, comp);
             auto f = factors.back();
             factors.pop_back();
             procdim.back().nproc *= f;
@@ -2031,7 +2031,7 @@ BoxArray decompose (Box const& domain, int nboxes,
         // swap to see if the decomposition can be improved.
         while (true)
         {
-            std::sort(procdim.begin(), procdim.end(), comp);
+            std::ranges::sort(procdim, comp);
             auto fit = std::find_if(procdim.begin(),procdim.end(),
                                     [] (ProcDim const& x) { return x.nproc > 1; });
             if (fit == procdim.end()) { break; } // This should not actually happen.

Src/Base/AMReX_BoxDomain.cpp

@@ -138,9 +138,7 @@ BoxDomain::add (const Box& b)
                 cbx = Box();
             }
         }
-        check.erase(std::remove_if(check.begin(), check.end(),
-                                   [](const Box& x) { return x.isEmpty(); }),
-                    check.end());
+        std::erase_if(check, [](const Box& x) { return x.isEmpty(); });
         check.insert(std::end(check), std::begin(tmp), std::end(tmp));
     }
     join(check);

Src/Base/AMReX_BoxList.cpp

@@ -92,9 +92,7 @@ BoxList::catenate (BoxList& blist)
 BoxList&
 BoxList::removeEmpty()
 {
-    m_lbox.erase(std::remove_if(m_lbox.begin(), m_lbox.end(),
-                                [](const Box& x) { return x.isEmpty(); }),
-                 m_lbox.end());
+    std::erase_if(m_lbox, [](const Box& x) { return x.isEmpty(); });
     return *this;
 }
 
@@ -236,8 +234,8 @@ BoxList::BoxList (const Box& bx, int nboxes, Direction dir)
 bool
 BoxList::ok () const noexcept
 {
-    return std::all_of(this->cbegin(), this->cend(),
-                       [] (Box const& b) { return b.ok(); });
+    return std::ranges::all_of(*this,
+                               [] (Box const& b) { return b.ok(); });
 }
 
 bool
@@ -259,8 +257,8 @@ BoxList::contains (const BoxList& bl) const
 
     BoxArray ba(*this);
 
-    return std::all_of(bl.cbegin(), bl.cend(),
-                       [&ba] (Box const& b) { return ba.contains(b); });
+    return std::ranges::all_of(bl,
+                               [&ba] (Box const& b) { return ba.contains(b); });
 }
 
 BoxList&
@@ -653,7 +651,7 @@ boxDiff (BoxList& bl_diff, const Box& b1in, const Box& b2)
 int
 BoxList::simplify (bool best)
 {
-    std::sort(m_lbox.begin(), m_lbox.end(), [](const Box& l, const Box& r) {
+    std::ranges::sort(m_lbox, [](const Box& l, const Box& r) {
             return l.smallEnd() < r.smallEnd(); });
 
     //

Src/Base/AMReX_CArena.cpp

@@ -408,24 +408,21 @@ CArena::freeUnused_protected ()
 {
     std::size_t nbytes = 0;
     std::vector<std::pair<void*, std::size_t>> to_free{};
-    m_alloc.erase(std::remove_if(m_alloc.begin(), m_alloc.end(),
-                                 [&nbytes,&to_free,this] (std::pair<void*,std::size_t> a)
-                                 {
-                                     // We cannot simply use std::set::erase because
-                                     // Node::operator== only compares the starting address.
-                                     auto it = m_freelist.find(Node(a.first,nullptr,0));
-                                     if (it != m_freelist.end() &&
-                                         it->owner() == a.first &&
-                                         it->size()  == a.second)
-                                     {
-                                         it = m_freelist.erase(it);
-                                         nbytes += a.second;
-                                         to_free.emplace_back(a.first, a.second);
-                                         return true;
-                                     }
-                                     return false;
-                                 }),
-                  m_alloc.end());
+    std::erase_if(m_alloc, [&nbytes,&to_free,this] (std::pair<void*,std::size_t> a) {
+        // We cannot simply use std::set::erase because
+        // Node::operator== only compares the starting address.
+        auto it = m_freelist.find(Node(a.first,nullptr,0));
+        if (it != m_freelist.end() &&
+            it->owner() == a.first &&
+            it->size()  == a.second)
+        {
+            it = m_freelist.erase(it);
+            nbytes += a.second;
+            to_free.emplace_back(a.first, a.second);
+            return true;
+        }
+        return false;
+    });
     m_used -= nbytes;
 
     // deallocate_system can call cudafree which may perform implicit synchronization

Src/Base/AMReX_DistributionMapping.cpp

@@ -182,10 +182,10 @@ DistributionMapping::Sort (std::vector<LIpair>& vec,
     if (std::ssize(vec) > 1)
     {
         if (reverse) {
-            std::stable_sort(vec.begin(), vec.end(), LIpairGT());
+            std::ranges::stable_sort(vec, LIpairGT());
         }
         else {
-            std::stable_sort(vec.begin(), vec.end(), LIpairLT());
+            std::ranges::stable_sort(vec, LIpairLT());
         }
     }
 }
@@ -1057,8 +1057,8 @@ DistributionMapping::KnapSackProcessorMap (const DistributionMapping& olddm,
                 }
             }
 
-            AMREX_ASSERT(std::none_of(m_ref->m_pmap.cbegin(), m_ref->m_pmap.cend(),
-                                      [] (int i) { return i < 0; }));
+            AMREX_ASSERT(std::ranges::none_of(m_ref->m_pmap,
+                                              [] (int i) { return i < 0; }));
         }
     }
 }
@@ -1324,7 +1324,7 @@ DistributionMapping::SFCProcessorMapDoIt (const BoxArray&          boxes,
     //
     // Put'm in Morton space filling curve order.
     //
-    std::sort(tokens.begin(), tokens.end(), SFCToken::Compare());
+    std::ranges::sort(tokens, SFCToken::Compare());
     //
     // Split'm up as equitably as possible per team.
     //
@@ -1569,7 +1569,7 @@ DistributionMapping::RRSFCDoIt (const BoxArray&          boxes,
     //
     // Put'm in Morton space filling curve order.
     //
-    std::sort(tokens.begin(), tokens.end(), SFCToken::Compare());
+    std::ranges::sort(tokens, SFCToken::Compare());
 
     Vector<int> ord;
 
@@ -1600,7 +1600,7 @@ DistributionMapping::ConvertCostRealToLong (const Vector<Real>& rcost)
 
     if (rcost.empty()) { return cost; }
 
-    Real wmax = *std::max_element(rcost.begin(), rcost.end());
+    Real wmax = *std::ranges::max_element(rcost);
     Real scale = (wmax == 0) ? 1.e9_rt : 1.e9_rt/wmax;
 
     for (Long i = 0; i < rcost.size(); ++i) {
@@ -1897,7 +1897,7 @@ DistributionMapping::makeSFC (const BoxArray& ba, bool use_box_vol, int nprocs)
     //
     // Put'm in Morton space filling curve order.
     //
-    std::sort(tokens.begin(), tokens.end(), SFCToken::Compare());
+    std::ranges::sort(tokens, SFCToken::Compare());
 
     Real volper = static_cast<Real>(vol_sum) / static_cast<Real>(nprocs);