Skip to content

Commit

Permalink
Clang-Tidy III
Browse files Browse the repository at this point in the history 
This is the third batch of changes fixing clang-tidy warnings. Most of the
warnings are in EB code.

Use Clang 14 in one of the Clang CI tests that used to use Clang 7.
  • Loading branch information
@WeiqunZhang
WeiqunZhang committed Mar 12, 2023
1 parent 6d30e83 commit 6e7b332
Show file tree
Hide file tree
Showing 69 changed files with 461 additions and 533 deletions.
3 changes: 3 additions & 0 deletions .clang-tidy
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Checks: >
'-*,
bugprone-*,
-bugprone-easily-swappable-parameters,
-bugprone-exception-escape,
clang-analyzer-*,
-clang-analyzer-optin.mpi.MPI-Checker,
Expand All @@ -24,9 +25,11 @@ Checks: >
performance-*,
readability-*,
-readability-braces-around-statements,
-readability-container-data-pointer,
-readability-else-after-return,
-readability-function-cognitive-complexity,
-readability-function-size,
-readability-identifier-length,
-readability-implicit-bool-conversion,
-readability-isolate-declaration,
-readability-magic-numbers,
Expand Down
13 changes: 7 additions & 6 deletions .github/workflows/clang.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ jobs:
ctest --output-on-failure
tests_clang:
name: Clang@7.0 C++17 SP Particles DP Mesh Debug [tests]
runs-on: ubuntu-20.04
name: Clang@14.0 C++17 SP Particles DP Mesh Debug [tests]
runs-on: ubuntu-22.04
env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -O1 -Wnon-virtual-dtor"}
# It's too slow with -O0
steps:
- uses: actions/checkout@v3
- name: Dependencies
run: .github/workflows/dependencies/dependencies_clang7.sh
run: .github/workflows/dependencies/dependencies_clang.sh 14
- name: Build & Install
run: |
mkdir build
Expand All @@ -69,9 +69,10 @@ jobs:
-DAMReX_PARTICLES=ON \
-DAMReX_PRECISION=DOUBLE \
-DAMReX_PARTICLES_PRECISION=SINGLE \
-DCMAKE_C_COMPILER=$(which clang) \
-DCMAKE_CXX_COMPILER=$(which clang++) \
-DCMAKE_Fortran_COMPILER=$(which gfortran)
-DCMAKE_C_COMPILER=$(which clang-14) \
-DCMAKE_CXX_COMPILER=$(which clang++-14) \
-DCMAKE_Fortran_COMPILER=$(which gfortran) \
-DAMReX_CLANG_TIDY=ON
make -j 2
ctest --output-on-failure -E GhostsAndVirtuals
Expand Down
17 changes: 17 additions & 0 deletions .github/workflows/dependencies/dependencies_clang.sh
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/usr/bin/env bash
#
# Copyright 2020-2022 The AMReX Community
#
# License: BSD-3-Clause-LBNL
# Authors: Axel Huebl

set -eu -o pipefail

sudo apt-get update

sudo apt-get install -y --no-install-recommends \
build-essential \
gfortran \
clang-$1 \
clang-tidy-$1 \
libomp-$1-dev
2 changes: 1 addition & 1 deletion Src/Amr/AMReX_AmrLevel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ AmrLevel::writePlotFile (const std::string& dir,
#ifdef AMREX_USE_EB
if (EB2::TopIndexSpaceIfPresent()) {
plotMF.setVal(0.0, cnt, 1, nGrow);
auto factory = static_cast<EBFArrayBoxFactory*>(m_factory.get());
auto *factory = static_cast<EBFArrayBoxFactory*>(m_factory.get());
MultiFab::Copy(plotMF,factory->getVolFrac(),0,cnt,1,nGrow);
}
#endif
Expand Down
4 changes: 2 additions & 2 deletions Src/Base/AMReX_FabArray.H
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ public:

bool hasEBFabFactory () const noexcept {
#ifdef AMREX_USE_EB
const auto f = dynamic_cast<EBFArrayBoxFactory const*>(m_factory.get());
const auto *const f = dynamic_cast<EBFArrayBoxFactory const*>(m_factory.get());
return (f != nullptr);
#else
return false;
Expand All @@ -436,7 +436,7 @@ public:

bool isAllRegular () const noexcept {
#ifdef AMREX_USE_EB
const auto f = dynamic_cast<EBFArrayBoxFactory const*>(m_factory.get());
const auto *const f = dynamic_cast<EBFArrayBoxFactory const*>(m_factory.get());
if (f) {
return f->isAllRegular();
} else {
Expand Down
3 changes: 2 additions & 1 deletion Src/Base/AMReX_PlotFileUtil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ EB_WriteMultiLevelPlotfile (const std::string& plotfilename, int nlevels,
PreBuildDirectorHierarchy(plotfilename, levelPrefix, nlevels, callBarrier);
if (!extra_dirs.empty()) {
for (const auto& d : extra_dirs) {
const std::string ed = plotfilename+"/"+d;
std::string ed = plotfilename;
ed.append("/").append(d);
amrex::PreBuildDirectorHierarchy(ed, levelPrefix, nlevels, callBarrier);
}
}
Expand Down
26 changes: 16 additions & 10 deletions Src/EB/AMReX_EB2.H
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ void Finalize ();
class IndexSpace
{
public:
virtual ~IndexSpace() {}
virtual ~IndexSpace() = default;

IndexSpace () noexcept = default;
IndexSpace (IndexSpace const&) = delete;
IndexSpace (IndexSpace &&) = delete;
IndexSpace& operator= (IndexSpace const&) = delete;
IndexSpace& operator= (IndexSpace &&) = delete;

// This function will take the ownership of the IndexSpace
// pointer, and put it on the top of the stack (i.e., back of the
Expand All @@ -44,11 +50,11 @@ public:
return *(m_instance.back());
}
static bool empty () noexcept { return m_instance.empty(); }
static int size () noexcept { return m_instance.size(); }
static int size () noexcept { return static_cast<int>(m_instance.size()); }

virtual const Level& getLevel (const Geometry & geom) const = 0;
virtual const Geometry& getGeometry (const Box& domain) const = 0;
virtual const Box& coarsestDomain () const = 0;
[[nodiscard]] virtual const Level& getLevel (const Geometry & geom) const = 0;
[[nodiscard]] virtual const Geometry& getGeometry (const Box& domain) const = 0;
[[nodiscard]] virtual const Box& coarsestDomain () const = 0;
virtual void addFineLevels (int num_new_fine_levels) = 0;

protected:
Expand All @@ -74,14 +80,14 @@ public:
void operator= (IndexSpaceImp<G> const&) = delete;
void operator= (IndexSpaceImp<G> &&) = delete;

virtual ~IndexSpaceImp () {}
~IndexSpaceImp () override = default;

virtual const Level& getLevel (const Geometry& geom) const final;
virtual const Geometry& getGeometry (const Box& dom) const final;
virtual const Box& coarsestDomain () const final {
[[nodiscard]] const Level& getLevel (const Geometry& geom) const final;
[[nodiscard]] const Geometry& getGeometry (const Box& dom) const final;
[[nodiscard]] const Box& coarsestDomain () const final {
return m_geom.back().Domain();
}
virtual void addFineLevels (int num_new_fine_levels) final;
void addFineLevels (int num_new_fine_levels) final;

using F = typename G::FunctionType;

Expand Down
8 changes: 4 additions & 4 deletions Src/EB/AMReX_EB2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include <AMReX.H>
#include <algorithm>

namespace amrex { namespace EB2 {
namespace amrex::EB2 {

AMREX_EXPORT Vector<std::unique_ptr<IndexSpace> > IndexSpace::m_instance;

Expand Down Expand Up @@ -234,7 +234,7 @@ Build (const Geometry& geom, int required_coarsening_level,
void addFineLevels (int num_new_fine_levels)
{
BL_PROFILE("EB2::addFineLevels()");
auto p = const_cast<IndexSpace*>(TopIndexSpace());
auto *p = const_cast<IndexSpace*>(TopIndexSpace());
if (p) {
p->addFineLevels(num_new_fine_levels);
}
Expand All @@ -255,7 +255,7 @@ BuildFromChkptFile (std::string const& fname,
}

namespace {
static int comp_max_crse_level (Box cdomain, const Box& domain)
int comp_max_crse_level (Box cdomain, const Box& domain)
{
int ilev;
for (ilev = 0; ilev < 30; ++ilev) {
Expand Down Expand Up @@ -283,4 +283,4 @@ maxCoarseningLevel (IndexSpace const* ebis, const Geometry& geom)
return comp_max_crse_level(cdomain,domain);
}

}}
}
170 changes: 68 additions & 102 deletions Src/EB/AMReX_EB2_3D_C.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <AMReX_EB2_C.H>

namespace amrex { namespace EB2 {
namespace amrex::EB2 {

namespace {

Expand Down Expand Up @@ -728,44 +728,31 @@ int build_faces (Box const& bx, Array4<EBCellFlag> const& cell,
AMREX_HOST_DEVICE_FOR_3D(nbxg1, i, j, k,
{
if (levset(i,j,k) < Real(0.0)) {
bool zero_levset = false;
if (xbx.contains(i ,j-1,k-1)
&& fx(i ,j-1,k-1) == Type::covered) {
zero_levset = true;
} else if (xbx.contains(i ,j ,k-1)
&& fx(i ,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (xbx.contains(i ,j-1,k )
&& fx(i ,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (xbx.contains(i ,j ,k )
&& fx(i ,j ,k ) == Type::covered) {
zero_levset = true;
} else if (ybx.contains(i-1,j ,k-1)
&& fy(i-1,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (ybx.contains(i ,j ,k-1)
&& fy(i ,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (ybx.contains(i-1,j ,k )
&& fy(i-1,j ,k ) == Type::covered) {
zero_levset = true;
} else if (ybx.contains(i ,j ,k )
&& fy(i ,j ,k ) == Type::covered) {
zero_levset = true;
} else if (zbx.contains(i-1,j-1,k )
&& fz(i-1,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (zbx.contains(i ,j-1,k )
&& fz(i ,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (zbx.contains(i-1,j ,k )
&& fz(i-1,j ,k ) == Type::covered) {
zero_levset = true;
} else if (zbx.contains(i ,j ,k )
&& fz(i ,j ,k ) == Type::covered) {
zero_levset = true;
}
bool zero_levset =
(xbx.contains(i ,j-1,k-1)
&& fx(i ,j-1,k-1) == Type::covered) ||
(xbx.contains(i ,j ,k-1)
&& fx(i ,j ,k-1) == Type::covered) ||
(xbx.contains(i ,j-1,k )
&& fx(i ,j-1,k ) == Type::covered) ||
(xbx.contains(i ,j ,k )
&& fx(i ,j ,k ) == Type::covered) ||
(ybx.contains(i-1,j ,k-1)
&& fy(i-1,j ,k-1) == Type::covered) ||
(ybx.contains(i ,j ,k-1)
&& fy(i ,j ,k-1) == Type::covered) ||
(ybx.contains(i-1,j ,k )
&& fy(i-1,j ,k ) == Type::covered) ||
(ybx.contains(i ,j ,k )
&& fy(i ,j ,k ) == Type::covered) ||
(zbx.contains(i-1,j-1,k )
&& fz(i-1,j-1,k ) == Type::covered) ||
(zbx.contains(i ,j-1,k )
&& fz(i ,j-1,k ) == Type::covered) ||
(zbx.contains(i-1,j ,k )
&& fz(i-1,j ,k ) == Type::covered) ||
(zbx.contains(i ,j ,k )
&& fz(i ,j ,k ) == Type::covered);
if (zero_levset) {
levset(i,j,k) = Real(0.0);
}
Expand Down Expand Up @@ -886,68 +873,47 @@ void build_cells (Box const& bx, Array4<EBCellFlag> const& cell,
AMREX_HOST_DEVICE_FOR_3D(nbxg1, i, j, k,
{
if (levset(i,j,k) < Real(0.0)) {
bool zero_levset = false;
if (bxg1.contains(i-1,j-1,k-1)
&& cell(i-1,j-1,k-1).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i ,j-1,k-1)
&& cell(i ,j-1,k-1).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i-1,j ,k-1)
&& cell(i-1,j ,k-1).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i ,j ,k-1)
&& cell(i ,j ,k-1).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i-1,j-1,k )
&& cell(i-1,j-1,k ).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i ,j-1,k )
&& cell(i ,j-1,k ).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i-1,j ,k )
&& cell(i-1,j ,k ).isCovered()) {
zero_levset = true;
} else if (bxg1.contains(i ,j ,k )
&& cell(i ,j ,k ).isCovered()) {
zero_levset = true;
} else if (bxg1x.contains(i ,j-1,k-1)
&& fx(i ,j-1,k-1) == Type::covered) {
zero_levset = true;
} else if (bxg1x.contains(i ,j ,k-1)
&& fx(i ,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (bxg1x.contains(i ,j-1,k )
&& fx(i ,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1x.contains(i ,j ,k )
&& fx(i ,j ,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1y.contains(i-1,j ,k-1)
&& fy(i-1,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (bxg1y.contains(i ,j ,k-1)
&& fy(i ,j ,k-1) == Type::covered) {
zero_levset = true;
} else if (bxg1y.contains(i-1,j ,k )
&& fy(i-1,j ,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1y.contains(i ,j ,k )
&& fy(i ,j ,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1z.contains(i-1,j-1,k )
&& fz(i-1,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1z.contains(i ,j-1,k )
&& fz(i ,j-1,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1z.contains(i-1,j ,k )
&& fz(i-1,j ,k ) == Type::covered) {
zero_levset = true;
} else if (bxg1z.contains(i ,j ,k )
&& fz(i ,j ,k ) == Type::covered) {
zero_levset = true;
}
bool zero_levset =
(bxg1.contains(i-1,j-1,k-1)
&& cell(i-1,j-1,k-1).isCovered()) ||
(bxg1.contains(i ,j-1,k-1)
&& cell(i ,j-1,k-1).isCovered()) ||
(bxg1.contains(i-1,j ,k-1)
&& cell(i-1,j ,k-1).isCovered()) ||
(bxg1.contains(i ,j ,k-1)
&& cell(i ,j ,k-1).isCovered()) ||
(bxg1.contains(i-1,j-1,k )
&& cell(i-1,j-1,k ).isCovered()) ||
(bxg1.contains(i ,j-1,k )
&& cell(i ,j-1,k ).isCovered()) ||
(bxg1.contains(i-1,j ,k )
&& cell(i-1,j ,k ).isCovered()) ||
(bxg1.contains(i ,j ,k )
&& cell(i ,j ,k ).isCovered()) ||
(bxg1x.contains(i ,j-1,k-1)
&& fx(i ,j-1,k-1) == Type::covered) ||
(bxg1x.contains(i ,j ,k-1)
&& fx(i ,j ,k-1) == Type::covered) ||
(bxg1x.contains(i ,j-1,k )
&& fx(i ,j-1,k ) == Type::covered) ||
(bxg1x.contains(i ,j ,k )
&& fx(i ,j ,k ) == Type::covered) ||
(bxg1y.contains(i-1,j ,k-1)
&& fy(i-1,j ,k-1) == Type::covered) ||
(bxg1y.contains(i ,j ,k-1)
&& fy(i ,j ,k-1) == Type::covered) ||
(bxg1y.contains(i-1,j ,k )
&& fy(i-1,j ,k ) == Type::covered) ||
(bxg1y.contains(i ,j ,k )
&& fy(i ,j ,k ) == Type::covered) ||
(bxg1z.contains(i-1,j-1,k )
&& fz(i-1,j-1,k ) == Type::covered) ||
(bxg1z.contains(i ,j-1,k )
&& fz(i ,j-1,k ) == Type::covered) ||
(bxg1z.contains(i-1,j ,k )
&& fz(i-1,j ,k ) == Type::covered) ||
(bxg1z.contains(i ,j ,k )
&& fz(i ,j ,k ) == Type::covered);
if (zero_levset) {
levset(i,j,k) = Real(0.0);
}
Expand Down Expand Up @@ -1152,4 +1118,4 @@ void set_connection_flags (Box const& bx,
});
}

}}
}
Loading

0 comments on commit 6e7b332

Please sign in to comment.